MVStore: bugfixes

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

@@ -84,14 +84,13 @@ public class MVMap<K, V> extends AbstractMap<K, V>
      *
      * @param p the page
      * @param writeVersion the write version
-     * @param removeOld whether the old page should be marked as deleted
      * @return a page with the given write version
      */
-    protected Page copyOnWrite(Page p, long writeVersion, boolean removeOld) {
+    protected Page copyOnWrite(Page p, long writeVersion) {
         if (p.getVersion() == writeVersion) {
             return p;
         }
-        return p.copy(writeVersion, removeOld);
+        return p.copy(writeVersion);
     }
 
     /**
@@ -108,7 +107,7 @@ public class MVMap<K, V> extends AbstractMap<K, V>
         beforeWrite();
         try {
             long v = writeVersion;
-            Page p = copyOnWrite(root, v, true);
+            Page p = copyOnWrite(root, v);
             p = splitRootIfNeeded(p, v);
             Object result = put(p, v, key, value);
             newRoot(p);
@@ -167,7 +166,7 @@ public class MVMap<K, V> extends AbstractMap<K, V>
         } else {
             index++;
         }
-        Page c = copyOnWrite(p.getChildPage(index), writeVersion, true);
+        Page c = copyOnWrite(p.getChildPage(index), writeVersion);
         if (c.getMemory() > store.getPageSplitSize() && c.getKeyCount() > 1) {
             // split on the way down
             int at = c.getKeyCount() / 2;
@@ -552,7 +551,7 @@ public class MVMap<K, V> extends AbstractMap<K, V>
         beforeWrite();
         try {
             long v = writeVersion;
-            Page p = copyOnWrite(root, v, true);
+            Page p = copyOnWrite(root, v);
             @SuppressWarnings("unchecked")
             V result = (V) remove(p, v, key);
             newRoot(p);
@@ -671,13 +670,14 @@ public class MVMap<K, V> extends AbstractMap<K, V>
             index++;
         }
         Page cOld = p.getChildPage(index);
-        Page c = copyOnWrite(cOld, writeVersion, false);
+        Page c = copyOnWrite(cOld, writeVersion);
         result = remove(c, writeVersion, key);
-        if (result == null) {
-            return null;
-        }
-        cOld.removePage();
-        if (c.getTotalCount() == 0) {
+        if (result == null || c.getTotalCount() != 0) {
+            // no change, or
+            // there are more nodes
+            p.setChild(index, c);
+            p.setCounts(index, c);
+        } else {
             // this child was deleted
             if (p.getKeyCount() == 0) {
                 p.setChild(index, c);
@@ -686,9 +686,6 @@ public class MVMap<K, V> extends AbstractMap<K, V>
             } else {
                 p.remove(index);
             }
-        } else {
-            p.setChild(index, c);
-            p.setCounts(index, c);
         }
         return result;
     }
@@ -733,7 +730,7 @@ public class MVMap<K, V> extends AbstractMap<K, V>
      *
      * @return the key type
      */
-    protected DataType getKeyType() {
+    public DataType getKeyType() {
         return keyType;
     }
 
@@ -742,7 +739,7 @@ public class MVMap<K, V> extends AbstractMap<K, V>
      *
      * @return the value type
      */
-    protected DataType getValueType() {
+    public DataType getValueType() {
         return valueType;
     }
 
@@ -1130,7 +1127,7 @@ public class MVMap<K, V> extends AbstractMap<K, V>
      * @param name the map name (or null)
      * @return the string
      */
-    public String asString(String name) {
+    String asString(String name) {
         StringBuilder buff = new StringBuilder();
         if (name != null) {
             DataUtils.appendMap(buff, "name", name);

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

@@ -29,8 +29,8 @@ public class MVMapConcurrent<K, V> extends MVMap<K, V> {
     }
 
     @Override
-    protected Page copyOnWrite(Page p, long writeVersion, boolean removeOld) {
-        return p.copy(writeVersion, removeOld);
+    protected Page copyOnWrite(Page p, long writeVersion) {
+        return p.copy(writeVersion);
     }
 
     @Override
@@ -48,7 +48,7 @@ public class MVMapConcurrent<K, V> extends MVMap<K, V> {
             get(key);
             long v = writeVersion;
             synchronized (this) {
-                Page p = copyOnWrite(root, v, true);
+                Page p = copyOnWrite(root, v);
                 p = splitRootIfNeeded(p, v);
                 V result = (V) put(p, v, key, value);
                 newRoot(p);
@@ -75,7 +75,7 @@ public class MVMapConcurrent<K, V> extends MVMap<K, V> {
             }
             long v = writeVersion;
             synchronized (this) {
-                Page p = copyOnWrite(root, v, true);
+                Page p = copyOnWrite(root, v);
                 result = (V) remove(p, v, key);
                 newRoot(p);
             }

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

@@ -484,6 +484,16 @@ public class MVStore {
         meta.remove("root." + id);
         maps.remove(id);
     }
+    
+    /**
+     * Check whether a given map exists.
+     * 
+     * @param name the map name
+     * @return true if it exists
+     */
+    public boolean hasMap(String name) {
+        return meta.containsKey("name." + name);
+    }
 
     /**
      * Mark a map as changed (containing unsaved changes).

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

@@ -267,19 +267,16 @@ public class Page {
      * Create a copy of this page.
      *
      * @param version the new version
-     * @param removeOld whether the old page should be marked as deleted
      * @return a page with the given version
      */
-    public Page copy(long version, boolean removeOld) {
-        if (removeOld) {
-            int todoRemoveAtCaller;
-            removePage();
-        }
+    public Page copy(long version) {
         Page newPage = create(map, version,
                 keyCount, keys, values, children, childrenPages,
                 counts, totalCount,
                 SHARED_KEYS | SHARED_VALUES | SHARED_CHILDREN | SHARED_COUNTS,
                 memory);
+        // mark the old as deleted
+        removePage();
         newPage.cachedCompare = cachedCompare;
         return newPage;
     }

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

@@ -5,10 +5,13 @@
  */
 package org.h2.test.store;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Random;
 import java.util.TreeMap;
 
 import org.h2.mvstore.MVMap;
+import org.h2.mvstore.MVMapConcurrent;
 import org.h2.mvstore.MVStore;
 import org.h2.store.fs.FileUtils;
 import org.h2.test.TestBase;
@@ -19,6 +22,7 @@ import org.h2.test.TestBase;
 public class TestRandomMapOps extends TestBase {
 
     private String fileName;
+    private boolean concurrent;
     private int seed;
     private int op;
 
@@ -33,17 +37,18 @@ public class TestRandomMapOps extends TestBase {
 
     @Override
     public void test() throws Exception {
-        test("memFS:randomOps.h3");
-        int todoTestConcurrentMap;
-        int todoTestMVRTreeMap;
+        concurrent = false;
+        testMap("memFS:randomOps.h3");
+        concurrent = true;
+        testMap("memFS:randomOps.h3");
     }
 
-    public void test(String fileName) {
+    public void testMap(String fileName) {
         this.fileName = fileName;
         int best = Integer.MAX_VALUE;
         int bestSeed = 0;
         Throwable failException = null;
-        for (seed = 0; seed < 1000; seed++) {
+        for (seed = 0; seed < 100; seed++) {
             FileUtils.delete(fileName);
             Throwable ex = null;
             try {
@@ -70,20 +75,23 @@ public class TestRandomMapOps extends TestBase {
     private void testCase() throws Exception {
         FileUtils.delete(fileName);
         MVStore s;
-        MVMap<Integer, byte[]> m;
-
         s = new MVStore.Builder().fileName(fileName).
                 pageSplitSize(50).writeDelay(0).open();
-        m = s.openMap("data");
+        MVMap<Integer, byte[]> m;
+        if (concurrent) {
+            m = s.openMap("data", new MVMapConcurrent.Builder<Integer, byte[]>());
+        } else {
+            m = s.openMap("data");
+        }
         
         Random r = new Random(seed);
         op = 0;
-        int size = getSize(10, 100);
+        int size = getSize(100, 1000);
         TreeMap<Integer, byte[]> map = new TreeMap<Integer, byte[]>();
         for (; op < size; op++) {
             int k = r.nextInt(100);
             byte[] v = new byte[r.nextInt(10) * 10];
-            int type = r.nextInt(11);
+            int type = r.nextInt(13);
             switch (type) {
             case 0:
             case 1:
@@ -105,7 +113,7 @@ public class TestRandomMapOps extends TestBase {
                 break;
             case 7:
                 log(op, k, v, "compact");
-                s.compact(80);
+                s.compact(90);
                 break;
             case 8:
                 log(op, k, v, "clear");
@@ -124,6 +132,22 @@ public class TestRandomMapOps extends TestBase {
                         pageSplitSize(50).writeDelay(0).open();
                 m = s.openMap("data");
                 break;
+            case 11:
+                log(op, k, v, "compactMoveChunks");
+                s.commit();
+                s.compactMoveChunks();
+                break;
+            case 12:
+                log(op, k, v, "getKeyIndex");
+                ArrayList<Integer> keyList = new ArrayList<Integer>(map.keySet());
+                int index = Collections.binarySearch(keyList, k, null);
+                int index2 = (int) m.getKeyIndex(k);
+                assertEquals(index, index2);
+                if (index >= 0) {
+                    int k2 = m.getKey(index);
+                    assertEquals(k2, k);
+                }
+                break;
             }
             assertEqualsMapValues(map.get(k), m.get(k));
             assertEquals(map.ceilingKey(k), m.ceilingKey(k));
@@ -131,8 +155,10 @@ public class TestRandomMapOps extends TestBase {
             assertEquals(map.higherKey(k), m.higherKey(k));
             assertEquals(map.lowerKey(k), m.lowerKey(k));
             assertEquals(map.isEmpty(), m.isEmpty());
-            if (map.size() != m.size()) {
-                assertEquals(map.size(), m.size());
+            assertEquals(map.size(), m.size());
+            if (!map.isEmpty()) {
+                assertEquals(map.firstKey(), m.firstKey());
+                assertEquals(map.lastKey(), m.lastKey());
             }
         }
         s.store();
@@ -149,6 +175,14 @@ public class TestRandomMapOps extends TestBase {
         }
     }
     
+    /**
+     * Log the operation
+     * 
+     * @param op the operation id
+     * @param k the key
+     * @param v the value
+     * @param msg the message
+     */
     private static void log(int op, int k, byte[] v, String msg) {
          // System.out.println(op + ": " + msg + " key: " + k + " value: " + v);
     }