Apply audit retention continuously to RocksDB transaction logs

[kriszyp]

RocksDB audit retention now runs on the existing self-rearming cleanup cadence instead of depending on startup or disk-pressure signals, so eligible transaction-log segments are reclaimed continuously according to logging.auditRetention. The Rocks delay is a pure function of the pressure-adjusted retention window (a tenth of it, floored at DEFAULT_AUDIT_CLEANUP_DELAY) rather than LMDB's adaptive backoff: segment eligibility only changes on rotation/flush, so a per-entry delete count would only make it rescan the same files.

Fixes #846 — Transaction-log retention has no config-driven time/size control (and the storageReclamation hardening isn't on v5.1). Also fixes #2140, closed as a duplicate.

Retiring the loop is two things, and teardown needs both. stopAuditCleanup() latches the loop closed and returns a drain barrier — the promise that settles once the pass already running has finished. The barrier is what makes closing safe: lmdb-js stamps the DBI number into its write instruction synchronously and the native writer consumes it later, so a pass suspended inside await removeAuditEntry() still has a delete pending against the primary and audit DBIs, and LMDB forbids closing a DBI an open transaction has modified. dropDatabase() and the legacy arm of Table.dropTable() await it. closeDatabase() and branch close() are synchronous and cannot; what covers them is that every environment touch remaining in a resumed pass — cursor advance, cursor release, marker write, re-arm — re-checks rootStore.status, plus the fact that their production callers reach them only for RocksDB stores, whose pass is one synchronous purgeLogs() call with nothing suspended mid-removal. resetDatabases() closes LMDB roots with no retirement call at all, so that re-check is a routine path rather than a defensive one.

The legacy per-table drop retires the audit store the table actually holds. It called primaryStore.auditStore?.stopAuditCleanup?.(), which nothing assigns — openAuditStore() assigns rootStore.auditStore and makeTable() captures the store it was handed — so the call was a no-op against a dead reference. closeDatabase and dropDatabase also deregister storage reclamation now: only closeBranchHandles did, so a closed or dropped store left a handler, and the closure pinning the store, registered for the life of the process, with every re-open of the same path appending another.

The last-removed marker is retained until it commits. It used to be a fire-and-forget put(), and a pass that deletes nothing never reaches the write again, so one transient failure would leave the recorded boundary permanently behind the entries already removed. It is now awaited inside error containment and carried to the next pass. The write that initializes a new audit store has no downstream owner at all — openAuditStore() is synchronous and returns the store — so its rejection is contained at the call site, through the same warnContained() helper as the pass's outermost backstop, because a failing log sink would otherwise turn one contained rejection into an uncontained one.

The pin moves to @harperfast/rocksdb-js 2.8.0, which is a prerequisite rather than hygiene. The previously pinned 2.7.1 already refuses to purge above the durable flush floor, but at the boundary segment it compares logFile->size without resolving the extent: a lazily-registered segment reads 0, so 0 > positionInLogFile is false, the guard passes, and the segment is deleted with a tail that never reached RocksDB. 2.8.0 calls ensureExtent() there and refuses to purge when the extent cannot be resolved; its should not purge a log file whose tail is past the flushed position regression covers exactly that case and does not exist in 2.7.1. One purge per boot rarely lands on whichever segment sits at the floor — a retention-derived cadence takes a fresh shot at it every pass, which is what turns a latent hole into a prerequisite. This supersedes the earlier claim that the change was blocked on rocksdb-js #799; that PR remains a safety improvement on top, restricting deletion to a contiguous oldest prefix so a non-contiguous purge cannot leave a hole that silently truncates a reader, but it is not required here.

Root msgpackr moves to 2.0.6 alongside it. rocksdb-js 2.8.0 declares that version exactly, which an exact 2.0.5 root pin cannot satisfy, so the install nested a second msgpackr under node_modules/@harperfast/rocksdb-js — two module instances, each with its own extension registry and its own msgpackr-extract native addon. That release is a single commit, "Reject array and map lengths that exceed the remaining source data".

For the human reviewer

1. Retention window doubles as the replication safety window, and this is the one finding rated major. Rocks segment purge is driven by logging.auditRetention with no floor for the slowest consumer. A peer offline longer than that window resumes, reaches the purged prefix, and gets done from TransactionLog.query() — indistinguishable from being caught up, so it is recorded as caught up while permanently missing that range. txnlogReplayGapBytes is an analytics gauge with no consumer that forces a base copy. Continuous retention is what moves this from unreachable-in-steady-state to routine. The escalation signal belongs in the reader, filed as HarperFast/rocksdb-js#805; the alternative is gating purge here on the oldest live consumer position, a semantics change operators would feel on a knob they tune for disk. Decide whether Create withHarper() Next.js config utility #805 lands before this ships enabled-by-default, and confirm whether harper-pro has an escalation path core cannot see — if it does, the DESIGN.md paragraph overstates the risk and should say so.

