Follow-ups left behind by #5680, whose two halves landed separately: #5707 made deleteVertex strict, and #5710 added the CHECK DATABASE RECORD <rid> scope. Each was reviewed against the state of main at the time, so a few seams between them were never closed. None is a defect in either merge; they are refinements the split made easy to miss.
Ordered by value.
1. The repair advice never names the scoped command it now has
Every place deleteVertex tells an operator how to recover predates the RECORD scope, so they all advise the whole-database (or whole-type) form:
GraphEngine.java:739 — "the reference survives, run a database check to repair it"GraphEngine.java:939 — "...edges behind the damage survive - run a database check to repair them"GraphEngine.java:921 (comment) — "CHECK DATABASE ... FIX rebuilds the chain from the surviving edge records"- and the retryable path at
:902 rethrows the ConcurrentModificationException bare, so what the operator sees is getEdgeHeadChunkForWrite's message, which does not mention recovery at all.
This is the case the RECORD scope was added for. A vertex whose chain is genuinely broken is undeletable until the adjacency is rebuilt, and the operator holds the one piece of information that makes the repair cheap — the RID — while being pointed at a run that costs two full passes over the vertex type plus an edge sweep.
Suggested: name the command with the RID substituted, e.g.
run `CHECK DATABASE RECORD #12:3 FIX` to rebuild the edge list from the surviving edge records, then retry the delete
Worth stating the limit in the same breath, since the scope does not bound everything: the edge sweep still runs once per distinct vertex type named.
2. ConcurrentModificationException has no (message, cause) constructor
NeedRetryException already declares one; ConcurrentModificationException exposes only (String). Six sites in GraphEngine alone construct one from a caught exception (:411, :825, :829, :843, :965, :1530) and each drops the original stack.
A retryable conflict is normally absorbed and never seen, so the single run where one does surface is the retry-exhausted one — i.e. exactly the run whose stack trace has to be diagnosable, and the one that currently arrives with the cause discarded.
One-line change; the plumbing already exists on the parent.
3. checkDocuments and checkScopedDocuments report different totals for the same finding
DatabaseChecker.checkScopedDocuments (the RECORD arm) increments totalWarnings and totalCorruptedRecords; the type-wide checkDocuments adds to the warnings/corruptedRecords sets without touching either total. So a corrupt document reports different numbers depending on which path found it.
The scoped arm is the correct one. It was left divergent on purpose in #5710 — aligning the type-wide path changes what CHECK DATABASE TYPE ... reports for every user, which was outside that PR — and the divergence is recorded in the Javadoc. Worth settling deliberately rather than leaving as a known difference.
While there: the type-wide arm's message reads "vertex <rid> cannot be loaded, removing it" for a document, and nothing removes it — corruptedRecords only drives the index rebuild. The scoped arm was corrected to "document <rid> cannot be loaded"; the type-wide one still says both wrong things.
4. The scoped vertex check runs one pass where the type-wide runs two
Type-wide checkVertices does a bucket scan that materialises each record from the raw page view (newImmutableRecord + asVertex(true)) and a connectivity walk. The RECORD-scoped arm runs only the connectivity walk, materialising through lookupByRID.
Equivalent for every corruption shape currently tested — checkDatabaseRecordFixRebuildsTheBucketIndexesOfAGenuinelyCorruptedRecord covers a record whose buffer is truncated below the 25-byte vertex prefix — but a shape that surfaces only through raw-view materialisation would be missed by a scoped run and caught by a type-wide one. Either confirm no such shape exists, or add the pass to the scoped enumeration.
Note on a non-issue
deleteVertex tolerating an undecodable chunk (GraphEngine.java:909-940) — swallowing the decode family even when force == false, deleting the vertex and losing the edges behind the damage — is correct and should stay. The reasoning in that comment holds: LocalDatabase.deleteRecordNoLock catches the same family around the index cleanup and proceeds without raising its force flag, so a vertex with a corrupt buffer reaches deleteVertex with force == false, and failing there would recreate the "records that can't be deleted" complaint from #4420/#4432. Recording it here so it is not "fixed" by mistake later; only the advice it logs (item 1) needs improving.
Patches for items 1 and 2 exist from #5710's pre-rescope branch and can be reproduced on request.
Follow-ups left behind by #5680, whose two halves landed separately: #5707 made
deleteVertexstrict, and #5710 added theCHECK DATABASE RECORD <rid>scope. Each was reviewed against the state ofmainat the time, so a few seams between them were never closed. None is a defect in either merge; they are refinements the split made easy to miss.Ordered by value.
1. The repair advice never names the scoped command it now has
Every place
deleteVertextells an operator how to recover predates theRECORDscope, so they all advise the whole-database (or whole-type) form:GraphEngine.java:739—"the reference survives, run a database check to repair it"GraphEngine.java:939—"...edges behind the damage survive - run a database check to repair them"GraphEngine.java:921(comment) —"CHECK DATABASE ... FIX rebuilds the chain from the surviving edge records":902rethrows theConcurrentModificationExceptionbare, so what the operator sees isgetEdgeHeadChunkForWrite's message, which does not mention recovery at all.This is the case the
RECORDscope was added for. A vertex whose chain is genuinely broken is undeletable until the adjacency is rebuilt, and the operator holds the one piece of information that makes the repair cheap — the RID — while being pointed at a run that costs two full passes over the vertex type plus an edge sweep.Suggested: name the command with the RID substituted, e.g.
Worth stating the limit in the same breath, since the scope does not bound everything: the edge sweep still runs once per distinct vertex type named.
2.
ConcurrentModificationExceptionhas no(message, cause)constructorNeedRetryExceptionalready declares one;ConcurrentModificationExceptionexposes only(String). Six sites inGraphEnginealone construct one from a caught exception (:411,:825,:829,:843,:965,:1530) and each drops the original stack.A retryable conflict is normally absorbed and never seen, so the single run where one does surface is the retry-exhausted one — i.e. exactly the run whose stack trace has to be diagnosable, and the one that currently arrives with the cause discarded.
One-line change; the plumbing already exists on the parent.
3.
checkDocumentsandcheckScopedDocumentsreport different totals for the same findingDatabaseChecker.checkScopedDocuments(theRECORDarm) incrementstotalWarningsandtotalCorruptedRecords; the type-widecheckDocumentsadds to thewarnings/corruptedRecordssets without touching either total. So a corrupt document reports different numbers depending on which path found it.The scoped arm is the correct one. It was left divergent on purpose in #5710 — aligning the type-wide path changes what
CHECK DATABASE TYPE ...reports for every user, which was outside that PR — and the divergence is recorded in the Javadoc. Worth settling deliberately rather than leaving as a known difference.While there: the type-wide arm's message reads
"vertex <rid> cannot be loaded, removing it"for a document, and nothing removes it —corruptedRecordsonly drives the index rebuild. The scoped arm was corrected to"document <rid> cannot be loaded"; the type-wide one still says both wrong things.4. The scoped vertex check runs one pass where the type-wide runs two
Type-wide
checkVerticesdoes a bucket scan that materialises each record from the raw page view (newImmutableRecord+asVertex(true)) and a connectivity walk. TheRECORD-scoped arm runs only the connectivity walk, materialising throughlookupByRID.Equivalent for every corruption shape currently tested —
checkDatabaseRecordFixRebuildsTheBucketIndexesOfAGenuinelyCorruptedRecordcovers a record whose buffer is truncated below the 25-byte vertex prefix — but a shape that surfaces only through raw-view materialisation would be missed by a scoped run and caught by a type-wide one. Either confirm no such shape exists, or add the pass to the scoped enumeration.Note on a non-issue
deleteVertextolerating an undecodable chunk (GraphEngine.java:909-940) — swallowing the decode family even whenforce == false, deleting the vertex and losing the edges behind the damage — is correct and should stay. The reasoning in that comment holds:LocalDatabase.deleteRecordNoLockcatches the same family around the index cleanup and proceeds without raising its force flag, so a vertex with a corrupt buffer reachesdeleteVertexwithforce == false, and failing there would recreate the "records that can't be deleted" complaint from #4420/#4432. Recording it here so it is not "fixed" by mistake later; only the advice it logs (item 1) needs improving.Patches for items 1 and 2 exist from #5710's pre-rescope branch and can be reproduced on request.