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

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

@@ -826,10 +826,10 @@ public class MVMap<K, V> extends AbstractMap<K, V>
         int writtenPageCount = 0;
         for (int i = 0; i < p.getChildPageCount(); i++) {
             long pos = p.getChildPagePos(i);
-            if (pos == 0) {
-                continue;
-            }
-            if (DataUtils.getPageType(pos) == DataUtils.PAGE_TYPE_LEAF) {
+            if (pos != 0 && DataUtils.getPageType(pos) == 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);
                 if (!set.contains(chunkId)) {
                     continue;

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

@@ -134,6 +134,12 @@ public class MVStore {
     private static final int FORMAT_WRITE = 1;
     private static final int FORMAT_READ = 1;
     
+    /**
+     * Used to mark a chunk as free, when it was detected that live bookkeeping
+     * is incorrect.
+     */
+    private static final int MARKED_FREE = 10000000;
+
     /**
      * The background thread, if any.
      */
@@ -1238,17 +1244,18 @@ public class MVStore {
                     // are not concurrently modified
                     c.maxLenLive += f.maxLenLive;
                     c.pageCountLive += f.pageCountLive;
-                    if (c.pageCountLive < 0 && c.pageCountLive > -Integer.MAX_VALUE / 2) {
+                    if (c.pageCountLive < 0 && c.pageCountLive > -MARKED_FREE) {
                         throw DataUtils.newIllegalStateException(
                                 DataUtils.ERROR_INTERNAL,
                                 "Corrupt page count {0}", c.pageCountLive);
                     }
-                    if (c.maxLenLive < 0 && c.maxLenLive > -Long.MAX_VALUE / 2) {
+                    if (c.maxLenLive < 0 && c.maxLenLive > -MARKED_FREE) {
                         throw DataUtils.newIllegalStateException(
                                 DataUtils.ERROR_INTERNAL,
                                 "Corrupt max length {0}", c.maxLenLive);
                     }
-                    if (c.pageCount == 0 && c.maxLenLive > 0) {
+                    if (c.pageCountLive <= 0 && c.maxLenLive > 0 ||
+                            c.maxLenLive <= 0 && c.pageCountLive > 0) {
                         throw DataUtils.newIllegalStateException(
                                 DataUtils.ERROR_INTERNAL,
                                 "Corrupt max length {0}", c.maxLenLive);
@@ -1706,11 +1713,11 @@ public class MVStore {
         long start = chunk.block * BLOCK_SIZE;
         int length = chunk.len * BLOCK_SIZE;
         ByteBuffer buff = fileStore.readFully(start, length);
-        Chunk c = Chunk.readChunkHeader(buff, start);
-        if (c.id != chunk.id) {
+        Chunk verify = Chunk.readChunkHeader(buff, start);
+        if (verify.id != chunk.id) {
             throw DataUtils.newIllegalStateException(
                     DataUtils.ERROR_FILE_CORRUPT,
-                    "Expected chunk {0}, got {1}", chunk.id, c.id);
+                    "Expected chunk {0}, got {1}", chunk.id, verify.id);
         }
         int pagesRemaining = chunk.pageCount;
         markMetaChanged();
@@ -1754,11 +1761,14 @@ public class MVStore {
                 }
             }
             if (!pendingChanges) {
+ ;                
+new Exception(fileStore.getFileName() + " chunk " + chunk.id + " fix live! " + chunk).printStackTrace(System.out);
+
                 // bookkeeping is broken for this chunk:
                 // fix it
                 registerFreePage(currentVersion, chunk.id,
-                        c.maxLenLive + Long.MAX_VALUE / 2, 
-                        c.pageCountLive + Integer.MAX_VALUE / 2);
+                        chunk.maxLenLive + MARKED_FREE, 
+                        chunk.pageCountLive + MARKED_FREE);
             }
         }
     }
@@ -1809,9 +1819,10 @@ public class MVStore {
         // we need to keep temporary pages,
         // to support reading old versions and rollback
         if (pos == 0) {
-            // the value could be smaller than 0 because
-            // in some cases a page is allocated,
-            // but never stored, so we need to use max
+            // the page was not yet stored:
+            // just using "unsavedMemory -= memory" could result in negative
+            // values, because in some cases a page is allocated, but never
+            // stored, so we need to use max
             unsavedMemory = Math.max(0, unsavedMemory - memory);
             return;
         }

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

@@ -103,6 +103,14 @@ public class Page {
      */
     private Page[] childrenPages;
     
+    /**
+     * Whether the page is an in-memory (not stored, or not yet stored) page,
+     * and it is removed. This is to keep track of pages that concurrently
+     * changed while they are being stored, in which case the live bookkeeping
+     * needs to be aware of such cases.
+     */
+    private volatile boolean removedInMemory;
+
     Page(MVMap<?, ?> map, long version) {
         this.map = map;
         this.version = version;
@@ -940,9 +948,13 @@ public class Page {
         store.cachePage(pos, this, getMemory());
         long max = DataUtils.getPageMaxLength(pos);
         chunk.maxLen += max;
-        chunk.maxLenLive += max;
         chunk.pageCount++;
+        if (!removedInMemory) {
+            // if the page was removed _before_ the position was assigned, we
+            // must not increase the live fields.
+            chunk.maxLenLive += max;
             chunk.pageCountLive++;
+        }
         return typePos + 1;
     }
 
@@ -1070,11 +1082,11 @@ public class Page {
      * Remove the page.
      */
     public void removePage() {
-        map.removePage(pos, memory);
+        long p = pos;
+        if (p == 0) {
+            removedInMemory = true;
         }
-
-    public void setPos(long pos) {
-        this.pos = pos;
+        map.removePage(p, memory);
     }
 
 }

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

@@ -78,12 +78,13 @@ public class TestConcurrent extends TestMVStore {
                 }
             };
             final MVMap<Integer, Integer> dataMap = s.openMap("data");
+            final AtomicInteger counter = new AtomicInteger();
             Task task2 = new Task() {
                 @Override
                 public void call() throws Exception {
-                    int i = 0;
                     while (!stop) {
-                        dataMap.put(i++, i);
+                        int i = counter.getAndIncrement();
+                        dataMap.put(i, i * 10);
                     }
                 }
             };
@@ -97,6 +98,9 @@ public class TestConcurrent extends TestMVStore {
             }
             task.get();
             task2.get();
+            for (int i = 0; i < counter.get(); i++) {
+                assertEquals(10 * i, dataMap.get(i).intValue());
+            }
         } finally {
             s.close();        
         }