Read every entry of a transaction-log segment that one transaction pushed past transactionLogMaxSize

[kriszyp]

Transaction-log readers now read every entry of an oversized transaction through a fixed-capacity mapping. The native writer closes the header/first-append gap and rotates before any append could outgrow a buffer already handed to a reader; JavaScript no longer needs to remap growing segments.

Breaking option change: transactionLogMaxSize: 0 (formerly unlimited segment size) now fails open() with a migration hint (Database::Open). Omitting the option keeps the 16 MB default.

For the human reviewer

1. Adopted planning alternative. The planning review returned Framing-Verdict: better-alternative-exists: withholding mappings for recovered empty files would stall frozen history and disturb tail positioning. We instead keep those files readable and enforce their exposed capacity at append admission. This fulfills the requested native invariant without changing the reader's initial position.

2. Requirement and compatibility: reject 0 rather than redefine it. Transactions remain whole, including a first transaction larger than the configured target. An earlier revision silently treated transactionLogMaxSize: 0 as the 16 MiB default. Per @cb1kenobi's review, a store configured with 0 must not start rotating and purging without being told, so Database::Open now throws with a migration hint, and the README states the retention effect of choosing a size.

   • Why 0 cannot stay unlimited: a reader's buffer has a fixed capacity, so an unbounded segment needs either reader remapping (the complexity this PR removes) or, on Windows, pre-extending the active file to the whole reservation on disk.
   • What 0 did on main: the writer never rotated by size, but readers mapped the current segment at capacity 0 (getMemoryMap → fileSize == 0 returns null), so query() read nothing from it. A segment reaching 4 GiB failed every later append (maximum supported file size). Age rotation after a write-idle gap (transactionLogMaxAgeThreshold) still rotated and purged.
   • Who is affected: Harper core and harper-pro never pass this option (only two Harper unit tests set 128).

   ❓ Your call: release handling. Rejecting 0 breaks any external config that passes it, so it needs a semver-major bump or an explicit release note. The alternative is the earlier silent default, which you had approved as one acceptable handling.

3. Review-depth decision: continue with the replacement design. The branch's third full pass was the first to inspect the native admission mechanism; it accepted one latent Windows normalization hazard, one inherited reader-cache defect, and small diagnostics/documentation repairs in about 27 minutes. The diff grew from 745 to 860 changed lines, mainly regression coverage. Three fresh full passes have now reviewed the replacement approach. The final pass found no new correctness defect after the Windows open and committed-watermark repairs. Stop further correctness review: the native capacity invariant survived all three passes, the accepted failures have discriminating regressions, and the remaining metadata race predates this diff.

Review-feedback round (Claude Opus 5.5): one closing full pass and three delta passes covered the 0 rejection, a first-append test seam moved into the open-to-append gap, a reader skip fix, and the README retention note. The final verdict was LGTM.

Declined from that round, with reasons:

• Running the first-append exclusion test in a subprocess. Only a self-deadlocking regression or a thread-creation failure can hang it, that regression would deadlock every lazily opened append in the suite, and the CI job timeout catches it.
• Removing "Test-only." from the two new seam docs. It matches the neighbouring seams.
• Moving hasAppendedSinceOpen after a refused batch. The ordering predates this PR, Windows open normalizes size first, and a refused segment rotates away.

Pre-existing concern left outside this diff: the native external-buffer wrapper reads mutable MemoryMap::fileSize after the file lock is released. The review identifies a C++ metadata race across worker handouts; this change does not claim to repair it.

CI maintenance round (Claude Opus 5.5): the red checks at fe4362a7 were GitHub runner failures, not test failures — 11 jobs never acquired a runner, and three runners died mid-test (no log uploaded, or "runner has received a shutdown signal" after every test had passed). On re-run all 24 checks passed, plus the 2 skipped draft-review workflows. 75f27c4d fixes a stale registry config comment that still said 0 selects the default limit, per @cb1kenobi's inline suggestion; delta review LGTM.

Product and architecture tour

Establish capacity before readers can see the segment

A current segment is first exposed at the configured capacity or its completed oversized batch size. Subsequent appends cannot exceed the smallest buffer capacity already exposed. Frozen segments can be mapped at their final written size.

