Skip to content

fix(rekey): preserve retained historical roots - #15

Merged
farhan-syah merged 3 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/rekey-retained-history
Jul 26, 2026
Merged

fix(rekey): preserve retained historical roots#15
farhan-syah merged 3 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/rekey-retained-history

Conversation

@presempathy-awb

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

Copy link
Copy Markdown
Contributor

fix(rekey): preserve retained historical roots

Summary

Online rekey now preserves every data and catalog B+ tree root named only by
retained commit-history metadata. Traversal state is shared across the current
and historical roots so copy-on-write pages are authenticated and re-sealed
once, and the walk rejects reserved references, cross-kind aliases, and
overflow cycles rather than skipping them.

The observable guarantee: after rekey_db returns success, the database may
retire the source epoch, drop, reopen, and still serve every retained
begin_read_at snapshot.

Problem

The commit-history index persists five fields per retained commit: data root,
catalog root, free-list root, next-page bound, and commit timestamp.
begin_read_at resolves a historical commit through that index and opens its
persisted data and catalog roots, which copy-on-write has usually made
unreachable from the current header.

Rekey rewrote the current data tree, the commit-history index tree, and the
current free-list chain, but never followed the historical roots stored inside
the index's values. That produced a partial success:

  1. rekey_db returns Ok(());
  2. the target header becomes authoritative;
  3. the source epoch retires once reader leases permit;
  4. begin_read_at still finds the requested commit metadata;
  5. the first historical page read requires the retired source key and fails
    with MissingPersistedKey { mk_epoch: 0, cipher_id: 1 }.

The failure survives drop and reopen, so it is durable state rather than an
in-process cache artifact: the history metadata promises a snapshot whose pages
the completed rekey left unreadable.

Implementation

rewrite_rekey_main_pages walks the current data tree, then delegates to
rewrite_retained_history_roots, which rewrites the history index and then, for
every retained row, walks the two reader-visible roots — active_root_page_id
and catalog_root_page_id.

Each historical tree is opened with its own persisted next_page_id rather than
the current allocator bound, so traversal stays faithful to the snapshot
metadata instead of silently widening an old tree's addressable page space.

Shared traversal state. BTree::rekey_walk is unchanged for ordinary
callers; it delegates to rekey_walk_unique, which takes a shared
BTreeMap<u64, PageKind>. Rekey supplies one map for the current data tree, the
history index, and every retained root. Snapshots share most of their physical
pages by construction, so this is what keeps the work proportional to unique
reachable pages instead of to the number of retained commits.

The map stores each page's kind, not just its id, because the two repeat cases
are not the same defect. A page already walked as a node and referenced again as
a node is an ordinary snapshot share and is skipped. The same id presented under
a different role means one of the two references survived the page being freed
and reused, and is reported as CorruptionDetail::PageKindAliased, naming the
page and both roles. A plain id set could not tell them apart.

Overflow traversal is a focused helper that rejects reserved root and chain
identifiers, authenticates the root under its exact kind, decodes each next
pointer before rewriting, rejects a repeated chain page, skips an
already-completed shared root, and returns the number of pages it rewrote.
Because overflow roots are reference-counted, a chain reached again through
another leaf arrives at the same root and is skipped whole — so a chain page
reached twice is never a legitimate share, and neither a loop nor two roots
claiming one page is treated as a stopping condition.

Internal zero child slots remain absent pointers and are not traversed, matching
the authoritative reachability collector.

Bounded history scan. Retention can be unbounded, so the rows are streamed
in fixed-size batches through the new BTree::collect_batch_from(start, limit)
rather than collected whole. Resuming on key ‖ 0x00 — the exact successor in
the key ordering — means paging never skips or repeats a record. Like
collect_all, it has no upper key bound, for the same reason: no concrete
maximum key lies outside the valid domain.

Kind agreement. The walk reads through read_node_guard, the only accessor
that proves a page's authenticated envelope kind and its encrypted body header
agree, and takes both the recorded kind and the re-seal kind from that one
verified value. Deriving them from different sources would let a mis-routed page
be laundered into a freshly authenticated one, and would leave the traversal map
describing a kind no longer on disk.

Historical free-list roots are intentionally excluded

The free_list_root_page_id in a history row is not reader-visible snapshot
state — historical reads consume only the data and catalog roots. Superseded
free-list chain pages are writer-only metadata that a later commit may already
have recycled.

