Report leftover MVV mark bits in IntegrityCheck

A prune interrupted before the issue #286 fix could leave 0x8000 mark
bits behind in on-disk version length fields. Since PR #288 the read
paths strip the mark bit silently, so such versions read normally and
icheck reported affected volumes as clean.

Add MVV.countMarkedVersions and scan every data record during icheck:
marked versions are counted in a new MarkedVersions counter (report,
CSV and getMarkedVersionCount) and reported as a fault so affected
volumes can be identified. icheck -p remains the remediation: pruning
rewrites the marked versions.

Fixes #290

Author: vharseko

persistit/core/src/main/java/com/persistit/IntegrityCheck.java

@@ -98,6 +98,7 @@ private static class Counters {
         private long _mvvCount = 0;
         private long _mvvOverhead = 0;
         private long _mvvAntiValues = 0;
+        private long _markedVersionCount = 0;
         private long _pruningErrorCount = 0;
         private long _prunedPageCount = 0;
         private long _garbagePageCount = 0;
@@ -118,6 +119,7 @@ private static class Counters {
             _mvvCount = counters._mvvCount;
             _mvvOverhead = counters._mvvOverhead;
             _mvvAntiValues = counters._mvvAntiValues;
+            _markedVersionCount = counters._markedVersionCount;
             _pruningErrorCount = counters._pruningErrorCount;
             _prunedPageCount = counters._prunedPageCount;
             _garbagePageCount = counters._garbagePageCount;
@@ -135,6 +137,7 @@ void difference(final Counters counters) {
             _mvvCount = counters._mvvCount - _mvvCount;
             _mvvOverhead = counters._mvvOverhead - _mvvOverhead;
             _mvvAntiValues = counters._mvvAntiValues - _mvvAntiValues;
+            _markedVersionCount = counters._markedVersionCount - _markedVersionCount;
             _pruningErrorCount = counters._pruningErrorCount - _pruningErrorCount;
             _prunedPageCount = counters._prunedPageCount - _prunedPageCount;
             _garbagePageCount = counters._garbagePageCount - _garbagePageCount;
@@ -144,20 +147,21 @@ void difference(final Counters counters) {
         public String toString() {
             return String.format("Index pages/bytes: %,d / %,d Data pages/bytes: %,d / %,d"
                     + " LongRec pages/bytes: %,d / %,d  MVV pages/records/bytes/antivalues: "
-                    + "%,d / %,d / %,d / %,d  Holes %,d Pages pruned %,d", _indexPageCount, _indexBytesInUse,
-                    _dataPageCount, _dataBytesInUse, _longRecordPageCount, _longRecordBytesInUse, _mvvPageCount,
-                    _mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount);
+                    + "%,d / %,d / %,d / %,d  Holes %,d Pages pruned %,d Marked versions %,d", _indexPageCount,
+                    _indexBytesInUse, _dataPageCount, _dataBytesInUse, _longRecordPageCount, _longRecordBytesInUse,
+                    _mvvPageCount, _mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount,
+                    _markedVersionCount);
         }
 
         private String toCSV() {
-            return String.format("%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d", _indexPageCount, _indexBytesInUse,
+            return String.format("%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d", _indexPageCount, _indexBytesInUse,
                     _dataPageCount, _dataBytesInUse, _longRecordPageCount, _longRecordBytesInUse, _mvvPageCount,
-                    _mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount);
+                    _mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount, _markedVersionCount);
         }
 
         private final static String CSV_HEADERS = "IndexPages,IndexBytes,"
                 + "DataPages,DataBytes,LongRecordPages,LongRecordBytes,MvvPages,"
-                + "MvvRecords,MvvOverhead,MvvAntiValues,IndexHoles,PrunedPages";
+                + "MvvRecords,MvvOverhead,MvvAntiValues,IndexHoles,PrunedPages,MarkedVersions";
 
     }
 
@@ -182,9 +186,28 @@ public void sawVersion(final long version, final int offset, final int valueLeng
 
     private final VerifyVisitor _visitor = new VerifyVisitor() {
 
+        private long _currentPage;
+
+        @Override
+        protected void visitPage(final long timestamp, final Volume volume, final long page, final int type,
+                final int bufferSize, final int keyBlockStart, final int keyBlockEnd, final int alloc,
+                final int available, final long rightSibling) throws PersistitException {
+            _currentPage = page;
+        }
+
         @Override
         protected void visitDataRecord(final Key key, final int foundAt, final int tail, final int klength,
                 final int offset, final int length, final byte[] bytes) throws PersistitException {
+            /*
+             * Leftover prune marks read normally (the length accessors strip
+             * the mark bit), so they must be counted before visitAllVersions
+             * and reported explicitly - see issue #290.
+             */
+            final int marked = MVV.countMarkedVersions(bytes, offset, length);
+            if (marked > 0) {
+                _counters._markedVersionCount += marked;
+                addFault("MVV has " + plural(marked, "version") + " with a leftover mark bit", _currentPage, 0, foundAt);
+            }
             MVV.visitAllVersions(_versionVisitor, bytes, offset, length);
             if (_versionVisitor._count > 0) {
                 _counters._mvvCount++;
@@ -544,6 +567,16 @@ public long getMvvAntiValues() {
         return _counters._mvvAntiValues;
     }
 
+    /**
+     * @return Count of MVV versions whose length field still carries a
+     *         leftover prune mark bit, left behind by a prune interrupted
+     *         before the issue #286 fix. Such versions read normally but
+     *         indicate the volume was corrupted; pruning rewrites them.
+     */
+    public long getMarkedVersionCount() {
+        return _counters._markedVersionCount;
+    }
+
     /**
      * @return Count of pages for which an expected index pointer is missing
      */

persistit/core/src/main/java/com/persistit/MVV.java

@@ -625,6 +625,37 @@ static boolean verify(final byte[] bytes, final int offset, final int length) {
         return true;
     }
 
+    /**
+     * Count versions whose length field still carries the mark bit. Marks are
+     * transient state private to {@link #prune} and must never be observable
+     * outside it; a non-zero count on a stored MVV means the value was
+     * corrupted by a prune interrupted before the issue #286 fix. The length
+     * accessors strip the mark bit silently, so such versions read normally
+     * and only this scan can detect them (issue #290).
+     *
+     * @param bytes
+     *            the byte array
+     * @param offset
+     *            the index of the first byte of the MVV within the byte array
+     * @param length
+     *            the count of bytes in the MVV
+     * @return the number of marked versions, 0 for a non-MVV value
+     */
+    static int countMarkedVersions(final byte[] bytes, final int offset, final int length) {
+        if (!isArrayMVV(bytes, offset, length)) {
+            return 0;
+        }
+        int marked = 0;
+        int from = offset + 1;
+        while (from + LENGTH_PER_VERSION <= offset + length) {
+            if (isMarked(bytes, from)) {
+                marked++;
+            }
+            from += getLength(bytes, from) + LENGTH_PER_VERSION;
+        }
+        return marked;
+    }
+
     static boolean verify(final TransactionIndex ti, final byte[] bytes, final int offset, final int length) {
         if (!isArrayMVV(bytes, offset, length)) {
             /*

persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java

@@ -99,6 +99,56 @@ public void testBrokenMVVs() throws Exception {
         assertTrue(icheck.getFaults().length > 0);
     }
 
+    /**
+     * Issue #290: a leftover prune mark bit (pre-#288 interrupted-prune
+     * corruption) reads normally through all fetch paths, so IntegrityCheck
+     * must detect it explicitly and report it as a fault.
+     */
+    @Test
+    public void testLeftoverMarkBitsReported() throws Exception {
+        final Exchange ex = _persistit.getExchange(_volumeName, "mvv", true);
+        disableBackgroundCleanup();
+        transactionalStore(ex);
+
+        IntegrityCheck icheck = icheck();
+        icheck.checkTree(ex.getTree());
+        assertEquals(0, icheck.getFaults().length);
+        assertEquals(0, icheck.getMarkedVersionCount());
+
+        final int corrupted = corrupt5(ex);
+        assertTrue(corrupted > 0);
+
+        icheck = icheck();
+        icheck.checkTree(ex.getTree());
+        assertEquals(corrupted, icheck.getFaults().length);
+        assertEquals(corrupted, icheck.getMarkedVersionCount());
+    }
+
+    /**
+     * Issue #290: pruning rewrites marked versions, so icheck with pruning
+     * enabled is the remediation path - a subsequent icheck must be clean.
+     */
+    @Test
+    public void testPruneClearsLeftoverMarkBits() throws Exception {
+        final Exchange ex = _persistit.getExchange(_volumeName, "mvv", true);
+        disableBackgroundCleanup();
+        transactionalStore(ex);
+        _persistit.getTransactionIndex().updateActiveTransactionCache();
+
+        assertTrue(corrupt5(ex) > 0);
+
+        IntegrityCheck icheck = icheck();
+        icheck.setPruneEnabled(true);
+        icheck.checkTree(ex.getTree());
+        assertTrue(icheck.getMarkedVersionCount() > 0);
+        assertTrue(icheck.getPrunedPagesCount() > 0);
+
+        icheck = icheck();
+        icheck.checkTree(ex.getTree());
+        assertEquals(0, icheck.getMarkedVersionCount());
+        assertEquals(0, icheck.getFaults().length);
+    }
+
     @Test
     public void testIndexFixHoles() throws Exception {
         final Exchange ex = _persistit.getExchange(_volumeName, "mvv", true);
@@ -338,6 +388,31 @@ private void corrupt4(final Exchange ex) throws PersistitException {
         buffer.release();
     }
 
+    /**
+     * Simulates the pre-#288 interrupted-prune corruption (issue #286) by
+     * setting the leftover mark bit on the first version of some MVV values
+     *
+     * @param ex
+     * @return the number of values corrupted
+     * @throws PersistitException
+     */
+    private int corrupt5(final Exchange ex) throws PersistitException {
+        ex.ignoreMVCCFetch(true);
+        ex.clear().to(key(500));
+        int corrupted = 0;
+        while (corrupted < 10 && ex.next()) {
+            final byte[] bytes = ex.getValue().getEncodedBytes();
+            final int length = ex.getValue().getEncodedSize();
+            if (MVV.isArrayMVV(bytes, 0, length)) {
+                MVV.mark(bytes, 1);
+                ex.store();
+                corrupted++;
+            }
+        }
+        ex.ignoreMVCCFetch(false);
+        return corrupted;
+    }
+
     private IntegrityCheck icheck() {
         final IntegrityCheck icheck = new IntegrityCheck(_persistit);
         icheck.setMessageLogVerbosity(Task.LOG_VERBOSE);

persistit/core/src/test/java/com/persistit/MVVTest.java

@@ -587,6 +587,34 @@ public void pruneExceptionUnmarksVersionsAtNonZeroOffset() throws Exception {
         assertArrayEquals("prune must leave the byte array unchanged when it throws", before, bytes);
     }
 
+    /**
+     * Issue #290: leftover mark bits are invisible to the read paths (the
+     * length accessors strip them), so IntegrityCheck needs a dedicated scan
+     * to detect them.
+     */
+    @Test
+    public void countMarkedVersions() {
+        final byte[] source = newArray(TYPE_MVV, 0, 0, 0, 0, 0, 0, 0, 10, 0, 2, 0xA, 0xB, 0, 0, 0, 0, 0, 0, 0, 11, 0,
+                3, 0xC, 0xD, 0xE);
+        assertEquals(0, MVV.countMarkedVersions(source, 0, source.length));
+        MVV.mark(source, 1);
+        assertEquals(1, MVV.countMarkedVersions(source, 0, source.length));
+        MVV.mark(source, 13);
+        assertEquals(2, MVV.countMarkedVersions(source, 0, source.length));
+        MVV.unmark(source, 1);
+        assertEquals(1, MVV.countMarkedVersions(source, 0, source.length));
+        MVV.unmark(source, 13);
+        assertEquals(0, MVV.countMarkedVersions(source, 0, source.length));
+    }
+
+    @Test
+    public void countMarkedVersionsNonMVV() {
+        final byte[] source = newArray(0xA, 0xB, 0xC);
+        assertEquals(0, MVV.countMarkedVersions(source, 0, source.length));
+        assertEquals(0, MVV.countMarkedVersions(source, 0, 0));
+        assertEquals(0, MVV.countMarkedVersions(source, 0, -1));
+    }
+
     //
     // Test helper methods
     //