Header creation and the first batch share one lock

A reader racing creation waits until the complete first batch has established the written extent.

sequenceDiagram
    participant Writer
    participant File
    participant Reader
    Writer->>File: Acquire fileMutex
    Writer->>File: Open and write header
    Reader->>File: Request mapping and wait
    Writer->>File: Append complete first batch
    Writer->>File: Release fileMutex
    File-->>Reader: Map capacity covering the batch

Loading

A crash or a known zero-byte append failure can still leave an empty segment. If a reader already holds its normal-capacity mapping, an oversized batch moves to a fresh segment. The empty mapping stays valid, and iteration crosses it to reach the new transaction. Capacity is retained after readers release their buffers so a later append cannot invalidate a previously exposed view.

Changes

• src/binding/transaction_log/transaction_log_file.cpp holds fileMutex through lazy open, header initialization and append; cleans up a rejected open; checks whole-batch admission against the exposed capacity; and publishes a bounded readable extent. Timestamp indexing raises mapping capacity under the same lock and throws on a nonempty mapping failure instead of fabricating EOF.
• src/binding/transaction_log/transaction_log_file.h records the smallest reader capacity, declares the typed internal open failure and the test seams, and documents lazy open/refusal, mapping ownership, and the bounded extent contract. Internal index mappings are distinct from public reader handouts; the mapping constructor clamps the initial readable extent as well.
• src/binding/transaction_log/transaction_log_store.cpp moves opening into the locked append operation, preserves the one-rotation open retry and partial-write retirement, and rotates a refused whole batch. Its constructor normalizes 0 using the shared default; src/binding/options/db_options.h shares the default constant with the store and includes the statistics declaration needed by that header's existing StatsLevel default. src/binding/transaction_log/transaction_log_store.h and src/binding/transaction_log/transaction_log_store_registry.h update their configuration contracts.
• src/binding/transaction_log/transaction_log_file_posix.cpp and src/binding/transaction_log/transaction_log_file_windows.cpp size new mappings to at least the written extent, replace undersized internal views before first reader exposure, and retain an exposed active mapping so its POSIX overlay remains current. Windows releases its internal view before extending for the first full-capacity handout and fails an open when mapping cannot establish the logical extent, preventing appends past zero padding. Both clamp readable extent to capacity and preserve the mapping-failure test seam.
• src/binding/database/database.cpp rejects transactionLogMaxSize: 0 at open; the store constructor still maps an internal 0 to the default for native callers.
• src/transaction-log-reader.ts has no mapping-growth handling. Its remaining changes refresh a filtered iterator's final segment extent before rotation, skip retired empty segments during traversal, cache final size only for segments older than the committed watermark, and wait at zero when the chosen segment is ahead of that watermark. Traversal also crosses an empty retired segment whose mapping fails, instead of stopping at it.
• README.md documents that 0 is rejected, the retention effect of migrating to a size, and the oversized mapping's memory accounting. src/load-binding.ts documents the binding option; src/store.ts repeats the default and the 0 rejection on the public StoreOptions property shown by editors.
• AGENTS.md states the mapping invariant and corrects the cause of a short internal index map. src/binding/transaction_log/DESIGN.md explains enforcement, recovery and platform ownership; DESIGN.md indexes that note.

Verification

Route: public transaction-log integration tests plus deterministic native concurrency and failure tests. The Harper replication sender itself is not exercised.

