Skip to content

fix(txn): preserve cache and transaction lifecycle state - #23

Merged
farhan-syah merged 3 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/transaction-cache-lifecycle
Jul 27, 2026
Merged

fix(txn): preserve cache and transaction lifecycle state#23
farhan-syah merged 3 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/transaction-cache-lifecycle

Conversation

@presempathy-awb

@presempathy-awb presempathy-awb commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Six lifecycle defects that leave PageDB reporting, retaining, or protecting
state that no longer matches the durable store.

  1. Db::set_realm_quotas() publishes a header with both commit-history root
    fields zeroed. Retained history stays readable through the live handle's
    in-memory state and disappears at the next open.
  2. Db::evict_main_pages() routes to the dirty-discard helper, which clears
    dirty markers and evicts nothing. The next read it was meant to force to
    storage is still served from cached plaintext.
  3. SIEVE eviction bounds its scan by the configured capacity even when pinned
    or dirty pages have legitimately grown the live cache past it, so a clean
    victim that is present can be unreachable.
  4. A WriteTxn dropped without commit() or abort() clears its dirty pages
    but leaves the spill-byte gauge nonzero.
  5. That same drop leaves its scratch file behind, and nothing ever removes it.
    The scratch of every transaction that dies with its process accumulates for
    the life of the store.
  6. The spill key is derived from the master key, the durable file_id, and the
    transaction 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() copies commit_history_root_page_id and
commit_history_root_version from writer state into the new A/B header instead
of writing zeros. It still leaves latest_commit_id alone and publishes only
its catalog-root update; it no longer drops unrelated durable roots on the way
past.

This is the only out-of-band HeaderFieldsParams construction in the crate. The
compaction 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

PageCache grows a clean-only clear alongside the existing full clear, sharing
one 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 that
is 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

Drop resets the spill gauge. It describes the live writer's scratch, and the
writer lock the drop is about to release means there is no other writer to
account for.

Drop cannot await, so it cannot remove the scratch file. It records the exact
path instead, and the next begin_write — the first point that both holds the
writer 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 crate
writes, 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

cargo fmt --all --check
cargo clippy --all-targets --all-features -- -D warnings   # clean
cargo nextest run --all-features --no-fail-fast            # 725 passed, 10 skipped
cargo check --target wasm32-unknown-unknown --features opfs --lib
cargo check --target wasm32-wasip1 --lib
cargo doc --no-deps --all-features                         # no warnings
cargo test --doc --all-features

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.

presempathy-awb and others added 3 commits July 27, 2026 19:48
…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
farhan-syah force-pushed the codex/upstream/transaction-cache-lifecycle branch from bad893f to 1bb5ccc Compare July 27, 2026 12:40
@farhan-syah
farhan-syah merged commit 031db94 into NodeDB-Lab:main Jul 27, 2026
19 checks passed
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.

2 participants