Repository navigation
Prevent heap exhaustion when boot replay scans already-durable records - #2988
Merged
Merged
Conversation
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>
Contributor
There was a problem hiding this comment.
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.
Contributor
|
Reviewed; no blockers found. |
This was referenced Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
DatabaseTransaction.saveuses the existing transaction handle withuncachedReadfor every replay, including writes withoutreloadCommitBase. 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.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
--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.reloadCommitBase. Both new tests fail on base at the expected assertions; heap grows from 34,793,176 to 104,220,488 bytes.npm run buildand three focused replay suites: pass, 61 tests. Full resource suite: 3,920 passing on RocksDB; 3,007 passing on LMDB.ERR_IMPORT_ATTRIBUTE_MISSINGin 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.npm run lint:required), changed-file strict lint, formatting, design-document checks, andgit diff --check: pass. Strict full-treenpm run lintreports 13 warnings in unchanged files.Complexity: complicated
🤖 Generated with Codex.
Review-Coverage: authored=unknown; ran=none; rounds=1 @ 6c5f176
Human-Review-Need: 4 @ 6c5f176