• test/transaction-log.test.ts covers oversized payloads after reopen, timestamp seeks after lowering the target, recovered empty mappings, committed/uncommitted tails across rotation, cached buffer identity, filtered iteration, default-size rotation, and the 0 rejection. The recovered-empty and default-capacity assertions failed against the previous PR binding before the native changes. Crossing an unmappable empty retired segment returns 1 of 201 entries without the reader fix.
• test/native/transaction_log_writev_test.cc is POSIX-only and proves an exposed empty mapping refuses an oversized batch, readers cannot acquire the map between header creation and the first append (fails 3/3 when writeEntries() releases fileMutex between lazy open and append), a zero-byte failed first append can rotate safely, and a rejected lazy open releases its handle and extent.
• test/native/transaction_log_mmap_test.cc is POSIX-only and checks internal-view replacement before first reader exposure, stable handout reuse, normalization of a stale capacity request, and index mapping failure. test/native/transaction_log_recovery_test.cc adds the Windows padded-file mapping-failure/retry regression and preserves short-map corruption coverage through an append-after-index-acquisition seam.
• test/fixtures/fork-transaction-log-ahead-cache.mts holds an actual commit between log append and RocksDB commit, creates a future-timestamp query, then appends and rotates. The 534-byte and oversized 2048-byte variants yield nothing on two polls before commit and resume afterward; the test failed with entries 4 and 5 missing before the cache condition changed. The oversized case also failed with false corruption before the bound repair. Each spawned child has a 20-second termination deadline.
• At 60d5bd93: pnpm test 1,118 passed, 10 skipped; native binary 309 passed. At 859d6302: transaction-log suite 122 passed; pnpm check clean.
• Earlier: pnpm test 1,116 passed, 10 skipped, zero failures before the final bound repair. The complete transaction-log suite was rerun after that repair (120 passed). Full native binary: 309 passed. STRESS_MODE=essential pnpm test:stress: 8 passed, 1 skipped. pnpm check and both native and bundle builds passed on Linux. The two final ahead-segment regressions also passed under the Deno test runner. CI at 223531fe: 24 checks passed and the two draft review workflows were skipped. Native tests and Node 22/24/26, Bun, and Deno integration passed on Linux, macOS, and Windows. Windows native CI passed all 279 tests, including MappingFailureCannotAppendPastZeroPadding.

Fixes #889

Origin — the dispatch brief this PR was written from

Read every entry of a transaction-log segment that one transaction pushed past transactionLogMaxSize

LIVE CONVERSATION about #890.

You are answering a person, in a thread, one turn at a time. Every turn:

1. Read the whole thread in this dispatch file's # Log — it is the conversation so far, and each of your previous turns is in it. Read the PR/issue and the code as needed.
2. Answer the LAST message. Append your answer to # Log as your turn. Prose, not a report: they are talking to you, and a status template is not an answer.
3. Set status: needs-input and stop. The thread stays open; their next message resumes it.

Each turn arrives as ASK (answer it, change nothing) or PERFORM (do it, then say what you did) — the person chose which when they sent it, and the run's own prompt tells you which one this is. Never infer it from the wording: an unrequested commit in the middle of a discussion and a polite description of work that was supposed to happen are the two failures this exists to prevent.

Never mark a PR ready and never merge from this conversation.

_{Dispatch: task chat-pr-rocksdb-js-890-kriszyp · queued by unknown · ran by codex/gpt-6-astra/low · worker kzyp-xps-1}

Related PRs: #545 overlaps, #742 independent, #767 overlaps, #842 independent, #897 independent, #900 independent, #901 independent, #902 overlaps, #74 independent
Complexity: complicated

_{Review-Coverage: authored=claude; ran=cursor-composer,codex,gemini; adjudicated=domain; declined=cursor-grok,cursor-kimi,cursor-muse; rounds=14; full=6 @ 75f27c4}

_{Review-Attention: deep ~40m (critical: transaction_log_file.cpp, transaction_log_file.h +6; decisions: reject-zero-max-size, writer-side-capacity, capacity-never-released, map-failure-throws, ahead-segment-hidden, do-less-alternative) @ 75f27c4}

[gemini-code-assist]

Code Review

This pull request addresses an issue where a transaction-log mapping handed to a reader does not cover the segment's committed extent (e.g., when a single transaction batch exceeds transactionLogMaxSize). It ensures that mappings are raised to the append-owned size under fileMutex on both POSIX and Windows, and updates the JS reader to detect outgrown buffers and remap them instead of reporting false corrupt frame errors or prematurely stepping over unread tails. Additionally, it throws an exception on mapping failures during index walks rather than returning a sentinel, and adds comprehensive unit and integration tests 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	24.74K ops/sec	40.43	39.10	726.925	0.082	123,681
🥈 rocksdb	2	10.43K ops/sec	95.85	90.19	24,067.723	0.974	52,166

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

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	28.86K ops/sec	34.65	33.33	516.604	0.102	144,299
🥈 rocksdb	2	10.44K ops/sec	95.76	92.67	561.631	0.056	52,217

