Repository navigation
feat(storage): generation pins and page quarantine; fact logs no longer defer checkpoints (#543, PR 2 of 3) - #579
Merged
Merged
Conversation
…er defer checkpoints (#543) PR 2 of 3 for #543 (plan steps 2, 3, 4, 6, 7). - src/storage/pins.rs: Pins (generation -> open pins, generation -> ids its checkpoint freed) and the RAII GenerationPin. An entry stays in force while a lower generation is pinned; it is dropped at publish or on release. - PageAllocator::with_quarantine: ids in force are never handed out and are listed again by finish_free_list. - save() and rebuild_indexes() allocate around pages_in_force() and record their freed pages in publish (under FactData's write lock). - FactStorage::snapshot() takes the committed reader, its pin and the pending store with its length under one FactData lock. FactLog reads from it; fact_log() no longer takes the write lock. LogPin, pin_log, log_pins and log_snapshot are gone, and so is the API-013 refusal in save/rebuild and the auto-checkpoint deferral in execute and WriteTransaction::commit. - API-013 deprecated: still registered, no longer emitted. Tests: pins unit tests, allocator quarantine tests, PFS tests that check by page id that saves and rebuilds never write a pinned generation's pages and reuse them after the pin is dropped; fact log tests for checkpoints, rebuilds, auto-checkpoints and concurrent writers with a log open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR
…andle (#543) Also notes at the browser release point that quarantined ids must be withheld once browser reads pin (plan step 8, Codex review). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This was referenced Oct 13, 2026
Merged
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.
Part of #543 (streaming engine 1/9, #473): PR 2 of 3, plan steps 2, 3, 4, 6, 7 (
docs/superpowers/plans/2026-10-11-543-snapshots-and-quarantine.md). PR 1 was #578.What changes
src/storage/pins.rs—Pinsmaps each generation to its open pins, and each generationgto the page ids its checkpoint freed (the quarantine entry).GenerationPinis RAII. An entry stays in force while some lower generation is pinned. It is dropped at publish if no lower generation is pinned, or later when the last such pin is released.PageAllocator::with_quarantine— never hands out an id in force, free-list chain reads included.finish_free_listlists skipped ids again, so nothing leaks, and a crash or reopen (no pins) reuses them at once.save()/rebuild_indexes()— allocate aroundpages_in_force(). Each records its freed pages inpublishunderFactData's write lock (save:freed; rebuild: the old referenced pages minus the new reached ones). The rebuild pool also leaves out ids in force.FactStorage::snapshot()— takes the committed reader, its pin, and the pending store with its length under oneFactDatalock.Snapshotonly has what the fact log needs for now; PR 3 (step 5) extends it for queries and cursors.Snapshot.Minigraf::fact_logno longer takes the write lock. Removed:LogPin,pin_log,log_pins,log_snapshot, the API-013 refusal in save/rebuild, and the auto-checkpoint deferral inexecuteandWriteTransaction::commit.API-013deprecated — still registered and documented, no longer emitted (same treatment as STG-003/004). Updated:ERROR_REFERENCE.md,DURABILITY.md, CHANGELOG, ROADMAP, CLAUDE.md, TEST_COVERAGE.Behaviour change (v3-only, unreleased): checkpoints now run while a fact log is open. Instead of a growing WAL, the file grows by the pages checkpoints replace until the log closes; later checkpoints reuse those pages.
Tests (1458 total: 1448 passing, 10 ignored)
pinsunit tests: entry in force under a lower pin, dropped at once with no pin, two pins released in either order.PersistentFactStorage(page-id level): saves, rebuilds, and the two alternated never write a page the pinned generation reads. After each one the pinned reader returns the same facts,assert_space_accountedholds andverify()is clean. After the pin is dropped, the held pages are reused.fact_log_test:WriteTransactionin_force()returning nothing, 3 fact-log tests and all 3 PFS pin tests fail.snapshot_atomicity_test+fact_log_testlooped 15×: no failures. Clippy is clean for native, wasm32-unknown-unknown (browser) and wasm32-wasip1.Codex review
One finding:
save()hands the browserlisted, which includes quarantined ids, and the browser drops those pages from IndexedDB. It can't happen in this PR: the only pin taker isMinigraf::fact_log, andBrowserDbhas no fact log. It becomes reachable when cursors pin in PR 3, whose step 8 (take_releasedwithholds ids in force) fixes it. There's a comment at the spot. No other findings.Not in this PR
PR 3 (
feat/543-snapshots-read): query/cursor snapshots,BrowserDbwithholding, crash test with an open snapshot, docs-site and #421 updates.No public API change and no new error code, so no binding cascade.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HyLscCUuJ1GQpdAeX79EdR