Raised in code review on PR #6299 and left as a follow-up there.
What happens
#6294 gave the orphaned-chunk sweep of LocalBucket.check(fix=true) a memory budget
(ORPHAN_RECLAIM_BUDGET_BYTES, 32 MB of pages): the enclosing transaction keeps a copy of every page it
modifies, so a large backlog freed in one transaction is how a repair turns into the OutOfMemoryError of
#4653. A run frees what fits, reports the rest through the gap between orphanedChunks and
orphanedChunksReclaimed, and the next FIX continues.
The record repairs in the very same method are not bounded the same way. Four sites call
deleteRecordInternal(rid, true, true, true) in the per-slot loop:
- the invalid record offset branch,
- the broken chunk chain branch,
- the
endPosition > pageSize branch,
- the generic
catch (Exception) fallback,
and each one takes its record's page (plus, for a chain, every page the chain touches) for modification in the
same transaction. A bucket with a large number of broken records hits exactly the failure the budget next
door exists to prevent, just through a different loop.
The same applies to the dangling-pointer repair #6292 added, which frees one slot per corrupted pointer.
Why it was not folded into #6299
The risk profiles differ, and that is worth stating rather than assuming:
- orphaned chunks accumulate in normal operation - see the leak filed as a companion to this - so a
backlog of hundreds of thousands is expected rather than exceptional;
- broken records are corruption, so the count is bounded by how much corruption a database has;
- and capping record deletion changes what a repair run leaves behind, which is a semantic decision about
CHECK DATABASE FIX rather than a memory fix. A record left un-repaired is still unreadable and still
throws on every access, where a chunk left un-reclaimed is only space.
Worth considering
The pattern already exists right next to it: count the distinct pages taken for modification, stop at a byte
budget derived from pageSize, report what was left, and let the next run continue. The two budgets should
probably be one - what is scarce is the transaction's page memory, and both repairs spend it - which argues
for a single accounting shared by every repair check(fix) makes rather than a second constant.
Whether a partially-repaired run is acceptable for records is the design question to settle first. It may be
that the honest answer is to keep record repair unbounded and instead refuse to start when the number of
records to repair would exceed the budget, telling the operator to raise the heap - a repair that stops
half-way through a record is worse than one that declines to start.
Raised in code review on PR #6299 and left as a follow-up there.
What happens
#6294 gave the orphaned-chunk sweep of
LocalBucket.check(fix=true)a memory budget(
ORPHAN_RECLAIM_BUDGET_BYTES, 32 MB of pages): the enclosing transaction keeps a copy of every page itmodifies, so a large backlog freed in one transaction is how a repair turns into the
OutOfMemoryErrorof#4653. A run frees what fits, reports the rest through the gap between
orphanedChunksandorphanedChunksReclaimed, and the nextFIXcontinues.The record repairs in the very same method are not bounded the same way. Four sites call
deleteRecordInternal(rid, true, true, true)in the per-slot loop:endPosition > pageSizebranch,catch (Exception)fallback,and each one takes its record's page (plus, for a chain, every page the chain touches) for modification in the
same transaction. A bucket with a large number of broken records hits exactly the failure the budget next
door exists to prevent, just through a different loop.
The same applies to the dangling-pointer repair #6292 added, which frees one slot per corrupted pointer.
Why it was not folded into #6299
The risk profiles differ, and that is worth stating rather than assuming:
backlog of hundreds of thousands is expected rather than exceptional;
CHECK DATABASE FIXrather than a memory fix. A record left un-repaired is still unreadable and stillthrows on every access, where a chunk left un-reclaimed is only space.
Worth considering
The pattern already exists right next to it: count the distinct pages taken for modification, stop at a byte
budget derived from
pageSize, report what was left, and let the next run continue. The two budgets shouldprobably be one - what is scarce is the transaction's page memory, and both repairs spend it - which argues
for a single accounting shared by every repair
check(fix)makes rather than a second constant.Whether a partially-repaired run is acceptable for records is the design question to settle first. It may be
that the honest answer is to keep record repair unbounded and instead refuse to start when the number of
records to repair would exceed the budget, telling the operator to raise the heap - a repair that stops
half-way through a record is worse than one that declines to start.