2. The 2.8.0 bump carries far more than the retention fix — 186 commits, 55 files, +4606/-593 in src/, including transaction-lifecycle work (Hash long URLs into primary key for cache table storage efficiency #741 close-pending-txns-with-owner, HTTP Logs showing up in system (non external) logs #744 bounding the coordinated-retry park, txn-registry weakptr, deferred orphan cleanup). Local suites are clean across that surface; CI is the authoritative gate. The narrower alternative is asking upstream for a 2.7.2 carrying only the ensureExtent commit.

3. purgeLogs() is synchronous and unbounded on the last worker, which also serves requests. The LMDB path below caps itself at MAX_DELETES_PER_CLEANUP and yields through setImmediate; the Rocks branch has no analogue. @cb1kenobi benchmarked the native call directly against the same 2.8.0 binary: ~0.12–0.17ms per eligible segment with no yielding, so a 20,000-segment backlog blocks the calling worker for about 3 seconds in one call. A steady-state pass has a handful of eligible segments; the case that hurts is an operator lowering logging.auditRetention from 30d to 1d, where the next pass stats and unlinks the whole accumulated backlog at once.

   Should purgeLogs() become async in rocksdb-js? My read: making it return a promise is the right end state but not sufficient on its own, and not this PR. An napi async worker moves the scan and the unlinks off the JS thread, which is what the 3-second number actually calls for — but it also changes a published API with three call sites in core (the cleanup loop, RocksTransactionLogStore's explicit purge, and ResourceBridge's operation), and it re-opens the pin this PR just settled. The cheaper half is additive and composes with either answer: PurgeLogsOptions takes no limit today, so give it one, and let the caller re-arm — that is exactly what MAX_DELETES_PER_CLEANUP already does for LMDB four lines below, and it bounds the block without touching the return type. My recommendation is to file both against rocksdb-js (bound first, async second) and keep this PR pinned to 2.8.0.

4. The retained watermark lives only in the cleanup closure. If the marker write fails transiently and the store is then closed and reopened, the retry is lost and getLastRemoved() reports a boundary behind the entries already deleted. Not built here: it self-heals on the next pass that deletes anything, and persisting it would need a second durable write on the path the first one just failed on. Called out because the review raised it and the "retained until it commits" claim above is scoped to the live loop.

5. Cadence formula: retention/10 floored at 10s, pressure shortening cutoff and cadence together, versus keying the backoff on the purge counters rocksdb-js now exposes. One expression to change.

6. The last-worker conjunct on the re-arm is unreachable through both arming paths and kept deliberately as a backstop for a direct caller of the exported scheduleAuditCleanup: onStorageReclamation registers only on the last worker (it takes no skipThreadCheck) and the store-open arm gates on the same index. A reviewer may prefer it deleted as dead code or promoted to the sole ownership check. Both re-arm guards are Rocks-only; the LMDB arm keeps main's unconditional re-arm.

7. stopAuditCleanup() is irreversible — it latches per audit-store instance, relying on reopen constructing a fresh store. Correct for today's callers; a future caller reusing a store object would get silent permanent retention loss.

8. Two changes here are not about retention. The merge of main conflicted on resources/Table.ts's reclamation import: this branch added path-wide removeStorageReclamation() for database-level teardown and the legacy per-table drop (which unlinks the path outright), while main added per-handler removeStorageReclamationHandler() for Table.cleanup(), because RocksDB column families share a path. They are complementary, so the resolution keeps both. Separately, one commit runs Prettier over unitTests/resources/query-array-scoping.test.js: main has been red on Format Check since Pin element-scoping semantics of queries over array-valued properties #2437 landed it unformatted, and every branch that merges main inherits that failure.

Verification

Unit: test:unit:resources 1936 passing / 28 pending under RocksDB and 1591 passing / 203 pending under HARPER_STORAGE_ENGINE=lmdb; test:unit:dataLayer 260 passing / 111 pending. test:unit:main is 5163 passing with two failures that are this sandbox, not the branch — nonInteractiveSpawn git credential scoping (the agent environment exports GIT_CONFIG_GLOBAL/GIT_EDITOR) and a getDomainSocketPathLengthWarning case that needs a rootPath under 107 bytes, which a worktree this deep cannot give it. Prettier, oxlint --quiet and tsc --noEmit clean.

Every guard added here has a regression that fails when that guard alone is reverted, verified by reverting it, rebuilding (#src/* resolves to dist/, so a source-only edit does not reach the tests) and re-running:

revert	test that goes red
stopAuditCleanup() returns Promise.resolve() instead of the active pass	drains the in-flight removal before stopAuditCleanup() reports the pass retired
cursor release no longer checks storeClosing()	touches nothing further once the root store closes under a suspended pass
dropDatabase() stops awaiting the barrier	makes dropDatabase wait for an in-flight cleanup pass before it closes the stores
the legacy drop goes back to primaryStore.auditStore	retires the legacy per-table drop against the table's own audit store
the init marker write logs through harperLogger.warn directly	contains a rejected last-removed initialization write (failure logging throws)
the priority term leaves the Rocks cadence formula	shortens the Rocks cadence while disk pressure is reported (100 vs 20)
the Rocks branch falls through to the LMDB backoff	holds Rocks cleanup at the retention-derived cadence whatever a pass purges

The legacy-drop regression loads a genuine legacy table through readMetaDb(path, table, schema, auditPath, true) — real .mdb root, real audit environment, real initStores/makeTable — and asserts the drop reaches the table's own audit store and waits on it before close()/unlinkSync(). The only synthetic part is the stopAuditCleanup spy on that real store; legacy audit stores are opened with a plain open() rather than openAuditStore(), so they arm no loop today.

The end-to-end route is a live integration regression, audit-retention-rocks.test.ts. It and the LMDB suite pass 5/5 in ~27s. It fails on origin/main with RocksDB transaction-log cleanup did not advance past purgeRuns=0, which is #2140's exact symptom. purgeRuns counts scans rather than deletions — transaction_log_store.cpp increments it before any eligibility check, marked observability-only — so an unproductive pass still advances it and the oracle is valid for the re-arm claim. Deletion itself is proved by the real-RocksDatabase unit test asserting only 3.txnlog survives.

Two retention regressions used to assert the cadence by replacing the process-global setTimeout with a stub that captured every timer under a second. That global is shared by every audit store in the process: under HARPER_STORAGE_ENGINE=lmdb the shared fixture's own loop is still re-arming at a sub-second cadence by the time they run, so its timer landed in the stub's queue, the assertion saw two timers, and the failing test then hung in its own finally awaiting a pass whose timer the stub had swallowed. .mocharc.json sets timeout: 0, so that wedged the whole LMDB unit run rather than reporting a failure — it reproduces on the branch as pushed, and stays hidden under RocksDB, where the shared store's cadence never drops below a second. The cadence now lives on the audit store and both tests drive real passes and read it, with no fake timers.

Three gaps stated rather than papered over: the last-worker gate has no multi-worker test, because asserting "no second loop exists" needs timing-based assertions; the live regression proves re-arm rather than on-disk deletion, which rocksdb-js's own suite covers; and it skips win32, where deleting a mapped segment raises a sharing violation instead of unlinking, so a Windows purge can warn-log and make no progress while a consumer holds a mapping.

Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex; declined=gemini,cursor-grok,cursor-composer,domain; rounds=5 @ c9debf0}

_{Human-Review-Need: 4 @ c9debf0}

[gemini-code-assist]

Code Review

This pull request unifies the audit retention cleanup lifecycle for both LMDB and RocksDB storage engines, establishing a self-rearming, engine-independent process. RocksDB now shares the same scheduling, error containment, and backoff logic as LMDB, purging native transaction-log segments during its cleanup passes. Feedback on the changes points out a high-severity issue where the backoff delay for RocksDB is never decreased or reset when logs are actively being purged, which could cause the cleanup loop to run at an extremely slow frequency even under active write traffic.

[kriszyp]

Reviewer routing: @cb1kenobi is the RocksDB responsible expert and highest-affinity reviewer; @heskew reported #846 and can validate the original retention failure mode.

— Codex

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Reviewed b997b444 — no new issues. The prior Medium (merge-ready framing vs. blocked-on-#799 body) is resolved: the body now correctly names rocksdb-js 2.8.0's ensureExtent() fix as the real prerequisite, and I independently verified that fix in rocksdb-js's own source and regression test at the v2.8.0 tag. The prior Low (un-unref'd Rocks cleanup timer) is unchanged in substance — the timer is .unref()'d — so nothing outstanding there either.

Verified independently: the last-worker re-arm conjunct is genuinely unreachable through both arming paths (confirmed by tracing onStorageReclamation's own last-worker gate); stopAuditCleanup()'s four logical teardown callers all destroy/close the store immediately after, so no reuse-after-stop path exists; the live integration test proves re-arm via purgeRuns (a scan counter, confirmed incremented unconditionally in transaction_log_store.cpp) while the real-RocksDatabase unit test independently proves on-disk deletion (asserts only 3.txnlog survives) — the two together prove what the PR claims, and a mutation test against stopAuditCleanup()'s irreversibility latch is caught by the dedicated unit test for it.

One data point worth having for item 3 in the human-review list: I benchmarked the native purgeLogs() call directly (synthetic segments, same rocksdb-js 2.8.0 binary) — it scales linearly at ~0.12–0.17ms per eligible segment with no yielding, so a 20,000-segment backlog blocks the calling worker for about 3 seconds in one synchronous call. That's a real number for whoever weighs the retention-lowering risk.

The six Human-Review-Need items are product/escalation calls, not defects — no comment on those. The PR body's own trailer says Human-Review-Need: 4 but lists 6 decision names; likely just a stale count, worth a one-line fix whenever convenient.

—
Generated by Barber AI

[cb1kenobi]

Reviewed c9debf0 and found no blocking issues. No new blocking findings were confirmed on changed lines. Previously raised issues were not repeated, and the remaining disclosed risks are tracked or require product decisions.

—
Generated by Barber AI