fix(txnlog): resume the recovery scan and timestamp index past a mid-file framing break

[kriszyp]

A transaction log with a mid-file framing break — a torn frame that intact, committed entries follow — is now fully readable after reopen. Two native walks of the framing stopped at the first break even though query() has resynced past such breaks since #750: the open-time recovery scan seeded the committed-read watermark (lastCompleteTransactionEnd) from the entries before the break, so a committed drain of a 60-entry log torn at frame 39 returned 39 entries and no error; and the timestamp index strode through the broken frame's declared length past the written extent and froze, so every seek at or after the break reported "past this file". Both walks now resume at the same offset the JS reader's findResyncPosition() uses (findFramingResumeOffset(), shared by the scan and the index), so the watermark covers every complete transaction in the file and the index covers every intact entry. The break is still surfaced as CorruptFrameError with resyncPosition; a file with a break is never truncated, including when a torn tail follows the break.

The shared resume rule also learned about pre-extended segments: on Windows a segment is zero-padded to its maximum size, and the native walks run before size has been corrected to the end of the data, so a chain of fewer than eight frames in front of the padding could never "reach EOF" and classified as a torn tail — recovery would have truncated committed entries. A chain landing exactly on the end of the nonzero bytes now counts the same as one landing on EOF.

Review feedback on this PR found the other half of that accounting in the index walk. It searches the mapped region (min(size, mapSize)), which stops short of the written extent whenever one batch exceeded transactionLogMaxSize or the limit was lowered — and an empty search there was treated as "nothing follows the break", parking lastIndexedPosition at the extent, past bytes the walk never read. That loss is permanent: a later, larger map resumes from lastIndexedPosition and never looks below it. An empty result is now scoped to the bytes that were searchable, so the walk stays at the break and reports an unindexed tail — the same treatment the header bound-check above it already gives a tail the map does not cover — and parks at the extent only when the whole extent was searchable and the break is therefore a torn tail. A short search also gives up the "chain lands on the written extent" signal: the region ends on an arbitrary cut, so a chain landing there proves nothing, and accepting one would let a garbage chain inside the corrupt gap resume the walk — whose bogus timestamp then caps this running-maxima index and hides every real entry behind it.

Fixes #815. This is the engine half of the harper-pro txnlogTearReplication red (follower stuck at 39/60 rows); harper core and harper-pro follow with their own PRs on top of the 2.8.1 release.

For the human reviewer

Framing-Verdict: better-alternative-exists (8fc19599da00) — resolved by ruling: system-level resync (core surfaces the mid-log-break halt, harper-pro forces the bounded base-copy) plus fix the engine watermark amputation and seek blindness here, rather than either alone.

