Skip to content

Prevent heap exhaustion when boot replay scans already-durable records - #2988

Merged
kriszyp merged 2 commits into
mainfrom
fix/bounded-boot-replay-2950
Oct 2, 2026
Merged

kriszyp merged 2 commits into
mainfrom
fix/bounded-boot-replay-2950

Conversation

@kriszyp

@kriszyp kriszyp commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Boot replay behind a stale transaction-log watermark now reads record bases without populating the primary-record cache. This prevents decoded records from accumulating on the V8 heap across already-durable commits while preserving synchronous recovery and native transaction boundaries.

Closes #2950

For the human reviewer

  1. Requirement and scope: the per-native-commit fix from fix(replay): replay each native commit separately so boot recovery can't run out of heap #2788/Replay each native commit separately so boot recovery can't run out of heap (#2788 → v5.2) #2824 is already on main, but cache WeakRef targets remain alive for the whole synchronous replay job. This PR addresses that reproduced heap failure. The retry’s slow out-of-order reconciliation abort, native mapped-log RSS, and one enormous transaction remain separate limitations; total startup memory is not bounded by this change.
  2. Cache policy: DatabaseTransaction.save uses the existing transaction handle with uncachedRead for every replay, including writes without reloadCommitBase. Periodically yielding would require broader startup/concurrency changes because database opening relies on synchronous replay. Look hardest at the snapshot and staging invariants: replay operations reach this read without a preloaded cached base. First reads after boot can be cold; the policy is independently reversible.
  3. Evidence: cache identity plus post-GC heap growth isolates decoded-record retention. The 16 MiB margin distinguishes the observed 66 MiB base regression across 8,000 commits. Review raised future V8 sensitivity, with no observed flake; retain the margin unless contrary runtime evidence identifies a better assertion.

Planning review: Framing-Verdict: chosen-approach-sound. Post-review delta: seven changed lines in the reviewed test, comment, and index, not re-reviewed because they only address the seed-count finding and documentation nits; production behavior is the reviewed behavior.

Changes

DatabaseTransaction.save forces uncached replay snapshot reads and explains why eviction cannot release WeakRef targets during the job. resources/DESIGN.md records that invariant and its tests; root DESIGN.md indexes it in document order. No API, configuration, or storage-format change needs a companion documentation PR.

Verification

  • End-to-end route: replayCommitBoundaries.test.js runs real writer/replayer processes with Node flags for --expose-gc. replayCommitBoundaries-crash.js adds stale-watermark modes: save an actual earlier checkpoint, durably write 10,000 records with 8 KiB payloads, write an unflushed tail, and SIGKILL. Reopening scans all backlog/tail commits (optionally the seed), recovers the tail, and checks every row’s contents. Its existing call-through commit observer also samples heap after GC at commits 1,000 and 9,000; prefix dispatch retains the original modes.
  • The cache-identity test also covers replay writes without reloadCommitBase. Both new tests fail on base at the expected assertions; heap grows from 34,793,176 to 104,220,488 bytes.
  • Final npm run build and three focused replay suites: pass, 61 tests. Full resource suite: 3,920 passing on RocksDB; 3,007 passing on LMDB.
  • Full core gate: initial run 6,384 passing with credential-environment and global-path-spy failures; rerun without the session’s Git settings 6,385 passing with one different deployment-recovery failure. The original two checks passed on rerun; the JWT and deployment-recovery suites also pass in isolation (5 and 20 tests). All failures are in unchanged tests; the full local core gate is not claimed green. Matching base unit CI is green.
  • Full integration gate: 2,286 passing, six cancelled by the optional Ollama suite’s setup failure (ERR_IMPORT_ATTRIBUTE_MISSING in unchanged source JSON imports). Its direct source import fails before any replay code executes. The full local integration gate is not claimed green; matching base integration CI is green.
  • PR CI: 48 checks pass and six are skipped; Unit Test (Node.js v22) is red. That job’s main step printed 6,394 passing / 201 pending / no test failures, then GitHub reported the four-minute step timeout. Both new regressions passed in its resource-core step; Node 24 and 26 unit jobs also pass. The remaining red check comes from the main-suite time budget.
  • CI lint (npm run lint:required), changed-file strict lint, formatting, design-document checks, and git diff --check: pass. Strict full-tree npm run lint reports 13 warnings in unchanged files.

Complexity: complicated

🤖 Generated with Codex.

Review-Coverage: authored=unknown; ran=none; rounds=1 @ 6c5f176

Human-Review-Need: 4 @ 6c5f176

kriszyp and others added 2 commits October 2, 2026 12:18
Replay snapshot reads must avoid WeakRefs that retain already-durable records throughout the synchronous boot scan. Add cache identity and crash-reopen heap-growth regressions for stale transaction-log watermarks.

Co-Authored-By: GPT-6.1 Codex <noreply@openai.com>
Allow the seed to replay while still requiring all durable backlog commits and the unflushed tail. Clarify the cache lifetime invariant and put its design index entry in document order.

Co-authored-by: GPT-6.1 Codex <noreply@openai.com>
@kriszyp kriszyp added this to the v5.3 milestone Oct 2, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request ensures that boot replay reads bypass the primary-record cache by setting uncachedRead to true when this.isReplay is active in DatabaseTransaction.ts. This prevents WeakRef targets from retaining the scanned backlog in memory during synchronous boot replay, keeping heap growth bounded. Corresponding design documentation has been updated, and unit tests have been added to verify that replay reads bypass the cache and that heap growth remains bounded during replay of a stale watermark. I have no feedback to provide as the changes are well-implemented and align with the repository's style guidelines.

@kriszyp
kriszyp marked this pull request as ready for review October 2, 2026 23:16
@kriszyp
kriszyp merged commit a95b34d into main Oct 2, 2026
56 of 57 checks passed
@kriszyp
kriszyp deleted the fix/bounded-boot-replay-2950 branch October 2, 2026 23:16
@claude

claude Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Boot replay on 5.3.0 still exhausts the main-thread heap when the log is far behind txn.state

1 participant