Keep transaction-log retention on a durable sequence floor

[kriszyp]

Ordinary and startup transaction-log retention now delete only an eligible contiguous oldest prefix below the persisted flush floor. The sequence file named by txn.state and every newer file remain, so txn.state, its cached file handle, the writer, and the existing contiguous reader stay in the same sequence space.

This deliberately does not change transaction-log reader behavior or add JavaScript/native cache-generation APIs. destroy: true remains the existing separate destructive path; Harper does not call it in production.

An idle store may retain one segment past the age cutoff until a later write rotates and flushes it; that extra segment is bounded by transactionLogMaxSize, except when a single transaction exceeds that target. The bound covers the floor segment only — a store whose txn.state lags also retains everything above the floor, which purge.retainedUnflushedFiles reports.

Windows CI on this branch failed the new should detach an already-missing prefix entry without wedging retention test with EBUSY on all five runtimes, because the segment the test unlinks was still open. The cause is startup discovery, not retention: registerLogFile() promoted whatever segment currently held the highest sequence — opening it and enabling its append-boundary marker — and nothing demoted it when a higher one turned up. Directory iteration order is unspecified, so on a sorted-enumeration filesystem every superseded segment kept an unused marker and an open handle for the life of the store; on Windows that handle is taken without FILE_SHARE_DELETE, so those segments were also undeletable from outside the process. Discovery now registers without opening, and load() marker-enables and opens the one file that is still current after the whole directory has been scanned. An unopenable current segment degrades as before — appends resume at a fresh sequence — while an append-boundary violation stays fatal.

Terminology follow-up (review feedback): the purge boundary local is now retentionFloorSequence, not durableFloorSequence. "Durable" described only the primary branch — with no persisted flush position the floor falls back to the newest file, which is kept because it is live, not because anything below it is durable. The README, docs/stats.md and docs/transaction-log.md say "retention floor" and now spell out that fallback instead of attributing the floor to txn.state unconditionally. The same round also scoped the README's disk bound: transactionLogMaxSize bounds the one extra floor file an idle store keeps, but not the suffix above the floor that a lagging txn.state retains — purge.retainedUnflushedFiles is what reports that case. No behavior change.

Refs HarperFast/harper#846 — Transaction-log retention has no config-driven time/size control (and the storageReclamation hardening isn't on v5.1).

For the human reviewer

