MVStore: some database file could not be compacted.

h2/src/docsrc/html/changelog.html

@@ -19,6 +19,9 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul><li>Improve error message when the user specifies an unsupported combination of database settings.
+</li><li>MVStore: some database file could not be compacted due to a bug in 
+    the bookkeeping of the fill rate. Also, database file were compacted quite slowly.
+    This has been improved; but more changes in this area are expected.
 </li><li>MVStore: support for volatile maps (that don't store changes).
 </li><li>MVStore mode: in-memory databases now also use the MVStore.
 </li><li>In server mode, appending ";autocommit=false" to the database URL was working,

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

@@ -126,6 +126,9 @@ public class Chunk {
             }
         } catch (Exception e) {
             // there could be various reasons
+            throw DataUtils.newIllegalStateException(
+                    DataUtils.ERROR_FILE_CORRUPT,
+                    "File corrupt reading chunk at position {0}", start, e);
         }
         throw DataUtils.newIllegalStateException(
                 DataUtils.ERROR_FILE_CORRUPT,

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

@@ -871,7 +871,7 @@ public class DataUtils {
      * @return the parsed value
      * @throws IllegalStateException if parsing fails
      */
-    public static long readHexLong(HashMap<String, ? extends Object> map,
+    public static long readHexLong(Map<String, ? extends Object> map,
             String key, long defaultValue) {
         Object v = map.get(key);
         if (v == null) {

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

@@ -459,13 +459,31 @@ public class MVMap<K, V> extends AbstractMap<K, V>
     }
 
     /**
-     * Get the page for the given value.
+     * Get a key that is referenced in the given page or a child page.
      *
-     * @param key the key
-     * @return the value, or null if not found
+     * @param p the page
+     * @return the key, or null if not found
      */
-    protected Page getPage(K key) {
-        return binarySearchPage(root, key);
+    protected K getLiveKey(Page p) {
+        while (!p.isLeaf()) {
+            p = p.getLiveChildPage(0);
+            if (p == null) {
+                return null;
+            }
+        }
+        @SuppressWarnings("unchecked")
+        K key = (K) p.getKey(0);
+        Page p2 = binarySearchPage(root, key);
+        if (p2 == null) {
+            return null;
+        }
+        if (p2.getPos() == 0) {
+            return p2 == p ? key : null;
+        }
+        if (p2.getPos() == p.getPos()) {
+            return key;
+        }
+        return null;
     }
 
     /**

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

@@ -798,6 +798,18 @@ public class MVStore {
         }
     }
 
+    /**
+     * Whether the chunk at the given position is live.
+     *
+     * @param pos the position of the page
+     * @return true if it is live
+     */
+    boolean isChunkLive(long pos) {
+        int chunkId = DataUtils.getPageChunkId(pos);
+        String s = meta.get(Chunk.getMetaKey(chunkId));
+        return s != null;
+    }
+
     /**
      * Get the chunk for the given position.
      *
@@ -805,6 +817,17 @@ public class MVStore {
      * @return the chunk
      */
     private Chunk getChunk(long pos) {
+        Chunk c = getChunkIfFound(pos);
+        if (c == null) {
+            int chunkId = DataUtils.getPageChunkId(pos);
+            throw DataUtils.newIllegalStateException(
+                    DataUtils.ERROR_FILE_CORRUPT,
+                    "Chunk {0} not found", chunkId);
+        }
+        return c;
+    }
+
+    private Chunk getChunkIfFound(long pos) {
         int chunkId = DataUtils.getPageChunkId(pos);
         Chunk c = chunks.get(chunkId);
         if (c == null) {
@@ -815,9 +838,7 @@ public class MVStore {
             }
             String s = meta.get(Chunk.getMetaKey(chunkId));
             if (s == null) {
-                throw DataUtils.newIllegalStateException(
-                        DataUtils.ERROR_FILE_CORRUPT,
-                        "Chunk {0} not found", chunkId);
+                return null;
             }
             c = Chunk.fromString(s);
             if (c.block == Long.MAX_VALUE) {
@@ -1450,7 +1471,8 @@ public class MVStore {
      * before calling this method.
      *
      * @param targetFillRate the minimum percentage of live entries
-     * @param minSaving the minimum amount of saved space
+     * @param minSaving the amount of saved space,
+     *      which is also the size of the new chunk
      * @return if a chunk was re-written
      */
     public synchronized boolean compact(int targetFillRate, int minSaving) {
@@ -1497,22 +1519,28 @@ public class MVStore {
         Collections.sort(old, new Comparator<Chunk>() {
             @Override
             public int compare(Chunk o1, Chunk o2) {
-                return new Integer(o1.collectPriority)
-                        .compareTo(o2.collectPriority);
+                int comp = new Integer(o1.collectPriority).
+                        compareTo(o2.collectPriority);
+                if (comp == 0) {
+                    comp = new Long(o1.maxLenLive).
+                        compareTo(o2.maxLenLive);
+                }
+                return comp;
             }
         });
-
         // find out up to were in the old list we need to move
         long saved = 0;
+        long totalSize = 0;
         Chunk move = null;
         for (Chunk c : old) {
-            long save = c.maxLen - c.maxLenLive;
+            long size = c.maxLen - c.maxLenLive;
+            totalSize += c.maxLenLive;
             if (move != null) {
-                if (saved > minSaving) {
+                if (saved > minSaving && totalSize > minSaving) {
                     break;
                 }
             }
-            saved += save;
+            saved += size;
             move = c;
         }
         if (saved < minSaving) {
@@ -1532,20 +1560,32 @@ public class MVStore {
 
         // iterate over all the pages in the old pages
         for (Chunk c : old) {
-            copyLive(c, old);
+            copyLive(c);
         }
 
         commitAndSave();
         return true;
     }
 
-    private void copyLive(Chunk chunk, ArrayList<Chunk> old) {
+    private void copyLive(Chunk chunk) {
+        if (chunk.pageCountLive == 0) {
+            // remove this chunk in the next save operation
+            registerFreePage(currentVersion, chunk.id, 0, 0);
+            return;
+        }
         long start = chunk.block * BLOCK_SIZE;
         int length = chunk.len * BLOCK_SIZE;
         ByteBuffer buff = fileStore.readFully(start, length);
-        Chunk.readChunkHeader(buff, start);
+        Chunk c = Chunk.readChunkHeader(buff, start);
+        if (c.id != chunk.id) {
+            throw DataUtils.newIllegalStateException(
+                    DataUtils.ERROR_FILE_CORRUPT,
+                    "Expected chunk {0}, got {1}", chunk.id, c.id);
+        }
         int pagesRemaining = chunk.pageCount;
         markMetaChanged();
+        boolean mapNotOpen = false;
+        int changeCount = 0;
         while (pagesRemaining-- > 0) {
             int offset = buff.position();
             int pageLength = buff.getInt();
@@ -1559,31 +1599,50 @@ public class MVStore {
             @SuppressWarnings("unchecked")
             MVMap<Object, Object> map = (MVMap<Object, Object>) getMap(mapId);
             if (map == null) {
-                // pages of maps that are not open or that have been removed
-                // later on are not moved (for maps that are not open, the live
-                // counter is not decremented, so the chunk is not removed)
+                boolean mapExists = meta.containsKey("root." + Integer.toHexString(mapId));
+                if (mapExists) {
+                    // pages of maps that were removed: the live count was
+                    // already decremented, but maps that are not open, the
+                    // chunk is not removed
+                    mapNotOpen = true;
+                }
                 buff.position(offset + pageLength);
                 continue;
             }
             buff.position(offset);
             Page page = new Page(map, 0);
             page.read(buff, chunk.id, buff.position(), length);
-            for (int i = 0; i < page.getKeyCount(); i++) {
-                Object k = page.getKey(i);
-                Page p = map.getPage(k);
-                if (p == null) {
-                    // was removed later - ignore
-                    // or the chunk no longer exists
-                } else if (p.getPos() == 0) {
-                    // temporarily changed - ok
-                    // TODO move old data if there is an uncommitted change?
-                } else {
-                    Chunk c = getChunk(p.getPos());
-                    if (old.contains(c)) {
+            int type = page.isLeaf() ? 0 : 1;
+            long pos = DataUtils.getPagePos(chunk.id, offset, pageLength, type);
+            page.setPos(pos);
+            Object k = map.getLiveKey(page);
+            if (k != null) {
                 Object value = map.remove(k);
+                if (value != null) {
                     map.put(k, value);
+                    changeCount++;
+                }
+            }
+        }
+        if (!mapNotOpen && changeCount == 0) {
+            // if all maps are open, but no changes were made,
+            // then live bookkeeping is wrong, and we anyway
+            // remove the chunk
+            // (but we first need to check that there are no
+            // pending changes)
+            for (HashMap<Integer, Chunk> e : freedPageSpace.values()) {
+                for (int x : e.keySet()) {
+                    if (x == chunk.id) {
+                        changeCount++;
+                        break;
+                    }
                 }
             }
+            if (changeCount == 0) {
+                // bookkeeping is broken for this chunk:
+                // fix it
+                registerFreePage(currentVersion, chunk.id,
+                        chunk.maxLenLive, chunk.pageCountLive);
             }
         }
     }

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

@@ -11,9 +11,14 @@ import java.io.PrintWriter;
 import java.io.Writer;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
+import java.sql.Timestamp;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.TreeMap;
 
 import org.h2.mvstore.type.StringDataType;
 import org.h2.store.fs.FilePath;
+import org.h2.store.fs.FileUtils;
 
 /**
  * Utility methods used in combination with the MVStore.
@@ -24,8 +29,10 @@ public class MVStoreTool {
      * Runs this tool.
      * Options are case sensitive. Supported options are:
      * <table>
-     * <tr><td>[-dump &lt;dir&gt;]</td>
+     * <tr><td>[-dump &lt;fileName&gt;]</td>
      * <td>Dump the contends of the file</td></tr>
+     * <tr><td>[-info &lt;fileName&gt;]</td>
+     * <td>Get summary information about a file</td></tr>
      * </table>
      *
      * @param args the command line arguments
@@ -35,6 +42,9 @@ public class MVStoreTool {
             if ("-dump".equals(args[i])) {
                 String fileName = args[++i];
                 dump(fileName, new PrintWriter(System.out));
+            } else if ("-info".equals(args[i])) {
+                String fileName = args[++i];
+                info(fileName, new PrintWriter(System.out));
             }
         }
     }
@@ -48,6 +58,15 @@ public class MVStoreTool {
         dump(fileName, new PrintWriter(System.out));
     }
 
+    /**
+     * Read the summary information of the file and write them to system out.
+     *
+     * @param fileName the name of the file
+     */
+    public static void info(String fileName) {
+        info(fileName, new PrintWriter(System.out));
+    }
+
     /**
      * Read the contents of the file and display them in a human-readable
      * format.
@@ -85,7 +104,13 @@ public class MVStoreTool {
                     continue;
                 }
                 block.position(0);
-                Chunk c = Chunk.readChunkHeader(block, pos);
+                Chunk c = null;
+                try {
+                    c = Chunk.readChunkHeader(block, pos);
+                } catch (IllegalStateException e) {
+                    pos += blockSize;
+                    continue;
+                }
                 int length = c.len * MVStore.BLOCK_SIZE;
                 pw.printf("%n%0" + len + "x chunkHeader %s%n",
                         pos, c.toString());
@@ -152,7 +177,7 @@ public class MVStoreTool {
                             pw.printf("    %d children >= %s @ chunk %x +%0" +
                                     len + "x%n",
                                     counts[entries],
-                                    keys[entries],
+                                    keys.length >= entries ? null : keys[entries],
                                     DataUtils.getPageChunkId(cp),
                                     DataUtils.getPageOffset(cp));
                         } else if (!compressed) {
@@ -204,4 +229,67 @@ public class MVStoreTool {
         pw.flush();
     }
 
+    /**
+     * Read the summary information of the file and write them to system out.
+     *
+     * @param fileName the name of the file
+     * @param writer the print writer
+     */
+    public static void info(String fileName, Writer writer) {
+        PrintWriter pw = new PrintWriter(writer, true);
+        if (!FilePath.get(fileName).exists()) {
+            pw.println("File not found: " + fileName);
+            return;
+        }
+        long fileLength = FileUtils.size(fileName);
+        MVStore store = new MVStore.Builder().
+                fileName(fileName).
+                readOnly().open();
+        try {
+            MVMap<String, String> meta = store.getMetaMap();
+            Map<String, Object> header = store.getStoreHeader();
+            long fileCreated = DataUtils.readHexLong(header, "created", 0L);
+            TreeMap<Integer, Chunk> chunks = new TreeMap<Integer, Chunk>();
+            long chunkLength = 0;
+            long maxLength = 0;
+            long maxLengthLive = 0;
+            for (Entry<String, String> e : meta.entrySet()) {
+                String k = e.getKey();
+                if (k.startsWith("chunk.")) {
+                    Chunk c = Chunk.fromString(e.getValue());
+                    chunks.put(c.id, c);
+                    chunkLength += c.len * MVStore.BLOCK_SIZE;
+                    maxLength += c.maxLen;
+                    maxLengthLive += c.maxLenLive;
+                }
+            }
+            pw.printf("Created: %s\n", formatTimestamp(fileCreated));
+            pw.printf("File length: %d\n", fileLength);
+            pw.printf("Chunk length: %d\n", chunkLength);
+            pw.printf("Chunk count: %d\n", chunks.size());
+            pw.printf("Used space: %d%%\n", 100 * chunkLength / fileLength);
+            pw.printf("Chunk fill rate: %d%%\n", 100 * maxLengthLive / maxLength);
+            for (Entry<Integer, Chunk> e : chunks.entrySet()) {
+                Chunk c = e.getValue();
+                long created = fileCreated + c.time;
+                pw.printf("  Chunk %d: %s, %d%% used, %d blocks\n",
+                        c.id, formatTimestamp(created),
+                        100 * c.maxLenLive / c.maxLen,
+                        c.len
+                        );
+            }
+        } catch (Exception e) {
+            pw.println("ERROR: " + e);
+            e.printStackTrace(pw);
+        } finally {
+            store.close();
+        }
+        pw.flush();
+    }
+
+    private static String formatTimestamp(long t) {
+        String x = new Timestamp(t).toString();
+        return x.substring(0, 19);
+    }
+
 }

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

@@ -222,6 +222,24 @@ public class Page {
         return p != null ? p : map.readPage(children[index]);
     }
 
+    /**
+     * Get the child page at the given index, if it is live.
+     *
+     * @param index the child index
+     * @return the page, or null if the chunk is not live
+     */
+    public Page getLiveChildPage(int index) {
+        Page p = childrenPages[index];
+        if (p != null) {
+            return p;
+        }
+        long pos = children[index];
+        if (!map.store.isChunkLive(pos)) {
+            return null;
+        }
+        return getChildPage(index);
+    }
+
     /**
      * Get the value at the given index.
      *
@@ -264,6 +282,10 @@ public class Page {
         StringBuilder buff = new StringBuilder();
         buff.append("id: ").append(System.identityHashCode(this)).append('\n');
         buff.append("pos: ").append(Long.toHexString(pos)).append("\n");
+        if (pos != 0) {
+            int chunkId = DataUtils.getPageChunkId(pos);
+            buff.append("chunk: ").append(Long.toHexString(chunkId)).append("\n");
+        }
         for (int i = 0; i <= keyCount; i++) {
             if (i > 0) {
                 buff.append(" ");
@@ -734,16 +756,16 @@ public class Page {
         if (pageLength > maxLength) {
             throw DataUtils.newIllegalStateException(
                     DataUtils.ERROR_FILE_CORRUPT,
-                    "File corrupted, expected page length =< {0}, got {1}",
-                    maxLength, pageLength);
+                    "File corrupted in chunk {0}, expected page length =< {1}, got {2}",
+                    chunkId, maxLength, pageLength);
         }
         short check = buff.getShort();
         int mapId = DataUtils.readVarInt(buff);
         if (mapId != map.getId()) {
             throw DataUtils.newIllegalStateException(
                     DataUtils.ERROR_FILE_CORRUPT,
-                    "File corrupted, expected map id {0}, got {1}",
-                    map.getId(), mapId);
+                    "File corrupted in chunk {0}, expected map id {1}, got {2}",
+                    chunkId, map.getId(), mapId);
         }
         int checkTest = DataUtils.getCheckValue(chunkId)
                 ^ DataUtils.getCheckValue(offset)
@@ -751,8 +773,8 @@ public class Page {
         if (check != (short) checkTest) {
             throw DataUtils.newIllegalStateException(
                     DataUtils.ERROR_FILE_CORRUPT,
-                    "File corrupted, expected check value {0}, got {1}",
-                    checkTest, check);
+                    "File corrupted in chunk {0}, expected check value {1}, got {2}",
+                    chunkId, checkTest, check);
         }
         int len = DataUtils.readVarInt(buff);
         keys = new Object[len];
@@ -1007,4 +1029,8 @@ public class Page {
         map.removePage(pos, memory);
     }
 
+    public void setPos(long pos) {
+        this.pos = pos;
+    }
+
 }

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

@@ -191,7 +191,7 @@ public class MVTableEngine implements TableEngine {
             if (s == null || s.isReadOnly()) {
                 return;
             }
-            if (!store.compact(50, 1024 * 1024)) {
+            if (!store.compact(50, 4 * 1024 * 1024)) {
                 store.commit();
             }
         }
@@ -297,7 +297,7 @@ public class MVTableEngine implements TableEngine {
         public void compactFile(long maxCompactTime) {
             store.setRetentionTime(0);
             long start = System.currentTimeMillis();
-            while (store.compact(99, 16 * 1024)) {
+            while (store.compact(99, 4 * 1024 * 1024)) {
                 store.sync();
                 long time = System.currentTimeMillis() - start;
                 if (time > maxCompactTime) {
@@ -320,7 +320,7 @@ public class MVTableEngine implements TableEngine {
                     if (!store.getFileStore().isReadOnly()) {
                         transactionStore.close();
                         long start = System.currentTimeMillis();
-                        while (store.compact(90, 32 * 1024)) {
+                        while (store.compact(90, 4 * 1024 * 1024)) {
                             long time = System.currentTimeMillis() - start;
                             if (time > maxCompactTime) {
                                 break;

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

@@ -24,7 +24,7 @@ import org.h2.mvstore.type.ObjectDataType;
 import org.h2.util.New;
 
 /**
- * A store that supports concurrent transactions.
+ * A store that supports concurrent MVCC read-committed transactions.
  */
 public class TransactionStore {
 

h2/src/main/org/h2/mvstore/rtree/MVRTreeMap.java

@@ -122,8 +122,25 @@ public class MVRTreeMap<V> extends MVMap<SpatialKey, V> {
     }
 
     @Override
-    protected Page getPage(SpatialKey key) {
-        return getPage(root, key);
+    protected SpatialKey getLiveKey(Page p) {
+        while (!p.isLeaf()) {
+            p = p.getLiveChildPage(0);
+            if (p == null) {
+                return null;
+            }
+        }
+        SpatialKey key = (SpatialKey) p.getKey(0);
+        Page p2 = getPage(root, key);
+        if (p2 == null) {
+            return null;
+        }
+        if (p2.getPos() == 0) {
+            return p2 == p ? key : null;
+        }
+        if (p2.getPos() == p.getPos()) {
+            return key;
+        }
+        return null;
     }
 
     private Page getPage(Page p, Object key) {

h2/src/main/org/h2/tools/Recover.java

@@ -585,6 +585,8 @@ public class Recover extends Tool implements DataHandler {
                 Constants.SUFFIX_MV_FILE.length()));
         MVStore mv = new MVStore.Builder().fileName(fileName).readOnly().open();
         dumpLobMaps(writer, mv);
+        writer.println("-- Meta");
+        dumpMeta(writer, mv);
         writer.println("-- Tables");
         TransactionStore store = new TransactionStore(mv);
         try {
@@ -654,6 +656,13 @@ public class Recover extends Tool implements DataHandler {
         }
     }
 
+    private static void dumpMeta(PrintWriter writer, MVStore mv) {
+        MVMap<String, String> meta = mv.getMetaMap();
+        for (Entry<String, String> e : meta.entrySet()) {
+            writer.println("-- " + e.getKey() + " = " + e.getValue());
+        }
+    }
+
     private void dumpLobMaps(PrintWriter writer, MVStore mv) {
         lobMaps = mv.hasMap("lobData");
         if (!lobMaps) {

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

@@ -1673,7 +1673,7 @@ public class TestMVStore extends TestBase {
 
         m = s.openMap("data");
         assertTrue(s.compact(80, 16 * 1024));
-        assertTrue(s.compact(80, 1024));
+        assertFalse(s.compact(80, 1024));
 
         int chunkCount3 = 0;
         for (String k : meta.keySet()) {