Following those stale identifiers is not conservative. It can reinterpret a
reused data or tree page as PageKind::Free, fail a healthy rekey, or rewrite
the wrong live object. Only the current header's live chain is walked.

This is safe in the other direction because the reclamation floor is gated by
retained-history roots as well as by live readers, so a retained row cannot name
a page that was recycled underneath it.

Failure semantics

For retained history, rekey now performs work the existing success contract
already required. Additional authenticated-read, decode, reserved-reference,
kind-alias, or cycle failures surface before source retirement. Failing closed
is required: completing a rekey after only a partial historical walk recreates
the unreadable-snapshot defect this patch exists to remove.

Databases with history disabled, or with no retained historical root, keep the
same observable behavior. BTree::rekey_walk's signature is unchanged. Page
encoding, commit-history record layout, header fields, and on-disk compatibility
are untouched; existing databases need no migration.

One semantic change worth naming: rekey_walk's returned count is now unique
pages rewritten
. Overflow roots are reference-counted and can be shared between
leaves of one tree, and the previous implementation rewrote and counted such a
chain once per referencing leaf. Nothing in-tree consumes the return value.

Coverage

tests/rekey_basic.rs adds an end-to-end regression. It opens with unbounded
history, changes a realm quota, and commits 256 ordered keys (forcing internal
nodes), an inline value, and a three-page overflow value. It changes the quota
again — so the retained and current snapshots name different catalog roots — and
commits replacement inline and two-page overflow values.

After rekey it drops and reopens the database, then requires the first retained
transaction to read the old inline value, the old overflow bytes, and a key
through the historical internal tree, and to authenticate its historical catalog
by returning NotFound for an absent segment. A separate latest read must return
the replacement values. This catches an implementation that migrates only data,
only leaves, only inline values, only the current snapshot, or only the history
index.

Four unit tests in src/btree/tree/maintenance.rs cover the traversal
properties directly, since none of them are reachable through the public API:
shared-page dedup across roots, a control proving an unshared walk still does the
full work, a cross-kind alias reported rather than silently skipped, and a page
whose envelope contradicts its body being refused instead of re-sealed. A test in
page_kind.rs asserts kind names are unique, since two kinds sharing a name
would make an alias error contradict itself.

Verification

On the merged head:

  • cargo nextest run --all-features --no-fail-fast: 452 passed, 5 skipped;
  • the same suite with PAGEDB_INVARIANT_CHECKS=1: 452 passed, 5 skipped;
  • cargo fmt --all --check: pass;
  • cargo clippy --all-targets --all-features -- -D warnings: pass;
  • cargo doc --no-deps --all-features: no new warnings;
  • doctests: pass;
  • checklist-scaffold gate: pass.

The kind-agreement regression was confirmed to fail with its fix reverted and
pass with it restored.

Cross-target checks, feature-matrix builds, cargo deny, and the equal-behavior
rekey benchmark (150 samples and 18,000 epoch rotations per subject, showing no
demonstrated regression: mean -0.076%, median -0.142%, P95 +0.148%) were run by
the original author against candidate head fc167a2, before this branch merged
main and took the review fixes above. The full CI matrix re-runs them on the
current head.

Scope exclusions

Historical free-list traversal; format or migration changes; segment-retention
policy changes; and bounding the traversal map itself. On the last: the map is
O(unique pages), but the same walk already holds every rewritten page dirty in
the buffer pool until the single flush_main — a whole decrypted page each,
against a page id and kind here. Removing it would make rekey O(roots × pages)
of AEAD to save under 1% of what rekey already holds.

The patch carries one reviewable guarantee: a successful rekey leaves every
retained historical data and catalog root readable after source-key retirement.

@presempathy-awb
presempathy-awb force-pushed the codex/upstream/rekey-retained-history branch from a161f2d to fc167a2 Compare July 26, 2026 15:39
Rekeying the commit-history index used to collect every retained row
into memory before rewriting the roots it names, making resident cost
proportional to retention depth. Stream the rows in fixed-size batches
via a new bounded scan primitive instead, and detect a page reached
under two incompatible kinds during the shared traversal as a reported
corruption rather than a generic illegal-kind error.
@farhan-syah
farhan-syah merged commit 46f7a8f into NodeDB-Lab:main Jul 26, 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