• The discovery regression test is order-dependent in both halves, not just the marker one — the comment claiming the open-descriptor assertion is "the order-independent half" is wrong. The pre-fix code opened a segment only when it raised the high-water mark, so if directory_iterator yields the highest sequence first it also leaves exactly one open fd and one marker and the test passes against the code it was written to catch. Deterministic on the sorted-enumeration NTFS runners where this failed; on hash-ordered ext4 it is a coin flip. A real fix drives the enumeration order or asserts that a marker for a lower sequence never appears.
• Independent review kept a set of pre-existing retention points that predate this fix and that I left alone as out of scope for a CI repair. All are minor or nit; they are decisions, not mechanical fixes, so they are listed rather than applied:
  • A non-ENOENT getLastWriteTime() failure breaks the whole scan with only a DEBUG_LOG, which release builds omit. The break is right — the silence is not: one EACCES/EIO segment wedges retention permanently while purgeRuns and lastPurgeMs keep advancing. A writeFailures bump or a transactionLog:warning would surface it.
  • The flushed-position extent guard is unreachable on the !all path (the floor breaks first), so it is live only for destroy: true — and the fix(txnlog): erase a failed append's partial bytes instead of baking a framing break into the log #751 test pair that says it "covers the guard itself" no longer reaches it.
  • purgeableFiles applies the floor but not the contiguous-prefix stop, so it over-reports when a lower sequence has a newer mtime than a higher one (restore copies, an operator's utimes). Gauge-only, and already documented in place as skew.
  • An explicit purgeLogs({ before }) is silently subject to the floor and returns [] with no reason code, so a disk-pressure runbook cannot tell "nothing old enough" from "blocked by an unadvancing txn.state".
  • Startup retention still deletes files during a read-only open — load() has no read-only signal. Pre-existing and strictly narrowed by this diff (main's discovery-time expiry had no flush guard at all), so it wants its own issue.
  • New this round, and fully pre-existing: DiscoverStores re-runs TransactionLogStore::load() over an already-loaded directory whenever a second descriptor opens the same path (DBKey is {path, readOnly}; the log registry entry is keyed by path alone). The throwaway store runs recoverTail() against the segment the live handle is appending to. Worth an issue on its own.

Verification

• The startup-retention regression fails on origin/main: an aged current segment is deleted.
• 90 transaction-log and stats tests passed; 3 platform-specific tests skipped.
• The full JavaScript suite passed: 828 tests, with 2 skipped.
• The native suite passed: 148 tests, with 3 macOS-only tests skipped.
• Native build, type checking, lint, formatting, and git diff --check passed.
• The discovery regression test fails against the pre-fix binding (1.txnlog.boundary present) and passes after.
• Re-verified after the discovery fix: full JavaScript suite 829 passed / 3 skipped, native suite 151 passed, pnpm check clean.
• Re-verified after the terminology follow-up: pnpm build:binding, pnpm check, native suite 159 passed, and transaction-log + transaction-log-stats 94 passed.

Complexity: moderate

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

_{Human-Review-Need: 3 @ a7a967b}

[gemini-code-assist]

Code Review

This pull request refactors the transaction log store to ensure that only closed stores can lose their directory or flush watermark, preventing issues where active stores are deleted during ordinary retention purges. Key changes include updating close logic, rewriting timestamp-based position lookups, writing the flush state via a local stream, and handling watermark resumption on startup. Additionally, the transaction log reader is updated to skip missing segments. Feedback on the reader implementation highlights two potential bugs where empty log segments (containing only the header) could cause the iterator or reader to prematurely terminate; using while loops instead of if conditions is recommended to continuously advance past empty segments.

[github-actions]

📊 Benchmark Results

get-sync.bench.ts

getSync() > random keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	24.65K ops/sec	40.57	39.09	642.387	0.113	123,238
🥈 rocksdb	2	11.39K ops/sec	87.82	85.27	4,379.225	0.179	56,936

getSync() > sequential keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	28.74K ops/sec	34.80	33.71	457.017	0.099	143,682
🥈 rocksdb	2	11.67K ops/sec	85.69	84.41	474.073	0.047	58,353

ranges.bench.ts

getRange() > small range (100 records, 50 range)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	25.03K ops/sec	39.94	36.52	1,923.149	0.292	125,174
🥈 rocksdb	2	15.75K ops/sec	63.51	55.57	1,108.608	0.125	78,733

realistic-load.bench.ts

Realistic write load with workers > write variable records with transaction log

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	364.76 ops/sec	2,741.515	68.23	50,233.289	16.24	732
🥈 lmdb	2	26.15 ops/sec	38,247.178	378.499	1,209,878.654	137.026	64.00

transaction-log.bench.ts

Transaction log > read 100 iterators while write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	39.12K ops/sec	25.56	11.29	20,825.823	0.849	195,599
🥈 lmdb	2	436.44 ops/sec	2,291.254	249.36	8,721.525	1.20	2,183

Transaction log > read one entry from random position from log with 1000 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	711.69K ops/sec	1.41	1.20	5,089.952	0.215	3,558,470
🥈 lmdb	2	463.96K ops/sec	2.16	1.21	753.068	0.279	2,319,813

worker-put-sync.bench.ts

putSync() > random keys - small key size (100 records, 10 workers)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	818.82 ops/sec	1,221.273	1,047.326	6,248.785	0.607	1,638
🥈 lmdb	2	1.15 ops/sec	870,566.647	812,989.962	931,668.121	3.00	10.00

worker-transaction-log.bench.ts

Transaction log with workers > write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	22.24K ops/sec	44.97	29.43	20,589.105	2.09	44,472
🥈 lmdb	2	821.97 ops/sec	1,216.594	149.438	15,540.102	5.51	1,644

---

Results from commit fc27768

[cb1kenobi]

Barbarian reviewed a7a967b and found no blocking issues. No confirmed blocking defects were found on the changed lines. Existing discussion was checked, and previously raised or non-blocking concerns were not repeated.

—
Generated by Barber AI