Merge pull request #1222 from h2database/leftovers-handling

Leftovers handling

h2/src/main/org/h2/command/dml/Select.java

@@ -593,28 +593,23 @@ public class Select extends Query {
                 limitRows += offset;
             }
         }
-        ArrayList<Row> forUpdateRows = null;
-        boolean lockRows = this.isForUpdateMvcc;
-        if (lockRows) {
-            forUpdateRows = Utils.newSmallArrayList();
-        }
+        ArrayList<Row> forUpdateRows = this.isForUpdateMvcc ? Utils.<Row>newSmallArrayList() : null;
         int sampleSize = getSampleSizeValue(session);
         LazyResultQueryFlat lazyResult = new LazyResultQueryFlat(expressionArray,
                 sampleSize, columnCount);
         if (result == null) {
             return lazyResult;
         }
-        while (lazyResult.next()) {
-            if (lockRows) {
+        if (sort != null && !sortUsingIndex || limitRows <= 0) {
+            limitRows = Long.MAX_VALUE;
+        }
+        while (result.getRowCount() < limitRows && lazyResult.next()) {
+            if (forUpdateRows != null) {
                 topTableFilter.lockRowAdd(forUpdateRows);
             }
             result.addRow(lazyResult.currentRow());
-            if ((sort == null || sortUsingIndex) && limitRows > 0 &&
-                    result.getRowCount() >= limitRows) {
-                break;
-            }
         }
-        if (lockRows) {
+        if (forUpdateRows != null) {
             topTableFilter.lockRows(forUpdateRows);
         }
         return null;

h2/src/main/org/h2/engine/Database.java

@@ -2264,13 +2264,13 @@ public class Database implements DataHandler {
     public void setLockMode(int lockMode) {
         switch (lockMode) {
         case Constants.LOCK_MODE_OFF:
-            if (multiThreaded) {
-                // currently the combination of LOCK_MODE=0 and MULTI_THREADED
+            if (multiThreaded && !isMVStore()) {
+                // currently the combination of MVCC=FALSE, LOCK_MODE=0 and MULTI_THREADED
                 // is not supported. also see code in
                 // JdbcDatabaseMetaData#supportsTransactionIsolationLevel(int)
                 throw DbException.get(
                         ErrorCode.UNSUPPORTED_SETTING_COMBINATION,
-                        "LOCK_MODE=0 & MULTI_THREADED");
+                        "MVCC=FALSE & LOCK_MODE=0 & MULTI_THREADED");
             }
             break;
         case Constants.LOCK_MODE_READ_COMMITTED:

h2/src/main/org/h2/mvstore/tx/RollbackDecisionMaker.java

@@ -30,9 +30,11 @@ final class RollbackDecisionMaker extends MVMap.DecisionMaker<Object[]> {
     @Override
     public MVMap.Decision decide(Object[] existingValue, Object[] providedValue) {
         assert decision == null;
-        // normally existingValue will always be there except of db initialization
-        // where some undo log entry was captured on disk but actual map entry was not
-        if (existingValue != null ) {
+        if (existingValue == null) {
+            // normally existingValue will always be there except of db initialization
+            // where some undo log entry was captured on disk but actual map entry was not
+            decision = MVMap.Decision.ABORT;
+        } else {
             VersionedValue valueToRestore = (VersionedValue) existingValue[2];
             long operationId;
             if (valueToRestore == null ||
@@ -47,8 +49,8 @@ final class RollbackDecisionMaker extends MVMap.DecisionMaker<Object[]> {
                     listener.onRollback(map, key, previousValue, valueToRestore);
                 }
             }
+            decision = MVMap.Decision.REMOVE;
         }
-        decision = MVMap.Decision.REMOVE;
         return decision;
     }
 

h2/src/main/org/h2/mvstore/tx/TxDecisionMaker.java

@@ -18,7 +18,7 @@ public abstract class TxDecisionMaker extends MVMap.DecisionMaker<VersionedValue
     final Object                 value;
     private final Transaction    transaction;
     long                         undoKey;
-    private       long           lastOperationId;
+    protected     long           lastOperationId;
     private       Transaction    blockingTransaction;
     private       MVMap.Decision decision;
 
@@ -54,16 +54,18 @@ public abstract class TxDecisionMaker extends MVMap.DecisionMaker<VersionedValue
             // should wait on blockingTransaction that was determined earlier
             decision = MVMap.Decision.ABORT;
         } else if(id == lastOperationId) {
-            // There is no transaction with that id, so we've retried it just before,
+            // There is no transaction with that id, and we've tried it just before,
             // but map root has not changed (which must be the case if we just missed a closed transaction),
             // therefore we came back here again.
             // Now we assume it's a leftover after unclean shutdown (map update was written but not undo log),
-            // and will effectively roll it back (just overwrite).
+            // and will effectively roll it back (just assume committed value and overwrite).
             Object committedValue = existingValue.getCommittedValue();
             logIt(committedValue == null ? null : VersionedValue.getInstance(committedValue));
             decision = MVMap.Decision.PUT;
         } else {
-            // condition above means transaction has been committed/rolled back and closed by now
+            // transaction has been committed/rolled back and is closed by now, so
+            // we can retry immediately and either that entry become committed
+            // or we'll hit case above
             decision = MVMap.Decision.REPEAT;
             lastOperationId = id;
         }
@@ -162,15 +164,28 @@ public abstract class TxDecisionMaker extends MVMap.DecisionMaker<VersionedValue
                     // and therefore will be committed soon
                     logIt(null);
                     return setDecision(MVMap.Decision.PUT);
-                } else if(fetchTransaction(blockingId) == null) {
-                    // map already has specified key from uncommitted
-                    // at the time transaction, which is closed by now
-                    // we can retry right away
-                    return setDecision(MVMap.Decision.REPEAT);
-                } else {
-                    // map already has specified key from uncommitted transaction
-                    // we need to wait for it to close and then try again
+                } else if(fetchTransaction(blockingId) != null) {
+                    // this entry comes from a different transaction, and this transaction is not committed yet
+                    // should wait on blockingTransaction that was determined earlier and then try again
                     return setDecision(MVMap.Decision.ABORT);
+                } else if(id == lastOperationId) {
+                    // There is no transaction with that id, and we've tried it just before,
+                    // but map root has not changed (which must be the case if we just missed a closed transaction),
+                    // therefore we came back here again.
+                    // Now we assume it's a leftover after unclean shutdown (map update was written but not undo log),
+                    // and will effectively roll it back (just assume committed value and overwrite).
+                    Object committedValue = existingValue.getCommittedValue();
+                    if(committedValue != null) {
+                        return setDecision(MVMap.Decision.ABORT);
+                    }
+                    logIt(null);
+                    return setDecision(MVMap.Decision.PUT);
+                } else {
+                    // transaction has been committed/rolled back and is closed by now, so
+                    // we can retry immediately and either that entry become committed
+                    // or we'll hit case above
+                    lastOperationId = id;
+                    return setDecision(MVMap.Decision.REPEAT);
                 }
             }
         }

h2/src/test/org/h2/test/bench/TestScalability.java

@@ -74,7 +74,7 @@ public class TestScalability implements Database.DatabaseTest {
         ArrayList<Database> dbs = new ArrayList<>();
         int id = 1;
         final String h2Url = "jdbc:h2:./data/test;" +
-                "LOCK_TIMEOUT=10000;MV_STORE=FALSE;LOCK_MODE=3";
+                "LOCK_TIMEOUT=10000;MV_STORE=FALSE";
         dbs.add(createDbEntry(id++, "H2", 1, h2Url));
         dbs.add(createDbEntry(id++, "H2", 2, h2Url));
         dbs.add(createDbEntry(id++, "H2", 4, h2Url));
@@ -84,7 +84,7 @@ public class TestScalability implements Database.DatabaseTest {
         dbs.add(createDbEntry(id++, "H2", 64, h2Url));
 
         final String mvUrl = "jdbc:h2:./data/mvTest;" +
-                "LOCK_TIMEOUT=10000;MULTI_THREADED=1";
+                "LOCK_TIMEOUT=10000;MULTI_THREADED=1;LOCK_MODE=0";
         dbs.add(createDbEntry(id++, "MV", 1, mvUrl));
         dbs.add(createDbEntry(id++, "MV", 2, mvUrl));
         dbs.add(createDbEntry(id++, "MV", 4, mvUrl));