1. The committed watermark now trusts the resync heuristic. lastCompleteTransactionEnd advances past a break to the last flagged entry the scan can reach, which means the committed-read bound accepts a run of frames that findFramingResumeOffset() judged to be real log data (≥ 8 well-formed frames, or a chain landing exactly on the written extent). Alternative: keep the watermark pinned before the first break and leave only uncommitted reads able to see past it. Chosen because the JS reader already delivers those frames as entries under the same heuristic, so pinning the watermark buys no additional safety, only the 39/60 amputation. A "no" here reverts to the amputation and pushes the whole recovery onto the base-copy resync in harper-pro. Cheap to reverse (one branch in the scan).
2. Accepted and filed, not fixed here: the unflagged prefix of the torn transaction is now reachable on committed reads of the current file (Committed txnlog reads deliver the unflagged prefix of a transaction torn at a mid-file break #816). When the break tore the last entry of a batch, the batch's earlier entries are intact frames below the advanced watermark, so a committed read delivers them before throwing at the break — entries the source never committed. Pinning the watermark only for that shape would silently re-amputate it (the sender would park with no error), which is worse under the ruling's invariant than a loud break plus a phantom the forced copy repairs; a proper fix is a hole-aware committed read (per-file torn-group range surfaced to the reader, including lazily for rotated files, where the same leak has existed since fix(txnlog): resync past a mid-log corrupt frame instead of ending the log #750). That is a new engine surface, out of scope for the red fix. Both the PR review and every pre-push round re-raised it as the top finding; the ruling on this task was to keep fix(txnlog): resume the recovery scan and timestamp index past a mid-file framing break #817 scoped and land the hole-aware reader in Committed txnlog reads deliver the unflagged prefix of a transaction torn at a mid-file break #816.
3. The index walk treats a frame whose length overruns size as a break, never as an in-flight append. This rests on writeEntriesV1 bumping size only after writev returns and on the POSIX overlay exposing bytes only up to an entry boundary, so a nonzero header below size is always a complete entry. If that invariant is ever relaxed (a writer publishing size before the bytes land), the walk would skip a live entry's index slot until resetTimestampIndex(). Same assumption the base code made implicitly by striding HEADER + length; this PR makes it explicit in the comment. Where to look hardest: the resume branch of findPositionByTimestamp and the min(size, mapSize) bound on the byte search.
4. A short map's empty search stops the index at the break; only a full-extent search parks it at the extent. The torn-tail branch still jumps lastIndexedPosition to size so later appends get indexed and the torn bytes never do — but only when the whole written extent was searchable, which is the only case that proves nothing follows. Staying at the break costs nothing per seek: the extent that failed is memoized, since only a larger map can change the answer, and the guard answers a repeat seek without re-scanning the corrupt gap under the store's dataSetsMutex. Every assignment that moves lastIndexedPosition off the break — including resetTimestampIndex() — clears the memo, so the two cannot disagree; that is the invariant to check hardest. A native-test-only counter (resyncSearchCountForTests, ROCKSDB_JS_NATIVE_TESTS only) makes the elision assertable rather than assumed. The landing rule is switched off for a short search through endIsWrittenExtent, which defaults to true so the recovery scan — which reads the real file — is unchanged.
5. "End of the nonzero bytes" as a second written-extent signal. nonzeroEnd() is computed once per scan, reading the file backwards from its physical size, only when a break has already been found. The residual: a run whose last payload ends in zero bytes still misses the signal (its chain lands past the nonzero end) and, if shorter than eight frames and in front of padding, still classifies as torn — Windows-only, and narrower than the base behaviour, which truncated every such run; the rule is recorded in AGENTS.md invariant 11. A false positive needs a forged chain landing on that exact offset (one in 2³² per candidate). Alternative: derive the extent from the caller's corrected size — but the callers are the ones computing size, so the scan has nothing to be handed. Fixing it exactly needs a persisted logical append extent, which is a format change and a follow-up, not part of this red fix; accepting pos >= nonzeroEnd() instead would let any forged length landing anywhere in megabytes of padding qualify, which is a much worse trade.
6. Classification stays pinned to the first break (firstBreak) while the walk continues past it (MidFileCorruption, validEnd = first break) — unchanged from base, which also stopped classifying at the first break and never truncated after it.

Declined: the resync byte search runs under fileMutex with no upper bound other than the file (Codex, round 1). It runs once per file per open, only when a break exists, and spans only the corrupt gap; the adjudicator agreed it is a latency cliff on an already-corrupt file, not a hang. The memo above removes the repeat-seek half of the same concern raised against the index walk (which holds the store's dataSetsMutex). The overlay-lag concern from the same round ("a live append mistaken for corruption") does not apply: the shared range ends at an entry boundary and the walk never reads a header at or past size, so a lagging overlay reads as zeros — the existing end-of-data branch.

Declined: "the partial-map retry cannot recover through the public reader, because a current file is always mapped at maxFileSize" (pre-push round 6-9). The retry is what makes recovery possible at all. While the segment is current, maxFileSize is the same bound the reader maps under (TransactionLogStore::getMemoryMap), so no entry past it was readable in the first place; once the segment rotates or is reopened as non-current it is mapped at its own size, the whole extent becomes searchable, and the walk — still parked at the break — resumes and indexes the run. Under the previous behaviour lastIndexedPosition had already jumped past those entries, so rotation could never recover them.

Verification

• Fails-on-base: with src/ reverted to origin/main (tests kept), the new behaviour tests in TransactionLogRecovery.* / TransactionLogTimestampIndex.* fail and the 3 new JS mid-file break on reopen tests fail with expected 39 to be 59, expected 20 to be 58, expected 39 to be 59 — the same 39/60 shape as the harper-pro red. The TransactionLogResumeOffset.* tests do not compile on base (new symbol). The padding rule proven the same way: with reachesWrittenExtent reduced to pos == fileSize, ShortRunReachingThePaddingIsNotTruncated fails (TruncateTail instead of MidFileCorruption, watermark 69 instead of 177) and ShortRunBeforeThePaddingIsIndexedAndEndsTheFile fails (seek reports past-file).
• Fails-on-branch for the new index rule: ShortMapDoesNotSkipTheUnsearchedPostBreakTail fails with the previous lastIndexedPosition = writtenExtent line restored — the short-map seek and both later full-map seeks report 4294967295 (past-file) instead of the break and the run's offsets — and, with the memo guard disabled, the counter assertions fail 2/3 against the expected 1/2. ShortMapCutIsNotTreatedAsTheWrittenExtent and ACutIsNotTheWrittenExtent both fail with endIsWrittenExtent forced true (the index one resumes at the run instead of holding at the break).
• Green: pnpm test:native 180/180; pnpm test 837 passed, 3 skipped; pnpm check clean. Note for anyone re-running the native suite: pnpm test:native rebuilds only when the binary is missing or the RocksDB pin changed, so a C++ edit needs NATIVE_TEST_REBUILD=1 or it silently tests the previous binary.
• Windows CI (first push): the four mid-file break on reopen tests failed on every Windows leg with errno -4094 (ERROR_USER_MAPPED_FILE) opening 1.txnlog. The tests queried the log before closing the database to collect seek timestamps; that query maps the file and the map outlives close() while its entries are reachable, so the truncating rewrite that tears the frame is refused on Windows. They now read the timestamps from the frame headers on disk after close, so nothing is mapped when the file is rewritten (test/transaction-log.test.ts headerTimestamps). The new native test needs the same care from the other direction: Windows indexes the whole file against a full-extent map inside openFile(), so it drops that mapping and the index before asserting. Both are proven by the Windows legs on this push.
• End-to-end route: harper-pro integrationTests/cluster/txnlogTearReplication.test.mjs against a core pinned to this build, in the harper-pro PR that lands on top (its own PR body carries the 60/60 result).

Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex; blocked=gemini(auth); declined=cursor-grok,cursor-composer,domain; rounds=11 @ 7012af7}

_{Human-Review-Need: 4 @ 7012af7}

[gemini-code-assist]

Code Review

This pull request improves the transaction log recovery and indexing mechanisms to handle mid-file framing breaks (corruptions) more robustly. Instead of stopping or truncating at a break, the recovery scan and timestamp index now resume past the corruption using a new findFramingResumeOffset helper. This ensures that valid entries after a break remain readable, searchable, and indexed. Additionally, the recovery logic is updated to handle pre-extended files with zero padding (such as on Windows) by detecting the end of nonzero bytes to prevent premature truncation. Comprehensive native and integration tests have been added to verify these behaviors. I have no feedback to provide as there are no review comments.

[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	23.90K ops/sec	41.85	39.93	575.996	0.115	119,476
🥈 rocksdb	2	10.80K ops/sec	92.62	89.41	31,587.044	1.25	53,983

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

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	28.41K ops/sec	35.20	34.10	470.054	0.097	142,030
🥈 rocksdb	2	11.08K ops/sec	90.25	87.85	2,925.902	0.124	55,403

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.52K ops/sec	39.19	36.07	3,836.746	0.322	127,579
🥈 rocksdb	2	16.91K ops/sec	59.12	51.22	1,094.665	0.128	84,570

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	345.25 ops/sec	2,896.476	128.407	75,662.348	16.84	691
🥈 lmdb	2	26.97 ops/sec	37,084.056	413.333	1,179,947.011	136.716	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	38.46K ops/sec	26.00	11.44	26,970.285	1.08	192,299
🥈 lmdb	2	439.58 ops/sec	2,274.922	110.892	16,447.32	1.36	2,198

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	746.32K ops/sec	1.34	1.18	4,662.359	0.206	3,731,576
🥈 lmdb	2	458.78K ops/sec	2.18	1.14	6,844.783	0.548	2,293,903

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.89 ops/sec	1,221.161	1,056.735	1,976.096	0.319	1,638
🥈 lmdb	2	1.18 ops/sec	849,866.56	806,646.471	901,566.286	2.80	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.22K ops/sec	45.01	29.82	20,602.775	2.09	44,438
🥈 lmdb	2	802.36 ops/sec	1,246.316	42.52	19,739.917	6.38	1,605

---

Results from commit 94b0fe3

[kriszyp]

I believe this resolves the broken CI failures on harper-pro main right now.

[cb1kenobi]

Critical — committed reads expose a torn transaction's prefix. Resuming here lets a later LAST_FLAG move the single contiguous lastCompleteTransactionEnd beyond the break; if the broken frame ended an uncommitted batch, its intact prefix is then emitted as committed before CorruptFrameError, creating phantom writes or replication. Simplest fix: keep the committed watermark before the first break; if post-break commits must remain readable, add a torn-group skip range that committed readers exclude before advancing it.

—
Generated by Barber AI

[kriszyp]

Deliberately not fixed here — this is #816, and the ruling on this task was to keep #817 scoped to the #815 recovery fix. Both suggested shapes were weighed: pinning the committed watermark before the first break restores the 39/60 amputation this PR exists to remove (a stuck sender with no error, which is worse than a loud break the forced base-copy repairs), and the torn-group skip range is the hole-aware committed read that #816 tracks — a new engine surface, and the same leak has existed for rotated files since #750. Left unresolved on purpose so it stays visible.

— Claude Opus 5

[cb1kenobi]

High — trailing-zero payloads are mistaken for a torn tail. nonzeroEnd() returns the byte after the last nonzero value, not necessarily the start of preallocation padding; when the final committed frame's payload ends in zero bytes and the resumed run has fewer than eight frames, pos lands after this value, resync returns 0, and recovery truncates valid committed entries at the break. Simplest fix: determine or persist the exact logical append extent independently of payload contents and compare against that boundary.

—
Generated by Barber AI

[kriszyp]

Accurate, and deliberately not fixed here: doing it exactly needs the persisted logical append extent you describe, which is a format change rather than part of this red fix. The scope is narrower than the base behaviour it replaces (which truncated every such run, not only one whose final payload ends in zero and is shorter than eight frames), and it is Windows-only since a POSIX segment ends at its physical size. Loosening reachesWrittenExtent to pos >= nonzeroEnd() instead would accept any forged length landing anywhere in megabytes of padding, so that is not a safe cheap version. Left unresolved so the follow-up stays visible; noted for issue triage.

— Claude Opus 5