MVStore: Fix "version to keep"; documentation

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

@@ -49,6 +49,7 @@ TransactionStore:
 MVStore:
 
 - maybe change the length code to have lower gaps
+- data kept in stream store if transaction is not committed?
 
 - automated 'kill process' and 'power failure' test
 - test and possibly improve compact operation (for large dbs)
@@ -1722,8 +1723,8 @@ public class MVStore {
     }
 
     /**
-     * How many versions to retain for in-memory stores. If not set, 5 versions
-     * are retained.
+     * How many versions to retain for in-memory stores. If not set, 5 old
+     * versions are retained.
      * 
      * @param count the number of versions to keep
      */
@@ -1732,7 +1733,7 @@ public class MVStore {
     }
 
     /**
-     * Get the oldest version to retain in memory.
+     * Get the oldest version to retain in memory (for in-memory stores).
      *
      * @return the version
      */
@@ -1749,7 +1750,7 @@ public class MVStore {
     long getOldestVersionToKeep() {
         long v = currentVersion;
         if (fileStore == null) {
-            return v - versionsToKeep;
+            return v - versionsToKeep + 1;
         }
         long storeVersion = currentStoreVersion;
         if (storeVersion > -1) {
@@ -2321,8 +2322,8 @@ public class MVStore {
 
         /**
          * Encrypt / decrypt the file using the given password. This method has
-         * no effect for in-memory stores. The password is passed as a char
-         * array so that it can be cleared as soon as possible. Please note
+         * no effect for in-memory stores. The password is passed as a
+         * char array so that it can be cleared as soon as possible. Please note
          * there is still a small risk that password stays in memory (due to
          * Java garbage collection). Also, the hashed encryption key is kept in
          * memory as long as the file is open.
@@ -2402,7 +2403,10 @@ public class MVStore {
         }
 
         /**
-         * Use the provided file store instead of the default one.
+         * Use the provided file store instead of the default one. Please note
+         * that any kind of store (including an off-heap store) is considered a
+         * "persistence", while an "in-memory store" means objects are not
+         * persisted and fully kept in the JVM heap.
          * 
          * @param store the file store
          * @return this

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

@@ -149,10 +149,12 @@ public class StreamStore {
         }
         boolean eof = len < maxBlockSize;
         if (len < minBlockSize) {
+            // in-place: 0, len (int), data
             id.write(0);
             DataUtils.writeVarInt(id, len);
             id.write(buff);
         } else {
+            // block: 1, len (int), blockId (long)
             id.write(1);
             DataUtils.writeVarInt(id, len);
             DataUtils.writeVarLong(id, writeBlock(buff));
@@ -181,6 +183,7 @@ public class StreamStore {
     private ByteArrayOutputStream putIndirectId(ByteArrayOutputStream id) throws IOException {
         byte[] data = id.toByteArray();
         id = new ByteArrayOutputStream();
+        // indirect: 2, total len (long), blockId (long)
         id.write(2);
         DataUtils.writeVarLong(id, length(data));
         DataUtils.writeVarLong(id, writeBlock(data));
@@ -241,15 +244,18 @@ public class StreamStore {
         while (idBuffer.hasRemaining()) {
             switch (idBuffer.get()) {
             case 0:
+                // in-place: 0, len (int), data
                 int len = DataUtils.readVarInt(idBuffer);
                 idBuffer.position(idBuffer.position() + len);
                 break;
             case 1:
+                // block: 1, len (int), blockId (long)
                 DataUtils.readVarInt(idBuffer);
                 long k = DataUtils.readVarLong(idBuffer);
                 map.remove(k);
                 break;
             case 2:
+                // indirect: 2, total len (long), blockId (long)
                 DataUtils.readVarLong(idBuffer);
                 long k2 = DataUtils.readVarLong(idBuffer);
                 // recurse
@@ -276,15 +282,18 @@ public class StreamStore {
         while (idBuffer.hasRemaining()) {
             switch (idBuffer.get()) {
             case 0:
+                // in-place: 0, len (int), data
                 int len = DataUtils.readVarInt(idBuffer);
                 idBuffer.position(idBuffer.position() + len);
                 length += len;
                 break;
             case 1:
+                // block: 1, len (int), blockId (long)
                 length += DataUtils.readVarInt(idBuffer);
                 DataUtils.readVarLong(idBuffer);
                 break;
             case 2:
+                // indirect: 2, total len (long), blockId (long)
                 length += DataUtils.readVarLong(idBuffer);
                 DataUtils.readVarLong(idBuffer);
                 break;

h2/src/main/org/h2/store/LobStorageMap.java

@@ -46,8 +46,7 @@ public class LobStorageMap implements LobStorageInterface {
      * (which is a long) to the stream store id (which is a byte array).
      *
      * Key: lobId (long)
-     * Value: { streamStoreId (byte[]), tableId (int),
-     * byteCount (long), hashCode (long) }.
+     * Value: { streamStoreId (byte[]), tableId (int), byteCount (long), hash (long) }.
      */
     private MVMap<Long, Object[]> lobMap;
 

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

@@ -27,6 +27,7 @@ import org.h2.mvstore.type.StringDataType;
 import org.h2.store.fs.FilePath;
 import org.h2.store.fs.FileUtils;
 import org.h2.test.TestBase;
+import org.h2.test.utils.AssertThrows;
 
 /**
  * Tests the MVStore.
@@ -55,6 +56,7 @@ public class TestMVStore extends TestBase {
         testCacheInfo();
         testRollback();
         testVersionsToKeep();
+        testVersionsToKeep2();
         testRemoveMap();
         testIsEmpty();
         testOffHeapStorage();
@@ -184,9 +186,9 @@ public class TestMVStore extends TestBase {
             map.put(i, i);
             s.commit();
             if (version >= 6) {
-                map.openVersion(version - 6);
+                map.openVersion(version - 5);
                 try {
-                    map.openVersion(version - 7);
+                    map.openVersion(version - 6);
                     fail();
                 } catch (IllegalArgumentException e) {
                     // expected
@@ -195,6 +197,33 @@ public class TestMVStore extends TestBase {
         }
     }
     
+    private void testVersionsToKeep2() {
+        MVStore s = new MVStore.Builder().autoCommitDisabled().open();
+        s.setVersionsToKeep(2);
+        final MVMap<Integer, String> m = s.openMap("data");
+        s.commit();
+        assertEquals(1, s.getCurrentVersion());
+        m.put(1, "version 1");
+        s.commit();
+        assertEquals(2, s.getCurrentVersion());
+        m.put(1, "version 2");
+        s.commit();
+        assertEquals(3, s.getCurrentVersion());
+        m.put(1, "version 3");
+        s.commit();
+        m.put(1, "version 4");
+        assertEquals("version 4", m.openVersion(4).get(1));
+        assertEquals("version 3", m.openVersion(3).get(1));
+        assertEquals("version 2", m.openVersion(2).get(1));
+        new AssertThrows(IllegalArgumentException.class) {
+            @Override
+            public void test() throws Exception {
+                m.openVersion(1);
+            }
+        };
+        s.close();
+    }
+
     private void testRemoveMap() throws Exception {
         String fileName = getBaseDir() + "/testCloseMap.h3";
         FileUtils.delete(fileName);