From e9efe413309027da47998eaaed05127e6167e26b Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Tue, 21 Jul 2026 14:32:43 +0300 Subject: [PATCH 1/2] 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. The scan runs only after visitAllVersions has validated the version structure, so a structurally corrupt MVV cannot produce a bogus marked-version fault. icheck -p remains the remediation: pruning rewrites the marked versions (issue #292 makes prune clear stale marks unconditionally). With icheck -P the marked-version faults are reported but do not block clearing the TransactionIndex, since the prune pass has already rewritten the marks. The icheck -c CSV example in Management.rst is updated for the new MarkedVersions column. Fixes #290 --- .../java/com/persistit/IntegrityCheck.java | 58 +++++- .../core/src/main/java/com/persistit/MVV.java | 31 +++ .../com/persistit/IntegrityCheckTest.java | 177 ++++++++++++++++-- .../src/test/java/com/persistit/MVVTest.java | 28 +++ persistit/doc/Management.rst | 6 +- 5 files changed, 275 insertions(+), 25 deletions(-) diff --git a/persistit/core/src/main/java/com/persistit/IntegrityCheck.java b/persistit/core/src/main/java/com/persistit/IntegrityCheck.java index 06f56d7eb..65337a411 100644 --- a/persistit/core/src/main/java/com/persistit/IntegrityCheck.java +++ b/persistit/core/src/main/java/com/persistit/IntegrityCheck.java @@ -60,6 +60,7 @@ public class IntegrityCheck extends Task { private Volume _currentVolume; private Tree _currentTree; + private long _currentPage; private LongBitSet _usedPageBits = new LongBitSet(); private long _totalPages = 0; private long _pagesVisited = 0; @@ -79,6 +80,7 @@ public class IntegrityCheck extends Task { private boolean _csv; private final ArrayList _faults = new ArrayList(); + private int _markedVersionFaultCount; private final ArrayList _holes = new ArrayList(); // Used in checking long values @@ -98,6 +100,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 +121,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 +139,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 +149,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"; } @@ -186,6 +192,20 @@ public void sawVersion(final long version, final int offset, final int valueLeng 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 { MVV.visitAllVersions(_versionVisitor, bytes, offset, length); + /* + * Scanned only after visitAllVersions has validated the version + * structure. Leftover prune marks read normally (the length + * accessors strip the mark bit), so only this dedicated scan can + * detect and report them - see issue #290. + */ + final int marked = MVV.countMarkedVersions(bytes, offset, length); + if (marked > 0) { + _counters._markedVersionCount += marked; + if (addFault("MVV has " + plural(marked, "version") + " with a leftover mark bit", _currentPage, 0, + foundAt)) { + _markedVersionFaultCount++; + } + } if (_versionVisitor._count > 0) { _counters._mvvCount++; final int voffset = _versionVisitor._lastOffset; @@ -289,7 +309,14 @@ protected void runTask() { postMessage("Total " + toString(), LOG_NORMAL); } if (_pruneAndClear) { - if (_faults.isEmpty() && _counters._mvvPageCount == _counters._prunedPageCount + /* + * Leftover mark-bit faults (issue #290) identify volumes + * affected by a pre-#286 interrupted prune but do not block + * clearing the TransactionIndex: the prune pass above has + * already rewritten those marks (issue #292). Only other + * faults and incomplete pruning count as failures here. + */ + if (_faults.size() == _markedVersionFaultCount && _counters._mvvPageCount == _counters._prunedPageCount && _counters._pruningErrorCount == 0) { final int count = _persistit.getTransactionIndex().resetMVVCounts(startTimestamp); postMessage(String.format("%,d aborted transactions were cleared by pruning", count), LOG_NORMAL); @@ -329,11 +356,13 @@ private String plural(final int n, final String m) { } } - private void addFault(final String description, final long page, final int level, final int position) { + private boolean addFault(final String description, final long page, final int level, final int position) { final Fault fault = new Fault(resourceName(), this, description, page, _treeDepth, level, position); - if (_faults.size() < MAX_FAULTS) + final boolean recorded = _faults.size() < MAX_FAULTS; + if (recorded) _faults.add(fault); postMessage(fault.toString(), LOG_VERBOSE); + return recorded; } private void addGarbageFault(final String description, final long page, final int level, final int position) { @@ -544,6 +573,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 */ @@ -1144,6 +1183,7 @@ private Buffer walkRight(final int level, final long toPage, final Key key, fina } private boolean verifyPage(final Buffer buffer, final long page, final int level, final Key key, final Tree tree) { + _currentPage = page; if (buffer.getPageAddress() != page) { addFault("Buffer contains wrong page " + buffer.getPageAddress(), page, level, 0); return false; diff --git a/persistit/core/src/main/java/com/persistit/MVV.java b/persistit/core/src/main/java/com/persistit/MVV.java index df90b07ac..2236404eb 100644 --- a/persistit/core/src/main/java/com/persistit/MVV.java +++ b/persistit/core/src/main/java/com/persistit/MVV.java @@ -654,6 +654,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)) { /* diff --git a/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java b/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java index 4e688f198..331e39986 100644 --- a/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java +++ b/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java @@ -23,8 +23,12 @@ import org.junit.Test; import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.SortedMap; +import java.util.TreeMap; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -99,6 +103,88 @@ 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); + final int corruptedValues = corrupted[0]; + final int markedVersions = corrupted[1]; + assertTrue(corruptedValues > 0); + assertTrue("some values must carry more than one mark", markedVersions > corruptedValues); + + icheck = icheck(); + icheck.checkTree(ex.getTree()); + assertEquals(corruptedValues, icheck.getFaults().length); + assertEquals(markedVersions, 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(); + + final SortedMap expectedContents = treeContents(ex); + assertTrue(corrupt5(ex)[0] > 0); + + IntegrityCheck icheck = icheck(); + icheck.setPruneEnabled(true); + icheck.checkTree(ex.getTree()); + assertTrue(icheck.getMarkedVersionCount() > 0); + assertTrue(icheck.getPrunedPagesCount() > 0); + + assertEquals("remediation must not change the visible contents", expectedContents, treeContents(ex)); + + icheck = icheck(); + icheck.checkTree(ex.getTree()); + assertEquals(0, icheck.getMarkedVersionCount()); + assertEquals(0, icheck.getFaults().length); + } + + /** + * Issues #290/#292: icheck -P reports leftover mark bits as faults but + * must still clear the TransactionIndex - the prune pass has already + * rewritten the marks, so they are not a pruning failure. + */ + @Test + public void testPruneAndClearWithLeftoverMarkBits() throws Exception { + final Exchange ex = _persistit.getExchange(_volumeName, "mvv", true); + disableBackgroundCleanup(); + transactionalStore(ex); + _persistit.getTransactionIndex().updateActiveTransactionCache(); + + assertTrue(corrupt5(ex)[0] > 0); + + final IntegrityCheck icheck = (IntegrityCheck) CLI.parseTask(_persistit, "icheck trees=* -u -P"); + final StringWriter output = new StringWriter(); + icheck.setMessageWriter(new PrintWriter(output)); + icheck.setup(1, "icheck", "cli", 0, 5); + icheck.run(); + + assertTrue(icheck.getMarkedVersionCount() > 0); + assertTrue(icheck.hasFaults()); + assertTrue(output.toString(), output.toString().contains("aborted transactions were cleared by pruning")); + assertFalse(output.toString(), output.toString().contains("PruneAndClear failed")); + } + @Test public void testIndexFixHoles() throws Exception { final Exchange ex = _persistit.getExchange(_volumeName, "mvv", true); @@ -283,26 +369,52 @@ private void corrupt1(final Exchange ex) throws PersistitException { buffer.release(); } + private interface MVVCorruption { + /** Corrupts one stored MVV value; returns its contribution to the total */ + int corrupt(byte[] bytes, int length); + } + /** - * Corrupts some MVV values on a page by damaging a version length field - * + * Applies a corruption to the raw bytes of up to ten stored MVV values + * * @param ex + * @param corruption + * @return the number of values corrupted and the summed corruption + * contributions, as a two-element array * @throws PersistitException */ - private void corrupt2(final Exchange ex) throws PersistitException { + private int[] corruptMVVs(final Exchange ex, final MVVCorruption corruption) 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)) { - bytes[9]++; - ex.store(); - corrupted++; + try { + ex.clear().to(key(500)); + int corrupted = 0; + int total = 0; + while (corrupted < 10 && ex.next()) { + final byte[] bytes = ex.getValue().getEncodedBytes(); + final int length = ex.getValue().getEncodedSize(); + if (MVV.isArrayMVV(bytes, 0, length)) { + total += corruption.corrupt(bytes, length); + ex.store(); + corrupted++; + } } + return new int[] { corrupted, total }; + } finally { + ex.ignoreMVCCFetch(false); } - ex.ignoreMVCCFetch(false); + } + + /** + * Corrupts some MVV values on a page by damaging a version length field + * + * @param ex + * @throws PersistitException + */ + private void corrupt2(final Exchange ex) throws PersistitException { + corruptMVVs(ex, (bytes, length) -> { + bytes[9]++; + return 1; + }); } /** @@ -338,6 +450,45 @@ 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 every version of some MVV values + * + * @param ex + * @return the number of values corrupted and the total number of versions + * marked, as a two-element array + * @throws PersistitException + */ + private int[] corrupt5(final Exchange ex) throws PersistitException { + return corruptMVVs(ex, (bytes, length) -> { + int marked = 0; + int from = 1; + while (from + MVV.LENGTH_PER_VERSION <= length) { + MVV.mark(bytes, from); + marked++; + from += MVV.getLength(bytes, from) + MVV.LENGTH_PER_VERSION; + } + return marked; + }); + } + + /** + * Returns the visible key/value contents of the tree as seen by a + * non-transactional traversal + * + * @param ex + * @return the visible contents, keyed by decoded key string + * @throws PersistitException + */ + private SortedMap treeContents(final Exchange ex) throws PersistitException { + final SortedMap contents = new TreeMap(); + ex.clear().append(Key.BEFORE); + while (ex.next()) { + contents.put(ex.getKey().decodeString(), ex.getValue().getString()); + } + return contents; + } + private IntegrityCheck icheck() { final IntegrityCheck icheck = new IntegrityCheck(_persistit); icheck.setMessageLogVerbosity(Task.LOG_VERBOSE); diff --git a/persistit/core/src/test/java/com/persistit/MVVTest.java b/persistit/core/src/test/java/com/persistit/MVVTest.java index 5fc3f5a23..c2e05416d 100644 --- a/persistit/core/src/test/java/com/persistit/MVVTest.java +++ b/persistit/core/src/test/java/com/persistit/MVVTest.java @@ -625,6 +625,34 @@ public void pruneExceptionMustNotUnmarkPastRegionEnd() throws Exception { assertArrayEquals("prune must not write outside the MVV region 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)); + } + // // Issue #292: prune uses the mark bit as private transient state and // assumes no version is marked on entry. A stale mark left on disk by a diff --git a/persistit/doc/Management.rst b/persistit/doc/Management.rst index bf02959b1..420b94f7b 100644 --- a/persistit/doc/Management.rst +++ b/persistit/doc/Management.rst @@ -133,9 +133,9 @@ programmatically or through JMX. (You may specify any valid, available port.) Th Where ``classpath`` includes the Persistit library. Assuming the name of the script is ``pcli`` you can then invoke commands from a shell as shown in this example:: /home/akiban:~$ pcli icheck -v -c "trees=*:Acc*" - Volume,Tree,Faults,IndexPages,IndexBytes,DataPages,DataBytes,LongRecordPages,LongRecordBytes,MvvPages,MvvRecords,MvvOverhead,MvvAntiValues,IndexHoles,PrunedPages - "persistit","AccumulatorRecoveryTest",0,3,24296,1519,15560788,0,0,1506,52192,721521,2397,0,0 - "*","*",0,3,24296,1519,15560788,0,0,1506,52192,721521,2397,0,0 + Volume,Tree,Faults,IndexPages,IndexBytes,DataPages,DataBytes,LongRecordPages,LongRecordBytes,MvvPages,MvvRecords,MvvOverhead,MvvAntiValues,IndexHoles,PrunedPages,MarkedVersions + "persistit","AccumulatorRecoveryTest",0,3,24296,1519,15560788,0,0,1506,52192,721521,2397,0,0,0 + "*","*",0,3,24296,1519,15560788,0,0,1506,52192,721521,2397,0,0,0 /home/akiban:~$ Alternatively, you can use ``curl`` as follows:: From a69c0b811c11d4020795efa0c7e27a09797ac812 Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 22 Jul 2026 14:05:22 +0300 Subject: [PATCH 2/2] Compare fault counts in the icheck -P gate, not the capped fault list The pruneAndClear gate excluded marked-version faults by comparing _faults.size() with the marked-fault count, but both stop moving once the fault list reaches MAX_FAULTS: on a volume with more than MAX_FAULTS marked-version records a genuine fault found afterwards (the garbage chain is checked after the trees) is dropped from the list and the arithmetic still converges, so -P cleared the TransactionIndex despite real corruption. Count every fault found - recorded in the list or not - and gate on the counts instead. --- .../java/com/persistit/IntegrityCheck.java | 24 ++++--- .../com/persistit/IntegrityCheckTest.java | 66 ++++++++++++++++++- 2 files changed, 77 insertions(+), 13 deletions(-) diff --git a/persistit/core/src/main/java/com/persistit/IntegrityCheck.java b/persistit/core/src/main/java/com/persistit/IntegrityCheck.java index 65337a411..51b8de83b 100644 --- a/persistit/core/src/main/java/com/persistit/IntegrityCheck.java +++ b/persistit/core/src/main/java/com/persistit/IntegrityCheck.java @@ -80,6 +80,7 @@ public class IntegrityCheck extends Task { private boolean _csv; private final ArrayList _faults = new ArrayList(); + private int _faultCount; private int _markedVersionFaultCount; private final ArrayList _holes = new ArrayList(); @@ -201,10 +202,9 @@ protected void visitDataRecord(final Key key, final int foundAt, final int tail, final int marked = MVV.countMarkedVersions(bytes, offset, length); if (marked > 0) { _counters._markedVersionCount += marked; - if (addFault("MVV has " + plural(marked, "version") + " with a leftover mark bit", _currentPage, 0, - foundAt)) { - _markedVersionFaultCount++; - } + _markedVersionFaultCount++; + addFault("MVV has " + plural(marked, "version") + " with a leftover mark bit", _currentPage, 0, + foundAt); } if (_versionVisitor._count > 0) { _counters._mvvCount++; @@ -314,9 +314,13 @@ protected void runTask() { * affected by a pre-#286 interrupted prune but do not block * clearing the TransactionIndex: the prune pass above has * already rewritten those marks (issue #292). Only other - * faults and incomplete pruning count as failures here. + * faults and incomplete pruning count as failures here. The + * comparison uses fault counts, not the _faults list: the + * list is capped at MAX_FAULTS, so a genuine fault found + * after marked-version faults fill the cap would be missing + * from the list but must still block the clear. */ - if (_faults.size() == _markedVersionFaultCount && _counters._mvvPageCount == _counters._prunedPageCount + if (_faultCount == _markedVersionFaultCount && _counters._mvvPageCount == _counters._prunedPageCount && _counters._pruningErrorCount == 0) { final int count = _persistit.getTransactionIndex().resetMVVCounts(startTimestamp); postMessage(String.format("%,d aborted transactions were cleared by pruning", count), LOG_NORMAL); @@ -356,17 +360,17 @@ private String plural(final int n, final String m) { } } - private boolean addFault(final String description, final long page, final int level, final int position) { + private void addFault(final String description, final long page, final int level, final int position) { final Fault fault = new Fault(resourceName(), this, description, page, _treeDepth, level, position); - final boolean recorded = _faults.size() < MAX_FAULTS; - if (recorded) + _faultCount++; + if (_faults.size() < MAX_FAULTS) _faults.add(fault); postMessage(fault.toString(), LOG_VERBOSE); - return recorded; } private void addGarbageFault(final String description, final long page, final int level, final int position) { final Fault fault = new Fault(resourceName(), this, description, page, 3, level, position); + _faultCount++; if (_faults.size() < MAX_FAULTS) _faults.add(fault); postMessage(fault.toString(), LOG_VERBOSE); diff --git a/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java b/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java index 331e39986..4afaa5cde 100644 --- a/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java +++ b/persistit/core/src/test/java/com/persistit/IntegrityCheckTest.java @@ -101,6 +101,12 @@ public void testBrokenMVVs() throws Exception { final IntegrityCheck icheck = icheck(); icheck.checkTree(ex.getTree()); assertTrue(icheck.getFaults().length > 0); + /* + * A structurally corrupt MVV must not inflate the marked-version + * counter: the leftover-mark scan runs only after visitAllVersions + * has validated the record. + */ + assertEquals(0, icheck.getMarkedVersionCount()); } /** @@ -185,6 +191,34 @@ public void testPruneAndClearWithLeftoverMarkBits() throws Exception { assertFalse(output.toString(), output.toString().contains("PruneAndClear failed")); } + /** + * The -P gate must compare fault counts, not the MAX_FAULTS-capped fault + * list: with more than MAX_FAULTS marked-version faults the list holds + * only them, and a genuine fault found later (here a corrupt garbage + * chain, checked after the trees) is not recorded in the list - it must + * still block clearing the TransactionIndex. + */ + @Test + public void testPruneAndClearBlockedByFaultBeyondFaultCap() throws Exception { + final Exchange ex = _persistit.getExchange(_volumeName, "mvv", true); + disableBackgroundCleanup(); + transactionalStore(ex); + _persistit.getTransactionIndex().updateActiveTransactionCache(); + + assertTrue(corrupt5(ex, 0, IntegrityCheck.MAX_FAULTS + 1)[0] > IntegrityCheck.MAX_FAULTS); + corrupt4(ex); + + final IntegrityCheck icheck = (IntegrityCheck) CLI.parseTask(_persistit, "icheck trees=* -u -P"); + final StringWriter output = new StringWriter(); + icheck.setMessageWriter(new PrintWriter(output)); + icheck.setup(1, "icheck", "cli", 0, 5); + icheck.run(); + + assertTrue(icheck.getMarkedVersionCount() > 0); + assertTrue(output.toString(), output.toString().contains("PruneAndClear failed")); + assertFalse(output.toString(), output.toString().contains("aborted transactions were cleared by pruning")); + } + @Test public void testIndexFixHoles() throws Exception { final Exchange ex = _persistit.getExchange(_volumeName, "mvv", true); @@ -376,6 +410,7 @@ private interface MVVCorruption { /** * Applies a corruption to the raw bytes of up to ten stored MVV values + * following key 500 * * @param ex * @param corruption @@ -384,12 +419,29 @@ private interface MVVCorruption { * @throws PersistitException */ private int[] corruptMVVs(final Exchange ex, final MVVCorruption corruption) throws PersistitException { + return corruptMVVs(ex, 500, 10, corruption); + } + + /** + * Applies a corruption to the raw bytes of up to limit + * stored MVV values following startKey + * + * @param ex + * @param startKey + * @param limit + * @param corruption + * @return the number of values corrupted and the summed corruption + * contributions, as a two-element array + * @throws PersistitException + */ + private int[] corruptMVVs(final Exchange ex, final int startKey, final int limit, final MVVCorruption corruption) + throws PersistitException { ex.ignoreMVCCFetch(true); try { - ex.clear().to(key(500)); + ex.clear().to(key(startKey)); int corrupted = 0; int total = 0; - while (corrupted < 10 && ex.next()) { + while (corrupted < limit && ex.next()) { final byte[] bytes = ex.getValue().getEncodedBytes(); final int length = ex.getValue().getEncodedSize(); if (MVV.isArrayMVV(bytes, 0, length)) { @@ -460,7 +512,15 @@ private void corrupt4(final Exchange ex) throws PersistitException { * @throws PersistitException */ private int[] corrupt5(final Exchange ex) throws PersistitException { - return corruptMVVs(ex, (bytes, length) -> { + return corrupt5(ex, 500, 10); + } + + /** + * Same as {@link #corrupt5(Exchange)} for up to limit values + * following startKey + */ + private int[] corrupt5(final Exchange ex, final int startKey, final int limit) throws PersistitException { + return corruptMVVs(ex, startKey, limit, (bytes, length) -> { int marked = 0; int from = 1; while (from + MVV.LENGTH_PER_VERSION <= length) {