MVStore: deal with the case that data was moved to a new chunk (work in progress)

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

@@ -792,45 +792,56 @@ public class MVMap<K, V> extends AbstractMap<K, V>
      * Re-write any pages that belong to one of the chunks in the given set.
      *
      * @param set the set of chunk ids
+     * @return whether rewriting was successful
      */
-    void rewrite(Set<Integer> set) {
+    boolean rewrite(Set<Integer> set) {
+        if (root.getVersion() < createVersion) {
+            // a new map
+            return true;
+        }
+        // read from old version, to avoid concurrent reads
+        MVMap<K, V> readMap = openVersion(root.getVersion() - 1);
         try {
-            rewrite(root, set);
+            rewrite(readMap.root, set);
+            return true;
         } catch (IllegalStateException e) {
             if (DataUtils.getErrorCode(e.getMessage()) == DataUtils.ERROR_CHUNK_NOT_FOUND) {
                 // ignore
-            } else {
-                throw e;
+                return false;
             }
+            throw e;
         }
     }
 
     private int rewrite(Page p, Set<Integer> set) {
         if (p.isLeaf()) {
             long pos = p.getPos();
-            if (pos == 0) {
-                return 0;
-            }
             int chunkId = DataUtils.getPageChunkId(pos);
             if (!set.contains(chunkId)) {
                 return 0;
             }
-            @SuppressWarnings("unchecked")
-            K key = (K) p.getKey(0);
-            V value = get(key);
-            if (value != null) {
-                replace(key, value, value);
+            if (p.getKeyCount() > 0) {
+                @SuppressWarnings("unchecked")
+                K key = (K) p.getKey(0);
+                V value = get(key);
+                if (value != null) {
+                    // this is to avoid storing while replacing, to avoid a
+                    // deadlock when rewriting the meta map
+                    // TODO there should be no deadlocks possible
+                    store.beforeWrite();
+                    replace(key, value, value);
+                }
             }
             return 1;
         }
         int writtenPageCount = 0;
         for (int i = 0; i < p.getChildPageCount(); i++) {
-            long pos = p.getChildPagePos(i);
-            if (pos != 0 && DataUtils.getPageType(pos) == DataUtils.PAGE_TYPE_LEAF) {
+            long childPos = p.getChildPagePos(i);
+            if (childPos != 0 && DataUtils.getPageType(childPos) == DataUtils.PAGE_TYPE_LEAF) {
                 // we would need to load the page, and it's a leaf:
                 // only do that if it's within the set of chunks we are
                 // interested in
-                int chunkId = DataUtils.getPageChunkId(pos);
+                int chunkId = DataUtils.getPageChunkId(childPos);
                 if (!set.contains(chunkId)) {
                     continue;
                 }
@@ -839,27 +850,25 @@ public class MVMap<K, V> extends AbstractMap<K, V>
         }
         if (writtenPageCount == 0) {
             long pos = p.getPos();
-            if (pos != 0) {
-                int chunkId = DataUtils.getPageChunkId(pos);
-                if (set.contains(chunkId)) {
-                    // an inner node page that is in one of the chunks,
-                    // but only points to chunks that are not in the set:
-                    // if no child was changed, we need to do that now
-                    // (this is not needed if anyway one of the children
-                    // was changed, as this would have updated this
-                    // page as well)
-                    Page p2 = p;
-                    while (!p2.isLeaf()) {
-                        p2 = p2.getChildPage(0);
-                    }
-                    @SuppressWarnings("unchecked")
-                    K key = (K) p2.getKey(0);
-                    V value = get(key);
-                    if (value != null) {
-                        replace(key, value, value);
-                    }
-                    writtenPageCount++;
+            int chunkId = DataUtils.getPageChunkId(pos);
+            if (set.contains(chunkId)) {
+                // an inner node page that is in one of the chunks,
+                // but only points to chunks that are not in the set:
+                // if no child was changed, we need to do that now
+                // (this is not needed if anyway one of the children
+                // was changed, as this would have updated this
+                // page as well)
+                Page p2 = p;
+                while (!p2.isLeaf()) {
+                    p2 = p2.getChildPage(0);
+                }
+                @SuppressWarnings("unchecked")
+                K key = (K) p2.getKey(0);
+                V value = get(key);
+                if (value != null) {
+                    replace(key, value, value);
                 }
+                writtenPageCount++;
             }
         }
         return writtenPageCount;

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

@@ -57,7 +57,7 @@ public class MVMapConcurrent<K, V> extends MVMap<K, V> {
             afterWrite();
         }
     }
-
+    
     @Override
     protected void waitUntilWritten(long version) {
         // no need to wait

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

@@ -1683,9 +1683,13 @@ public class MVStore {
         for (MVMap<?, ?> m : maps.values()) {
             @SuppressWarnings("unchecked")
             MVMap<Object, Object> map = (MVMap<Object, Object>) m;
-            map.rewrite(set);
+            if (!map.rewrite(set)) {
+                return;
+            }
+        }
+        if (!meta.rewrite(set)) {
+            return;
         }
-        meta.rewrite(set);
         commitAndSave();
         boolean commitAgain = false;
         for (Chunk c : old) {

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

@@ -64,8 +64,9 @@ public class TestConcurrent extends TestMVStore {
     private void testConcurrentAutoCommitAndChange() throws InterruptedException {
         String fileName = "memFS:testConcurrentChangeAndBackgroundCompact";
         FileUtils.delete(fileName);
-        final MVStore s = new MVStore.Builder().fileName(
-                fileName).open();
+        final MVStore s = new MVStore.Builder().
+                fileName(fileName).pageSplitSize(100).
+                open();
         try {
             s.setRetentionTime(1000);
             s.setAutoCommitDelay(1);
@@ -78,6 +79,8 @@ public class TestConcurrent extends TestMVStore {
                 }
             };
             final MVMap<Integer, Integer> dataMap = s.openMap("data");
+            final MVMap<Integer, Integer> dataSmallMap = s.openMap("dataSmall");
+            s.openMap("emptyMap");
             final AtomicInteger counter = new AtomicInteger();
             Task task2 = new Task() {
                 @Override
@@ -85,6 +88,10 @@ public class TestConcurrent extends TestMVStore {
                     while (!stop) {
                         int i = counter.getAndIncrement();
                         dataMap.put(i, i * 10);
+                        dataSmallMap.put(i % 100, i * 10);
+                        if (i % 100 == 0) {
+                            dataSmallMap.clear();
+                        }
                     }
                 }
             };