remove syncronized from MVStore.renameMap()

make beforeWrite() to call commit() instead of tryCommit() in case of unsaved memory churn

h2/src/main/org/h2/mvstore/FileStore.java

@@ -128,7 +128,6 @@ public class FileStore {
         if (file != null) {
             return;
         }
-        if (fileName != null) {
         // ensure the Cache file system is registered
         FilePathCache.INSTANCE.getScheme();
         FilePath p = FilePath.get(fileName);
@@ -139,7 +138,6 @@ public class FileStore {
             FilePathNio.class.getName();
             fileName = "nio:" + fileName;
         }
-        }
         this.fileName = fileName;
         FilePath f = FilePath.get(fileName);
         FilePath parent = f.getParent();

h2/src/main/org/h2/mvstore/MVStore.java

@@ -354,6 +354,8 @@ public class MVStore {
             retentionTime = this.fileStore.getDefaultRetentionTime();
             int kb = DataUtils.getConfigParam(config, "autoCommitBufferSize", 1024);
             // 19 KB memory is about 1 KB storage
+            // TODO: maybe keep 19 MB as an upper bound,
+            // TODO: but derive actual value from the amount of RAM available
             autoCommitMemory = kb * 1024 * 19;
             autoCompactFillRate = DataUtils.getConfigParam(config, "autoCompactFillRate", 40);
             char[] encryptionKey = (char[]) config.get("encryptionKey");
@@ -1068,8 +1070,16 @@ public class MVStore {
      * @return the new version (incremented if there were changes)
      */
     public synchronized long commit() {
-        currentStoreThread.set(Thread.currentThread());
+        Thread currentThread = Thread.currentThread();
+        Thread storeThread = currentStoreThread.get();
+        if (currentThread != storeThread) { // to avoid re-entrance
+            currentStoreThread.set(currentThread);
+            try {
                 store();
+            } finally {
+                currentStoreThread.set(storeThread);
+            }
+        }
         return currentVersion;
     }
 
@@ -1652,7 +1662,6 @@ public class MVStore {
      * @return if there are any changes
      */
     public boolean hasUnsavedChanges() {
-        assert !metaChanged || meta.hasChangesSince(lastStoredVersion) : metaChanged;
         if (metaChanged) {
             return true;
         }
@@ -2293,14 +2302,21 @@ public class MVStore {
      * @param map the map
      */
     void beforeWrite(MVMap<?, ?> map) {
-        if (saveNeeded && fileStore != null && !closed && autoCommitDelay > 0) {
+        if (saveNeeded && fileStore != null && !closed) {
             saveNeeded = false;
             // check again, because it could have been written by now
             if (unsavedMemory > autoCommitMemory && autoCommitMemory > 0) {
+                // if unsaved memory creation rate is to high,
+                // some back pressure need to be applied
+                // to slow things down and avoid OOME
+                if (3 * unsavedMemory > 4 * autoCommitMemory) {
+                    commit();
+                } else {
                     tryCommit();
                 }
             }
         }
+    }
 
     /**
      * Get the store version. The store version is usually used to upgrade the
@@ -2495,7 +2511,7 @@ public class MVStore {
      * @param map the map
      * @param newName the new name
      */
-    public synchronized void renameMap(MVMap<?, ?> map, String newName) {
+    public void renameMap(MVMap<?, ?> map, String newName) {
         checkOpen();
         DataUtils.checkArgument(map != meta,
                 "Renaming the meta map is not allowed");
@@ -2505,9 +2521,12 @@ public class MVStore {
             DataUtils.checkArgument(
                     !meta.containsKey("name." + newName),
                     "A map named {0} already exists", newName);
-            meta.remove("name." + oldName);
-            meta.put(MVMap.getMapKey(id), map.asString(newName));
+            // at first create a new name as an "alias"
             meta.put("name." + newName, Integer.toHexString(id));
+            // switch roles of a new and old names - old one is an alias now
+            meta.put(MVMap.getMapKey(id), map.asString(newName));
+            // get rid of the old name completely
+            meta.remove("name." + oldName);
             markMetaChanged();
         }
     }

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

@@ -95,8 +95,8 @@ public class TransactionStore {
     private int nextTempMapId;
 
     private static final String UNDO_LOG_NAME_PEFIX = "undoLog";
-    private static final char UNDO_LOG_OPEN = '-';
-    private static final char UNDO_LOG_COMMITTED = '.';
+    private static final char UNDO_LOG_COMMITTED = '-'; // must come before open in lexicographical order
+    private static final char UNDO_LOG_OPEN = '.';
 
     /**
      * Hard limit on the number of concurrently opened transactions
@@ -105,6 +105,11 @@ public class TransactionStore {
     private static final int MAX_OPEN_TRANSACTIONS = 65535;
 
 
+    public static String getUndoLogName(boolean committed, int transactionId) {
+        return UNDO_LOG_NAME_PEFIX +
+                (committed ? UNDO_LOG_COMMITTED : UNDO_LOG_OPEN) +
+                (transactionId > 0 ? String.valueOf(transactionId) : "");
+    }
 
     /**
      * Create a new transaction store.
@@ -158,6 +163,8 @@ public class TransactionStore {
                 if (mapName.startsWith(UNDO_LOG_NAME_PEFIX)) {
                     if (store.hasData(mapName)) {
                         int transactionId = Integer.parseInt(mapName.substring(UNDO_LOG_NAME_PEFIX.length() + 1));
+                        VersionedBitSet openTxBitSet = openTransactions.get();
+                        if (!openTxBitSet.get(transactionId)) {
                             Object[] data = preparedTransactions.get(transactionId);
                             int status;
                             String name;
@@ -169,7 +176,7 @@ public class TransactionStore {
                                 status = (Integer) data[0];
                                 name = (String) data[1];
                             }
-                        MVMap<Long,Object[]> undoLog = store.openMap(mapName, undoLogBuilder);
+                            MVMap<Long, Object[]> undoLog = store.openMap(mapName, undoLogBuilder);
                             undoLogs[transactionId] = undoLog;
                             Long lastUndoKey = undoLog.lastKey();
                             assert lastUndoKey != null;
@@ -179,6 +186,7 @@ public class TransactionStore {
                         }
                     }
                 }
+            }
             init = true;
         }
     }
@@ -333,7 +341,7 @@ public class TransactionStore {
         transactions.set(transactionId, transaction);
 
         if (undoLogs[transactionId] == null) {
-            String undoName = UNDO_LOG_NAME_PEFIX + UNDO_LOG_OPEN + transactionId;
+            String undoName = getUndoLogName(false, transactionId);
             undoLogs[transactionId] = store.openMap(undoName, undoLogBuilder);
         }
         return transaction;
@@ -416,7 +424,7 @@ public class TransactionStore {
             try {
                 t.setStatus(Transaction.STATUS_COMMITTED);
                 MVMap<Long, Object[]> undoLog = undoLogs[transactionId];
-                store.renameMap(undoLog, UNDO_LOG_NAME_PEFIX + UNDO_LOG_COMMITTED + transactionId);
+                store.renameMap(undoLog, getUndoLogName(true, transactionId));
                 try {
                     Cursor<Long, Object[]> cursor = undoLog.cursor(null);
                     while (cursor.hasNext()) {
@@ -432,7 +440,7 @@ public class TransactionStore {
                     }
                     undoLog.clear();
                 } finally {
-                    store.renameMap(undoLog, UNDO_LOG_NAME_PEFIX + UNDO_LOG_OPEN + transactionId);
+                    store.renameMap(undoLog, getUndoLogName(false, transactionId));
                 }
             } finally {
                 flipCommittingTransactionsBit(transactionId, false);

h2/src/test/org/h2/test/store/TestTransactionStore.java

@@ -396,7 +396,8 @@ public class TestTransactionStore extends TestBase {
             store.close();
             s = MVStore.open(fileName);
             // roll back a bit, until we have some undo log entries
-            assertTrue(s.hasMap("undoLog-1") || s.hasMap("undoLog.1"));
+            assertTrue(s.hasMap(TransactionStore.getUndoLogName(false, 1)) ||
+                        s.hasMap(TransactionStore.getUndoLogName(true, 1)));
             for (int back = 0; back < 100; back++) {
                 int minus = r.nextInt(10);
                 s.rollbackTo(Math.max(0, s.getCurrentVersion() - minus));
@@ -424,7 +425,7 @@ public class TestTransactionStore extends TestBase {
 
     private boolean hasDataUndoLog(MVStore s) {
         for (int i = 0; i < 255; i++) {
-            if(s.hasData("undoLog."+i)) {
+            if(s.hasData(TransactionStore.getUndoLogName(true, 1))) {
                 return true;
             }
         }