HarperFast / HarperFast/harper
Blob reclamation cancelled at encode time leaks the file when the write aborts
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Gap
`cancelBlobReclamation` (added in #2145) drops a queued blob reclamation when a record version being written references the file again. It runs during msgpackr encoding — **before** the write commits — so an aborted transaction leaves the file both unreferenced and unqueued.
Sequence:
1. Write A supersedes blob `F`. On commit, `F` is queued for reclamation.
2. Write B encodes a record that references `F` again. `cancelBlobReclamation` removes the queued entry at encode time.
3. B aborts. `F` is referenced by no live record (A's supersession stands) and is no longer queued for reclamation.
`F` then survives until someone runs `cleanup_orphan_blobs`. No data is lost — this is a reclaimable leak, not a dangling reference — but the file is retained indefinitely without operator action.
## Why it was left this way
Cancelling at commit instead of at encode would close the leak but open a worse window: between encode and commit, the reclaimer could unlink `F` while B is committing a record that points at it, producing exactly the dangling reference #2145 exists to prevent. Given the choice between a reclaimable leak on abort and a dangling reference on commit, #2145 took the leak.
## Suggested fix
Make the cancellation transactional rather than speculative: have the encoder record which reclamations it cancelled for a given write, and re-queue them from the transaction's abort path. That mirrors how newly-written blobs are already handled on abort (`cleanupUnusedBlobs(write.savedBlobs, …)` in `LMDBTransaction.ts:277`/`:323`), so the lifecycle hook already exists — this is plumbing from the encoder into it, not a new mechanism.
Re-queueing (rather than unlinking directly) is the safe direction: the file returns to the normal reclamation path, where the snapshot watermark, holds, and retention window all still apply before anything is unlinked.
## Scope
- Narrow: requires a write that re-references a blob already superseded by an earlier committed write, and that write then aborting.
- No data-loss risk in either the current behavior or the fix.
- Test shape: supersede a blob, begin a write that re-references it, abort, then assert the file is reclaimed without an orphan sweep.
## Related
- #2145 — the PR that introduced the cancellation (see `cancelBlobReclamation` in `resources/blob.ts` and its docstring, which records this trade-off).
- #1364 — the same class of "only act once the write commits" reasoning, applied to `removeEntry`.
🤖 Generated by Claude (claude-opus-5)
Contributor guide
Research direction
Start with cancelBlobReclamation and its docstring in resources/blob.ts, then trace how the encoder's cancelled reclamations can reach LMDBTransaction.ts abort handling, especially cleanupUnusedBlobs at lines 277 and 323. Add the regression test described in the issue: supersede a blob, re-reference it in a write, abort, and verify it is reclaimed without an orphan sweep.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100