MVStore bugfixes

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

@@ -803,10 +803,6 @@ public class MVMap<K, V> extends AbstractMap<K, V>
                 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);
                 }
             }
@@ -1047,7 +1043,7 @@ public class MVMap<K, V> extends AbstractMap<K, V>
             throw DataUtils.newUnsupportedOperationException(
                     "This map is read-only");
         }
-        store.beforeWrite();
+        store.beforeWrite(this);
     }
 
     @Override

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

@@ -43,6 +43,9 @@ TransactionStore:
     if there is only one connection
 
 MVStore:
+- better and clearer memory usage accounting rules
+    (heap memory versus disk memory), so that even there is never an out of memory
+    even for a small heap, and so that chunks are still relatively big on average
 - make sure serialization / deserialization errors don't corrupt the file
 - FileStore: don't open and close when set using MVStore.Builder.fileStore
 - test and possibly improve compact operation (for large dbs)
@@ -270,6 +273,8 @@ public class MVStore {
     private long autoCompactLastFileOpCount;
 
     private Object compactSync = new Object();
+    
+    private IllegalStateException panicException;
 
     /**
      * Create and open the store.
@@ -326,7 +331,7 @@ public class MVStore {
                     segmentCount, stackMoveDistance);
         }
         o = config.get("autoCommitBufferSize");
-        int kb = o == null ? 512 : (Integer) o;
+        int kb = o == null ? 1024 : (Integer) o;
         // 19 KB memory is about 1 KB storage
         autoCommitMemory = kb * 1024 * 19;
 
@@ -366,11 +371,11 @@ public class MVStore {
     }
 
     private void panic(IllegalStateException e) {
-        try {
-            closeStore(false);
-        } catch (Exception e2) {
-            // ignore
+        if (backgroundExceptionHandler != null) {
+            backgroundExceptionHandler.uncaughtException(null, e);
         }
+        panicException = e;
+        closeImmediately();
         throw e;
     }
 
@@ -772,7 +777,7 @@ public class MVStore {
         try {
             fileStore.writeFully(pos, buffer);
         } catch (IllegalStateException e) {
-            closeImmediately();
+            panic(e);
             throw e;
         }
     }
@@ -792,7 +797,7 @@ public class MVStore {
         }
         closeStore(true);
     }
-
+    
     /**
      * Close the file and the store, without writing anything. This will stop
      * the background thread. This method ignores all errors.
@@ -1256,7 +1261,8 @@ public class MVStore {
                             DataUtils.ERROR_FILE_CORRUPT,
                             "Negative position {0}", filePos);
                 }
-                r = PageChildren.read(fileStore, filePos, mapId, pos);
+                long maxPos = (c.block + c.len) * BLOCK_SIZE;
+                r = PageChildren.read(fileStore, pos, mapId, filePos, maxPos);
             } else {
                 r = new PageChildren(p);
             }
@@ -1803,7 +1809,8 @@ public class MVStore {
                         DataUtils.ERROR_FILE_CORRUPT,
                         "Negative position {0}", filePos);
             }
-            p = Page.read(fileStore, map, pos, filePos, fileStore.size());
+            long maxPos = (c.block + c.len) * BLOCK_SIZE;
+            p = Page.read(fileStore, pos, map, filePos, maxPos);
             cachePage(pos, p, p.getMemory());
         }
         return p;
@@ -2042,9 +2049,18 @@ public class MVStore {
 
     /**
      * This method is called before writing to a map.
+     *
+     * @param map the map
      */
-    void beforeWrite() {
+    void beforeWrite(MVMap<?, ?> map) {
         if (saveNeeded) {
+            if (map == meta) {
+                // to, don't save while the metadata map is locked
+                // this is to avoid deadlocks that could occur when we
+                // synchronize on the store and then on the metadata map
+                // TODO there should be no deadlocks possible
+                return;
+            }
             saveNeeded = false;
             // check again, because it could have been written by now
             if (unsavedMemory > autoCommitMemory && autoCommitMemory > 0) {
@@ -2242,7 +2258,7 @@ public class MVStore {
     private void checkOpen() {
         if (closed) {
             throw DataUtils.newIllegalStateException(DataUtils.ERROR_CLOSED,
-                    "This store is closed");
+                    "This store is closed", panicException);
         }
     }
 
@@ -2298,6 +2314,7 @@ public class MVStore {
      * @return the name, or null if not found
      */
     public synchronized String getMapName(int id) {
+        checkOpen();
         String m = meta.get(MVMap.getMapKey(id));
         return m == null ? null : DataUtils.parseMap(m).get("name");
     }
@@ -2571,7 +2588,7 @@ public class MVStore {
          * stores). Unless auto-commit is disabled, changes are automatically
          * saved if there are more than this amount of changes.
          * <p>
-         * The default is 512 KB.
+         * The default is 1024 KB.
          * <p>
          * When the value is set to 0 or lower, data is not automatically
          * stored.

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

@@ -163,14 +163,14 @@ public class Page {
      * Read a page.
      *
      * @param fileStore the file store
+     * @param pos the position
      * @param map the map
-     * @param pos the page position
      * @param filePos the position in the file
-     * @param fileSize the file size (to avoid reading past EOF)
+     * @param maxPos the maximum position (the end of the chunk)
      * @return the page
      */
-    static Page read(FileStore fileStore, MVMap<?, ?> map,
-            long pos, long filePos, long fileSize) {
+    static Page read(FileStore fileStore, long pos, MVMap<?, ?> map,
+            long filePos, long maxPos) {
         ByteBuffer buff;
         int maxLength = DataUtils.getPageMaxLength(pos);
         if (maxLength == DataUtils.PAGE_LARGE) {
@@ -178,13 +178,13 @@ public class Page {
             maxLength = buff.getInt();
             // read the first bytes again
         }
-        maxLength = (int) Math.min(fileSize - filePos, maxLength);
+        maxLength = (int) Math.min(maxPos - filePos, maxLength);
         int length = maxLength;
         if (length < 0) {
             throw DataUtils.newIllegalStateException(
                     DataUtils.ERROR_FILE_CORRUPT,
-                    "Illegal page length {0} reading at {1}; file size {2} ",
-                    length, filePos, fileSize);
+                    "Illegal page length {0} reading at {1}; max pos {2} ",
+                    length, filePos, maxPos);
         }
         buff = fileStore.readFully(filePos, length);
         Page p = new Page(map, 0);
@@ -1003,12 +1003,14 @@ public class Page {
          * values.
          *
          * @param fileStore the file store
-         * @param filePos the position in the file
-         * @param mapId the map id
          * @param pos the position
+         * @param mapId the map id
+         * @param filePos the position in the file
+         * @param maxPos the maximum position (the end of the chunk)
          * @return the page children object
          */
-        static PageChildren read(FileStore fileStore, long filePos, int mapId, long pos) {
+        static PageChildren read(FileStore fileStore, long pos, int mapId,
+                long filePos, long maxPos) {
             ByteBuffer buff;
             int maxLength = DataUtils.getPageMaxLength(pos);
             if (maxLength == DataUtils.PAGE_LARGE) {
@@ -1016,14 +1018,13 @@ public class Page {
                 maxLength = buff.getInt();
                 // read the first bytes again
             }
-            long fileSize = fileStore.fileSize;
-            maxLength = (int) Math.min(fileSize - filePos, maxLength);
+            maxLength = (int) Math.min(maxPos - filePos, maxLength);
             int length = maxLength;
             if (length < 0) {
                 throw DataUtils.newIllegalStateException(
                         DataUtils.ERROR_FILE_CORRUPT,
-                        "Illegal page length {0} reading at {1}; file size {2} ",
-                        length, filePos, fileSize);
+                        "Illegal page length {0} reading at {1}; max pos {2} ",
+                        length, filePos, maxPos);
             }
             buff = fileStore.readFully(filePos, length);
             int chunkId = DataUtils.getPageChunkId(pos);

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

@@ -54,7 +54,7 @@ public class TransactionStore {
      */
     private HashMap<Integer, MVMap<Object, VersionedValue>> maps =
             New.hashMap();
-    
+
     private final DataType dataType;
 
     private boolean init;
@@ -102,7 +102,7 @@ public class TransactionStore {
                     "Undo map open with a different value type");
         }
     }
-    
+
     /**
      * Initialize the store. This is needed before a transaction can be opened.
      * If the transaction store is corrupt, this method can throw an exception,
@@ -222,7 +222,7 @@ public class TransactionStore {
     public synchronized Transaction begin() {
         if (!init) {
             throw DataUtils.newIllegalStateException(
-                    DataUtils.ERROR_TRANSACTION_ILLEGAL_STATE, 
+                    DataUtils.ERROR_TRANSACTION_ILLEGAL_STATE,
                     "Not initialized");
         }
         int transactionId = ++lastTransactionId;

h2/src/main/org/h2/mvstore/type/ObjectDataType.java

@@ -1267,7 +1267,9 @@ public class ObjectDataType implements DataType {
                     }
                 }
             }
-            return size;
+            // we say they are larger, because these objects
+            // use quite a lot of disk space
+            return size * 2;
         }
 
         @Override
@@ -1527,7 +1529,9 @@ public class ObjectDataType implements DataType {
                 return;
             }
             byte[] data = serialize(obj);
-            int size = data.length;
+            // we say they are larger, because these objects
+            // use quite a lot of disk space
+            int size = data.length * 2;
             // adjust the average size
             // using an exponential moving average
             averageSize = (size + 15 * averageSize) / 16;