Retain superseded blob files long enough for in-flight readers

[kriszyp]

Blob files are unlinked on record supersession plus a fixed 500ms timer, with nothing checking whether anything still needs the bytes. The file is opened lazily, by path, when a consumer calls stream()/bytes() — not when the record is decoded — so a reader that resolved a record just before a concurrent write opens a file that is already gone. On the HTTP path that ENOENT is raised after the response headers are committed, so it reaches the client as a truncated body while every signal reports success. deleteBlob's own comment described the gap: "we need to determine when any read transaction are done with the file, and then delete it, this is a hack to just give it some time for that."

• The delay is now configurable and defaults to 2s (storage.blobRetention), an order of magnitude above a value that was too short for a slow or backpressured response. The cost is only the churn produced within the window; 0 reclaims as soon as the queue drains.
• holdBlobFile(blob) retains a file for a consumer whose need for the bytes outlives the record version that referenced it — primarily replication, where a peer that has not yet fetched a superseded blob gets a clean 404 from the sender, classifies it as unrecoverable at source, and advances its resume cursor past a record whose bytes it will never have (Blob replication wedges permanently when source blob is gone (ENOENT) on an expiration cache table — held resume cursor never recovers harper-pro#403/Tolerate missing role on user #388). Holds are counted in the store's shared buffer with Atomics — the same mechanism the blob file-id allocator in this file already uses — because the consumer and the write that supersedes it routinely run on different workers. Concurrent holders on different workers each keep the file alive; a reclaimer takes the count from 0 to a RECLAIMING sentinel in one compareExchange, so a hold cannot be acquired between "no holders" and the unlink, and a hold arriving after the claim returns null instead of falsely reporting success.
• Cancellation: a record version that references a file again sets a shared flag the reclaiming worker consumes, so a re-reference on any worker cancels the reclamation — the queue itself is per worker. The existing retainedFileIds check covers only the write that supersedes, not a file already queued by an earlier one.
• Open read snapshots: a blob reference is fixed when the reader's snapshot is taken, not when the record is decoded, so a reader inside a transaction is entitled to the bytes while its snapshot lives. getOldestSnapshotTimestamp() reports the oldest unreleased snapshot, and a queued file whose supersession is not yet behind it stays. Reads outside a transaction take no snapshot ("no transaction means read latest", DatabaseTransaction.getReadTxn), and LMDB exposes no watermark — both fall through to the window.
• Read holds: stream() retains the file for the life of the read and releases on completion, cancel, or a failed open, so a slow or backpressured response cannot have its bytes reclaimed mid-read.
• Age cap: a held file is reclaimed after 20 minutes anyway (logged) — above the replication blob timeout, so it cannot expire under a send that is still legitimately running — meaning an unreleased hold cannot pin a file forever. The queue entry survives until its unlink lands, so a concurrent re-reference can tell the file is going away.

Pending reclamations are keyed by file path rather than fileId, which is a per-store counter two databases can collide on. The shared hold state is one fixed 4096-slot table per store rather than a buffer per fileId: getUserSharedBuffer documents no eviction and cancel() only detaches a listener, so a key per blob would grow without bound on a churning table. Slots are shared by hash, which can only ever over-retain — a collision defers someone else's reclamation, it never unlinks early — and the reclaim claim is handed back after the unlink so a shared slot cannot stay poisoned.

Reported in #2134 as progressive page_cache blob loss on a 4-node cluster. That report attributes the loss to the unmerged retain-on-invalidate branch from #1302; that diagnosis does not hold — an invalidated record no longer references the blob at all, which the reporter's own cleanupOrphans run demonstrates by finding the retained file as an orphan — so this addresses the lifetime rule underneath instead.

Paired with HarperFast/harper-pro#687, the first caller of the hold API. Docs: HarperFast/documentation#622.

Where to look

resources/blob.ts — runReclamation is the whole policy in one function; read it first, then deleteBlob, holdBlobFile, and isBlobHeld.

The first review of this change found a blocker worth knowing about while reading: the original version tracked reader references and holds in module-level Maps, which are per-worker-thread, so neither signal reached the thread doing the unlinking — it passed its unit tests while protecting nothing in production. The reference tracking is gone (a time window needs no cross-thread signal) and holds moved to the store lock. a hold taken on another thread defers reclamation is the regression test for that.

Verification

Route: extended unit tests in unitTests/resources/blob.test.js, plus the repo's full unit gates.

• npx mocha unitTests/resources/blob.test.js — 70 passing on RocksDB, 69 + 1 skipped on LMDB (the snapshot case skips itself where no watermark exists). Each new case verified to fail without its fix, including forcing the stream hold to null and disabling the snapshot check.
• Fails-on-base check (git checkout origin/main -- resources/blob.ts utility/hdbTerms.ts, rm tsconfig.build.tsbuildinfo, rebuild — an incremental build silently keeps the new dist and invalidates this check): all three new tests fail, with superseded blob must be retained while a reader still references it, holdBlobFile is not a function, and a blob held by another thread's lock must not be reclaimed. The reader test exercises the shipped default rather than a compressed knob, which is what makes it meaningful — driving it through setDeletionDelay would pass on base, since on base that knob is the delay.
• npm run test:unit:main — 4355 passing, 0 failing.
• npm run test:unit:resources — 1512 passing, 3 failing; all three (randomAccessFields ×2, replayStructures) reproduce identically on a clean origin/main tree with the same dependencies, so they are pre-existing here, not regressions.
• Paired harper-pro branch, HARPER_RUN_STRESS_TESTS=1 npm run test:integration over the blob replication suites: Receive-side blob save rejection containment and Receive-side blob resume-cursor clamp on transient save failure both pass against this core. Authoritative-table blob byte-integrity after receive-side save failure fails identically on a clean baseline (local ECONNREFUSED during a restart-phase deploy).
• Not run: test:integration:all — CI covers it.

Open concerns from review

• Four review passes ran against this change (Codex graded leg + Harper domain adjudication each time; Gemini contributed on three of the four and produced no output on one; both Cursor legs were pruned by policy). The first pass found the thread-locality blocker; the later passes converged on the design decisions listed below rather than new defects.
• Cancellation happens at encode, not at commit, so an aborted write leaves the file unreclaimed until the orphan sweeper runs. That is the deliberate direction: the alternative failure mode is unlinking a file the committed record still points at.
• Nothing exercises the age cap or a cross-database fileId collision end-to-end.

For the human reviewer

Seven judgment calls, ranked by what a "no" costs. Each is answerable from the entry alone.

1. What the window is still load-bearing for. Reclamation now defers on open snapshots (exact for transactional reads), on holds (exact for streams and replication sends), and only then on time. The window is what remains for a bare read that takes no snapshot, and for LMDB, which exposes no watermark. Whether that residual is acceptable — or whether LMDB warrants an equivalent watermark upstream — is the call.
2. Cancellation happens at encode, not at commit. An aborted write therefore leaks a file until the orphan sweeper runs — tracked as Blob reclamation cancelled at encode time leaks the file when the write aborts #2156, with the transactional fix sketched there. Cancelling at commit instead makes aborts clean but opens a window where a file the committed record points at gets unlinked, so this takes the reclaimable leak over the dangling reference. (The cross-worker half of this is resolved — the re-reference signal is shared, so the worker that queued the reclamation sees it.) A "no" here means picking the other failure mode, not removing the choice.
3. A hold is a lease on a binary lock. Resolved. Holds are an atomic count in the store's shared buffer, so concurrent holders on different workers are independent and a hold means what its name says. What remains for review is the table: 4096 slots per store, hashed, collisions over-retain only.
4. The 20-minute age cap deletes bytes something still claims to need (logged), rather than pinning forever and relying on a restart. Bounded disk over guaranteed bytes. The constant is trivial to change; the policy is the question.
5. 2000ms as the shipped default. 4× the old window: it widens the crash-orphan window and the steady-state disk overhang by the same factor. A config default, trivially reversible — but it is what makes the fix work out of the box rather than only for operators who tune it.
6. One process-global queue across all databases and stores, rather than per-store queues. This is what makes the cross-worker cancel gap and the tail-deadline clamp possible in the first place. Refactoring later touches every entry point in this file.
7. holdBlobFile has no failure signal. Resolved. It returns null when reclamation has already claimed the file, so the caller is told rather than misled.

_{Human-Review-Need: 4 (decisions: retention-default-2s, binary-lock-vs-refcount, cancel-at-encode-not-commit, age-cap-deletes-held-bytes, ship-hold-api-before-caller, worker-local-queue, retention-config-shape) @ d747a82}

[gemini-code-assist]

Code Review

This pull request replaces the fixed 500ms blob deletion delay with a robust, queue-based reclamation system. It introduces configurable retention delays, explicit retention holds, and cross-thread lock-backed hold checks, along with comprehensive unit tests. The reviewer identified three high-severity concurrency and lifecycle issues: first, the binary lock key for holding blobs does not safely count holders across multiple worker threads; second, cancelBlobReclamation only cancels reclamation on the local worker, allowing other workers to prematurely unlink re-referenced files; and third, rescheduling the reclamation queue fails to propagate the keepProcessAlive flag, which can cause short-lived processes to exit prematurely and leak files.

[claude]

Blocker still open (re-verified against current HEAD, c66e09ef5 — no commits to resources/blob.ts since it was flagged): the shared blob-hold table's hash collision remains unfixed. cancelBlobReclamation's early-return branch (resources/blob.ts:1062-1069, Atomics.load(...) < 0) skips both Atomics.store(..., REREFERENCED, 1) and the local pendingReclamation.delete(filePath) whenever the fileId being re-referenced shares its hashed slot with a different fileId that's currently mid-reclaim (RECLAIMING). That silently drops the cancellation signal for the colliding file, so its queued unlink still fires on schedule even though a write just referenced it again — reproducing the exact cross-worker data-loss bug this PR exists to fix, just triggered by a hash collision instead of the original timing gap. Open inline thread on that line has the full trace and a suggested fix (tag-based slot disambiguation); still unresolved with no author reply.

[claude]

Hash collision on the shared hold slot still unlinks a re-referenced file early, despite the thread being marked resolved

File: resources/blob.ts:1038-1080 (isBlobHeld / cancelBlobReclamation), root cause in blobHoldSlot (line 938)

What: This is the same defect raised in the now-resolved thread on this line (comment id 3762003686) — re-checked against the current code, and it's still present verbatim, with no tag/disambiguation added. HOLD_TABLE_SLOTS is a fixed 4096-slot table shared by hash (blobHoldSlot), and fileId is a per-store sequential counter, so two unrelated fileIds routinely land on the same slot. While a reclaimer holds slot S at RECLAIMING for fileId A (the window between the compareExchange claim in isBlobHeld and releaseReclaimClaim in the unlink callback), a write referencing a colliding fileId B hits the early-return branch in cancelBlobReclamation (Atomics.load(...) < 0 → logger.warn + return). That return skips Atomics.store(..., REREFERENCED, 1) and the local pendingReclamation.delete(filePath) below it — so if B's own reclamation was queued (by this worker or another), nothing cancels it. B's queued unlink still fires on schedule even though a new record now references file B again.

Why it matters: This reproduces the exact bug class the PR sets out to fix (harper-pro#403/#388 — a peer treats a superseded-but-still-referenced blob as gone and permanently loses the bytes), just triggered by an unrelated hash collision instead of the original timing gap. It also contradicts the table's own documented invariant a few lines up ("a collision defers someone else's reclamation, it never unlinks early") — this path unlinks a referenced file early. The same misattribution makes holdBlobFile spuriously return null for a live, unrelated file whenever its hash collides with a fileId that's mid-reclaim; that null is documented (and consumed by the paired replication caller) as "already being reclaimed," which isn't true for the collision case.

Suggested fix: Disambiguate collisions — e.g. store a tag derived from the owning fileId alongside the counter and treat a slot as applicable only on a tag match, falling back to the safe default (not held / not reclaiming) on mismatch.