fix(txn): preserve cache and transaction lifecycle state - #23
Merged
farhan-syah merged 3 commits intoJul 27, 2026
Merged
Conversation
…ratch The transaction sequence restarts at 1 on every open and doubles as the spill nonce, so a spill key derived only from the durable file id and that sequence would repeat across two opens of one store at the same sequence, and the nonce built from it would repeat with it. Mix a random per-open epoch into the derivation so the key is distinct per open, leaving the sequence free to serve as the nonce. Pair this with reclaiming the scratch such keys ever protected: a transaction dropped without commit or abort cannot await the removal, so it now hands its exact path to the next begin_write, and whatever still survives at the next open is swept outright, since a spill file is never read back across handles.
evict_clean_main_pages and its cache-layer counterpart are exercised only by tests, so mark them #[cfg(test)] rather than shipping them in the crate's public build surface. Move the test that exercises them into the unit tests beside Db, per this crate's convention that implementation-detail coverage lives with the code it tests.
farhan-syah
force-pushed
the
codex/upstream/transaction-cache-lifecycle
branch
from
July 27, 2026 12:40
bad893f to
1bb5ccc
Compare
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.
Summary
Six lifecycle defects that leave PageDB reporting, retaining, or protecting
state that no longer matches the durable store.
Db::set_realm_quotas()publishes a header with both commit-history rootfields zeroed. Retained history stays readable through the live handle's
in-memory state and disappears at the next open.
Db::evict_main_pages()routes to the dirty-discard helper, which clearsdirty markers and evicts nothing. The next read it was meant to force to
storage is still served from cached plaintext.
or dirty pages have legitimately grown the live cache past it, so a clean
victim that is present can be unreachable.
WriteTxndropped withoutcommit()orabort()clears its dirty pagesbut leaves the spill-byte gauge nonzero.
The scratch of every transaction that dies with its process accumulates for
the life of the store.
file_id, and thetransaction sequence, and the spill nonce is built from that same sequence.
The sequence is process-local and restarts at 1 on every open, so two opens
of one store encrypt different scratch payloads under one key at one nonce —
and (5) is what leaves the earlier ciphertext on disk to be paired with.
No dependency, on-disk format, configuration value, feature flag, background
task, or public API change. The existing architecture and handle-mode matrix are
unchanged.
Header publication
set_realm_quotas()copiescommit_history_root_page_idandcommit_history_root_versionfrom writer state into the new A/B header insteadof writing zeros. It still leaves
latest_commit_idalone and publishes onlyits catalog-root update; it no longer drops unrelated durable roots on the way
past.
This is the only out-of-band
HeaderFieldsParamsconstruction in the crate. Thecompaction paths also write zeros there, but they zero the matching in-memory
writer state in the same breath, because repacking moves the pages those roots
name — that is a deliberate discard, not this defect.
Covered by writing two commits under unbounded retention, publishing quotas,
then reading the first commit both through the live handle and after reopening
the same VFS. Exercising both sides is the point: the old behavior looks correct
until writer state is reconstructed from the damaged header.
Buffer-pool eviction
PageCachegrows a clean-only clear alongside the existing full clear, sharingone body: pinned entries are always spared, and a flag decides whether unflushed
entries are spared too or dropped with their dirty markers. The pager and handle
accessors route to it, so the diagnostic helper now does what its name says —
after durable bytes are inspected or deliberately corrupted, the next read misses
the buffer pool and authenticates what is on disk.
The whole clean-only chain is
#[cfg(test)], matching the handle accessor thatis its only caller. Correctness never depends on whether a page is warm, so
nothing in a build an embedder links against should be steering that.
The SIEVE hand still makes at most two passes, but over the live map rather than
the configured capacity. Temporary over-capacity growth is an explicit, already
documented state of this cache; once a clean unpinned entry appears there, the
loop has to be able to reach it. At or below capacity the two bounds are equal,
so the common path is untouched, and saturating arithmetic keeps the scan finite.
Two unit tests: a capacity-two cache forced to five entries with four pinned,
where the clean fifth must still be evicted; and a clear that removes clean
entries while dirty entries and their markers survive.
Dropped write transactions
Dropresets the spill gauge. It describes the live writer's scratch, and thewriter lock the drop is about to release means there is no other writer to
account for.
Dropcannot await, so it cannot remove the scratch file. It records the exactpath instead, and the next
begin_write— the first point that both holds thewriter lock and can await — removes what it finds. Removal stays best effort, as
it is in commit and abort: a scratch file nothing references cannot make the
durable store unopenable. Paths that fail to remove are not requeued, since a
path that cannot be removed now will not become removable by retrying it on
every subsequent transaction.
That closes the in-process case but not the one that matters most. A handle can
close, or a process can die, with scratch still on disk and no later transaction
to collect it. Open now sweeps
tmp/for the scratch-file names this cratewrites, gated to handles with standalone recovery authority — which is exactly
the mode holding the exclusive writer sentinel, so the sweep cannot race a live
writer. Unrelated names under
tmp/are left alone.Spill key scope
A spill file is scratch for one live transaction and is never read back by a
later one, so the key protecting it has no reason to outlive the handle that
wrote it. It now carries a random per-open epoch that is generated at open and
never persisted.
That is what makes the transaction sequence a sound nonce source. The sequence
alone is not one: it restarts at 1 on every open, and deleting the file does not
change that — the file is exactly what a crash leaves behind. With a per-open key
a repeated sequence lands under a different key, so no durable nonce anchor is
needed, and the scratch a crash leaves is swept rather than decrypted.
Comments asserting the previous, weaker rationale are corrected rather than left
to be believed by the next reader.
Covered by key-separation tests across opens at one sequence, across sequences
within one open, and across stores; by a non-repeating epoch test; and by an
end-to-end test that drops a transaction with spill, closes the handle, asserts
the scratch genuinely survives, and then asserts the next open reclaims it.
Verification
All four VFS backends agree that listing an absent directory yields no entries,
so the open sweep is a no-op on a store that has never spilled.
Compatibility
No migration and no on-disk format change. Stores whose history roots are intact
open identically. The header repair applies to future publications; it cannot
reconstruct roots a previous quota update already zeroed.
Spill keys are not compatible across opens, which is the point of the change and
costs nothing: no code path has ever read a spill file written by an earlier
open.
No README or changelog update is needed. The change restores the behavior the
existing public and inline contracts already describe, and corrects the
evict_main_pages()documentation to state its clean-only semantics.