Skip to content

Commit 39ddc41

Browse files
authored
[#290] Report leftover MVV mark bits in IntegrityCheck (#291)
* 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 * 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.
1 parent 19c9d70 commit 39ddc41

5 files changed

Lines changed: 337 additions & 23 deletions

File tree

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

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public class IntegrityCheck extends Task {
6060

6161
private Volume _currentVolume;
6262
private Tree _currentTree;
63+
private long _currentPage;
6364
private LongBitSet _usedPageBits = new LongBitSet();
6465
private long _totalPages = 0;
6566
private long _pagesVisited = 0;
@@ -79,6 +80,8 @@ public class IntegrityCheck extends Task {
7980
private boolean _csv;
8081

8182
private final ArrayList<Fault> _faults = new ArrayList<Fault>();
83+
private int _faultCount;
84+
private int _markedVersionFaultCount;
8285
private final ArrayList<CleanupIndexHole> _holes = new ArrayList<CleanupIndexHole>();
8386

8487
// Used in checking long values
@@ -98,6 +101,7 @@ private static class Counters {
98101
private long _mvvCount = 0;
99102
private long _mvvOverhead = 0;
100103
private long _mvvAntiValues = 0;
104+
private long _markedVersionCount = 0;
101105
private long _pruningErrorCount = 0;
102106
private long _prunedPageCount = 0;
103107
private long _garbagePageCount = 0;
@@ -118,6 +122,7 @@ private static class Counters {
118122
_mvvCount = counters._mvvCount;
119123
_mvvOverhead = counters._mvvOverhead;
120124
_mvvAntiValues = counters._mvvAntiValues;
125+
_markedVersionCount = counters._markedVersionCount;
121126
_pruningErrorCount = counters._pruningErrorCount;
122127
_prunedPageCount = counters._prunedPageCount;
123128
_garbagePageCount = counters._garbagePageCount;
@@ -135,6 +140,7 @@ void difference(final Counters counters) {
135140
_mvvCount = counters._mvvCount - _mvvCount;
136141
_mvvOverhead = counters._mvvOverhead - _mvvOverhead;
137142
_mvvAntiValues = counters._mvvAntiValues - _mvvAntiValues;
143+
_markedVersionCount = counters._markedVersionCount - _markedVersionCount;
138144
_pruningErrorCount = counters._pruningErrorCount - _pruningErrorCount;
139145
_prunedPageCount = counters._prunedPageCount - _prunedPageCount;
140146
_garbagePageCount = counters._garbagePageCount - _garbagePageCount;
@@ -144,20 +150,21 @@ void difference(final Counters counters) {
144150
public String toString() {
145151
return String.format("Index pages/bytes: %,d / %,d Data pages/bytes: %,d / %,d"
146152
+ " LongRec pages/bytes: %,d / %,d MVV pages/records/bytes/antivalues: "
147-
+ "%,d / %,d / %,d / %,d Holes %,d Pages pruned %,d", _indexPageCount, _indexBytesInUse,
148-
_dataPageCount, _dataBytesInUse, _longRecordPageCount, _longRecordBytesInUse, _mvvPageCount,
149-
_mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount);
153+
+ "%,d / %,d / %,d / %,d Holes %,d Pages pruned %,d Marked versions %,d", _indexPageCount,
154+
_indexBytesInUse, _dataPageCount, _dataBytesInUse, _longRecordPageCount, _longRecordBytesInUse,
155+
_mvvPageCount, _mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount,
156+
_markedVersionCount);
150157
}
151158

152159
private String toCSV() {
153-
return String.format("%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d", _indexPageCount, _indexBytesInUse,
160+
return String.format("%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d,%d", _indexPageCount, _indexBytesInUse,
154161
_dataPageCount, _dataBytesInUse, _longRecordPageCount, _longRecordBytesInUse, _mvvPageCount,
155-
_mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount);
162+
_mvvCount, _mvvOverhead, _mvvAntiValues, _indexHoleCount, _prunedPageCount, _markedVersionCount);
156163
}
157164

158165
private final static String CSV_HEADERS = "IndexPages,IndexBytes,"
159166
+ "DataPages,DataBytes,LongRecordPages,LongRecordBytes,MvvPages,"
160-
+ "MvvRecords,MvvOverhead,MvvAntiValues,IndexHoles,PrunedPages";
167+
+ "MvvRecords,MvvOverhead,MvvAntiValues,IndexHoles,PrunedPages,MarkedVersions";
161168

162169
}
163170

@@ -186,6 +193,19 @@ public void sawVersion(final long version, final int offset, final int valueLeng
186193
protected void visitDataRecord(final Key key, final int foundAt, final int tail, final int klength,
187194
final int offset, final int length, final byte[] bytes) throws PersistitException {
188195
MVV.visitAllVersions(_versionVisitor, bytes, offset, length);
196+
/*
197+
* Scanned only after visitAllVersions has validated the version
198+
* structure. Leftover prune marks read normally (the length
199+
* accessors strip the mark bit), so only this dedicated scan can
200+
* detect and report them - see issue #290.
201+
*/
202+
final int marked = MVV.countMarkedVersions(bytes, offset, length);
203+
if (marked > 0) {
204+
_counters._markedVersionCount += marked;
205+
_markedVersionFaultCount++;
206+
addFault("MVV has " + plural(marked, "version") + " with a leftover mark bit", _currentPage, 0,
207+
foundAt);
208+
}
189209
if (_versionVisitor._count > 0) {
190210
_counters._mvvCount++;
191211
final int voffset = _versionVisitor._lastOffset;
@@ -289,7 +309,18 @@ protected void runTask() {
289309
postMessage("Total " + toString(), LOG_NORMAL);
290310
}
291311
if (_pruneAndClear) {
292-
if (_faults.isEmpty() && _counters._mvvPageCount == _counters._prunedPageCount
312+
/*
313+
* Leftover mark-bit faults (issue #290) identify volumes
314+
* affected by a pre-#286 interrupted prune but do not block
315+
* clearing the TransactionIndex: the prune pass above has
316+
* already rewritten those marks (issue #292). Only other
317+
* faults and incomplete pruning count as failures here. The
318+
* comparison uses fault counts, not the _faults list: the
319+
* list is capped at MAX_FAULTS, so a genuine fault found
320+
* after marked-version faults fill the cap would be missing
321+
* from the list but must still block the clear.
322+
*/
323+
if (_faultCount == _markedVersionFaultCount && _counters._mvvPageCount == _counters._prunedPageCount
293324
&& _counters._pruningErrorCount == 0) {
294325
final int count = _persistit.getTransactionIndex().resetMVVCounts(startTimestamp);
295326
postMessage(String.format("%,d aborted transactions were cleared by pruning", count), LOG_NORMAL);
@@ -331,13 +362,15 @@ private String plural(final int n, final String m) {
331362

332363
private void addFault(final String description, final long page, final int level, final int position) {
333364
final Fault fault = new Fault(resourceName(), this, description, page, _treeDepth, level, position);
365+
_faultCount++;
334366
if (_faults.size() < MAX_FAULTS)
335367
_faults.add(fault);
336368
postMessage(fault.toString(), LOG_VERBOSE);
337369
}
338370

339371
private void addGarbageFault(final String description, final long page, final int level, final int position) {
340372
final Fault fault = new Fault(resourceName(), this, description, page, 3, level, position);
373+
_faultCount++;
341374
if (_faults.size() < MAX_FAULTS)
342375
_faults.add(fault);
343376
postMessage(fault.toString(), LOG_VERBOSE);
@@ -544,6 +577,16 @@ public long getMvvAntiValues() {
544577
return _counters._mvvAntiValues;
545578
}
546579

580+
/**
581+
* @return Count of MVV versions whose length field still carries a
582+
* leftover prune mark bit, left behind by a prune interrupted
583+
* before the issue #286 fix. Such versions read normally but
584+
* indicate the volume was corrupted; pruning rewrites them.
585+
*/
586+
public long getMarkedVersionCount() {
587+
return _counters._markedVersionCount;
588+
}
589+
547590
/**
548591
* @return Count of pages for which an expected index pointer is missing
549592
*/
@@ -1144,6 +1187,7 @@ private Buffer walkRight(final int level, final long toPage, final Key key, fina
11441187
}
11451188

11461189
private boolean verifyPage(final Buffer buffer, final long page, final int level, final Key key, final Tree tree) {
1190+
_currentPage = page;
11471191
if (buffer.getPageAddress() != page) {
11481192
addFault("Buffer contains wrong page " + buffer.getPageAddress(), page, level, 0);
11491193
return false;

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,37 @@ static boolean verify(final byte[] bytes, final int offset, final int length) {
654654
return true;
655655
}
656656

657+
/**
658+
* Count versions whose length field still carries the mark bit. Marks are
659+
* transient state private to {@link #prune} and must never be observable
660+
* outside it; a non-zero count on a stored MVV means the value was
661+
* corrupted by a prune interrupted before the issue #286 fix. The length
662+
* accessors strip the mark bit silently, so such versions read normally
663+
* and only this scan can detect them (issue #290).
664+
*
665+
* @param bytes
666+
* the byte array
667+
* @param offset
668+
* the index of the first byte of the MVV within the byte array
669+
* @param length
670+
* the count of bytes in the MVV
671+
* @return the number of marked versions, 0 for a non-MVV value
672+
*/
673+
static int countMarkedVersions(final byte[] bytes, final int offset, final int length) {
674+
if (!isArrayMVV(bytes, offset, length)) {
675+
return 0;
676+
}
677+
int marked = 0;
678+
int from = offset + 1;
679+
while (from + LENGTH_PER_VERSION <= offset + length) {
680+
if (isMarked(bytes, from)) {
681+
marked++;
682+
}
683+
from += getLength(bytes, from) + LENGTH_PER_VERSION;
684+
}
685+
return marked;
686+
}
687+
657688
static boolean verify(final TransactionIndex ti, final byte[] bytes, final int offset, final int length) {
658689
if (!isArrayMVV(bytes, offset, length)) {
659690
/*

0 commit comments

Comments
 (0)