ranges.bench.ts

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

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	24.49K ops/sec	40.83	35.88	1,767.663	0.291	122,457
🥈 rocksdb	2	15.93K ops/sec	62.76	54.20	1,081.328	0.120	79,665

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	376.48 ops/sec	2,656.183	56.55	57,330.285	10.12	753
🥈 lmdb	2	26.46 ops/sec	37,793.609	424.251	1,204,447.946	136.763	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	35.33K ops/sec	28.31	12.54	20,907.284	0.856	176,642
🥈 lmdb	2	434.06 ops/sec	2,303.825	144.507	17,927.039	1.48	2,171

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	646.31K ops/sec	1.55	1.34	5,158.793	0.215	3,231,556
🥈 lmdb	2	484.40K ops/sec	2.06	1.15	8,655.786	0.540	2,422,010

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	827.57 ops/sec	1,208.363	1,043.327	1,853.399	0.358	1,656
🥈 lmdb	2	1.14 ops/sec	880,158.169	811,940.3	1,005,931.104	4.90	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	21.37K ops/sec	46.80	29.70	21,548.468	2.30	42,734
🥈 lmdb	2	831.94 ops/sec	1,202.005	48.53	12,157.45	5.69	1,664

---

Results from commit cbb80be

[cb1kenobi]

No confirmed blocking defects were found on changed lines in the supplied bundle. The changes enforce fixed reader capacity and correct traversal across oversized segments and committed boundaries.

—
Reviewed 223531f

[cb1kenobi]

@kriszyp changing the behavior of transactionLogMaxSize to assume the default (16MB) when the value is 0 is technically a breaking change.

The oversized-transaction reader fix makes sense, but I don’t think we should redefine transactionLogMaxSize: 0 as part of it.

Harper doesn't appear to currently set a transactionLogMaxSize. Omitting the option already uses the 16MB default, which is how Harper uses it. Changing explicit 0 from unlimited segment size to the default changes existing configurations and can make previously retained history eligible for deletion.

Can we preserve 0 while fixing oversized-transaction reads? The description explains how fixed-capacity mappings simplify the implementation, but doesn’t establish why unlimited segment size must be removed.

If preserving unlimited growth is impractical, then maybe a value of 0 should throw an error.

[kriszyp]

Changing explicit 0 from unlimited segment size

But I think the key design invariant (and how the overflow was designed) is that we should be able to create a memory map, hand it to JS, and it should never grow (it either starts larger than max size, or never extends beyond). And naturally you can't allocate an "unlimited" size memory map (you can allocate a pretty big one though!)

Ironically I actually thought 0 should go the other way: it should mean that every transaction should get its own log segment/file (every transaction exceeds the max size of 0). Did claude weigh our different expectations and try to split it 🤔 .

If preserving unlimited growth is impractical, then maybe a value of 0 should throw an error.

Sounds good to me 👍 .

[kriszyp]

@cb1kenobi Thanks for catching this — agreed, quietly redefining 0 was the wrong trade. Preserving unlimited growth turned out not to be practical, so this now takes your fallback: transactionLogMaxSize: 0 fails open() with a migration hint (460937cd, README note in fe4362a7).

Why 0 can't stay unlimited

• A reader's buffer now has a fixed capacity. An unbounded segment would need either reader remapping as it grows (the complexity this PR removes) or, on Windows, pre-extending the active file on disk to the whole reservation. Windows read-only mappings cannot exceed the file's size.

What 0 actually did on main (for the record, it was only half-working)

• Readers mapped the current segment at capacity 0 (getMemoryMap → returns null for size 0), so query() read nothing from it.
• With no size rotation, a segment that reached 4 GiB failed every later append (maximum supported file size).
• Age rotation after a write-idle gap still rotated, and retention then purged.

Migration

• The README now says to omit the option (16 MB default) or set a size. It also warns that once the store rotates by size, retention purges rotated segments, so a deployment that wants more history should raise transactionLogRetention.
• As you noted, Harper never passes this option (only two unit tests set 128).
• It is still a breaking option change. I flagged release handling (semver-major or a release note) for @kriszyp in the PR description.

— Claude Opus 5.5

[cb1kenobi]

Looks great!