fix(storage): harden B-tree and chain integrity - #21
Merged
farhan-syah merged 3 commits intoJul 26, 2026
Merged
Conversation
…ow records Give the sizing math a name tied to the layout it mirrors: the separator record overhead and slot-directory entry size internal nodes use to decide whether a key fits, and the overflow-reference encoded size leaf values use for the same purpose. Previously these were repeated as bare arithmetic in both the preflight check and the encoder, free to drift apart silently.
Replace the generic HeaderUnverifiable/IO stand-ins used for btree descent cycles, leaf sibling-link mismatches, and non-monotonic bulk_load input with dedicated PagedbError variants that name the walk (btree_descent, leaf_siblings, free_list) and the pages involved, so fsck and callers can act on what actually went wrong. Also stop discarding put_append's cached rightmost path before its monotonic check has passed, so a rejected append leaves the session exactly as it found it, and log (rather than silently drop) an overflow chain that can't be reclaimed on a failed put.
farhan-syah
force-pushed
the
codex/upstream/btree-structural-integrity
branch
from
July 26, 2026 23:51
dc00f15 to
4e8ee7c
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.
Harden B-tree and chain integrity
Summary
PageDB now fails closed when authenticated storage bytes describe an impossible
B-tree or page-chain structure, and a live tree keeps its logical state when a
write is rejected.
These failures share one rule: authenticated bytes are not automatically valid
database semantics. AEAD and page-kind binding prove that bytes belong to the
right page, realm, and durable identity. They do not prove that a pointer graph
terminates, that a declared length is allocatable, that a separator can fit the
format, or that an operation which returned
Errleft no new logical statebehind.
Four boundaries are hardened without changing the durable format:
traversal;
cache mutation;
No new dependencies, feature flags, public types, file-format fields, or
migration requirements.
Chains that never terminate
overflow::read_chainconverts the declared total length through checkedplatform bounds, reserves at most one page up front, and rejects root or
continuation bytes that exceed the declared total before extending its output.
Growing only as bytes authenticate is the difference between a
capacity overflowpanic and a typed error: durable metadata must not choose anarbitrarily large allocation before a single chain page has been verified.
The same visited-page rule protects
overflow::releaseandoverflow::collect_chain, so the cleanup and maintenance variants of a cycleare rejected even where the read path would already have caught it, and
pager::freelist::read_chainrejects a repeated free-list page.Topology that never terminates
Point reads and path-building descents maintain operation-local visited-page
sets. A zero child page or a repeated page within one descent is impossible
topology and returns corruption instead of spinning. Independent operations may
of course revisit the same valid pages; there is no global state or lock.
The common shallow descent stays in a four-entry inline array, so point reads do
not allocate merely to enforce the invariant. Deeper trees spill into an exact
set rather than imposing a depth limit or weakening detection.
delete_rangeadvances using the authoritative parent path rather than relyingon leaf sibling links alone. That matters before flush, because leaves created
by same-session splits live in
fresh_leavesand intentionally have no siblinglinks yet. A nonzero persisted sibling is still validated against the
parent-mediated next leaf.
Each structure names itself when it fails
A cycle is reported as
CorruptionDetail::PageChainCycle { structure, page_id },naming both the walk that found the loop —
btree_descent,leaf_siblings,free_list— and the page that repeated, alongside the existingOverflowChainCycle. A generic corruption variant would tell an operator thatsomething is wrong without saying where to look.
The sibling/parent disagreement in
delete_rangeis deliberately not a cycleand gets its own
LeafSiblingMismatch { leaf_page_id, right_sibling, parent_next }. Both fields encode "the next leaf", so both must name it; adisagreement means one of them outlived the page it points at.
Rejection before mutation
Leaf::single_record_fits_encoded,internal::separator_entry_size, andinternal::separator_fitsexpress the two format constraints a key mustsatisfy: the key plus its stored value representation must fit in an otherwise
empty leaf, and the key must also fit as an internal separator after a future
split.
put,put_append, andbulk_loadapply this preflight before writingoverflow pages, touching dirty or fresh caches, freeing pages, or changing the
root — so a rejected oversized key cannot poison the live tree, and dense bulk
load cannot discover an impossible separator after materialization has started.
The sizes come from named constants the encoders themselves use
(
OVERFLOW_REF_ENCODED_SIZE,SEPARATOR_RECORD_OVERHEAD,SLOT_DIRECTORY_ENTRY_SIZE) rather than being restated next to them. Apreflight that quietly disagrees with the layout it is predicting is worse than
no preflight.
Bulk load also validates adjacent-key order before any mutation. Descending keys
and duplicates return
PagedbError::BulkLoadNotMonotonic— the sibling ofAppendNotMonotonic, same invariant, different entry point. Unsorted input doesnot merely misplace a record; it would build a tree whose separators do not
describe its leaves.
The bottom-up internal builder uses checked separator sizing and rejects the
no-progress case where more than one child remains but the first separator
cannot fit. A final one-child remainder after earlier multi-child groups is
still allowed; only the level-width-preserving loop is rejected.
Rollback on a failed release
Replacement and delete use the existing refcount-aware overflow release. If that
fallible release fails, the old
LeafValueis restored under the same keybefore the original error is returned, and the unused copy-on-write page goes
back to the session free pool.
For a failed replacement the attempted new value may already own a fresh
overflow chain; that uncommitted chain is collected after the old value is
restored. Cleanup never masks the original error — and when cleanup itself
fails, it logs rather than returning, because the cost of giving up quietly is a
page leak nothing else will notice.
Successful release accounting is unchanged, and inline-value writes take no
extra path.
put_appendmonotonicityput_append's invariant now holds across sessions, not only within one. On thefirst append of a session — when there is no previously-appended key to compare
against — the largest key already in the tree serves as the bound. Both halves
are the same rule: the cached rightmost path is only meaningful while every
append lands to the right of everything before it. The doc comment states both.
A rejected append leaves the session exactly as it found it, cache included.
Regression coverage
New or strengthened B-tree tests cover cyclic leaf siblings during range delete;
cyclic internal children during
getandput; an internal separator thatcannot fit and would otherwise preserve level width forever; unsorted and
duplicate bulk input with unchanged allocation state and a successful valid
retry; oversized-key rejection for both regular and append writes followed by
successful writes and flush; replacement and delete rollback when old-overflow
release fails; traversal of all fresh split leaves during pre-flush range
deletion; and append behavior after a regular write invalidates the cached
rightmost path.
Assertions name the exact corruption variant and, where it is meaningful, the
structure and page id. A test that accepts "some corruption" cannot tell a
precise diagnosis from a generic one — that is how a misclassification survives.
Overflow module tests cover impossible declared length, data exceeding the
declared total, and read, release, and collection cycles. The free list has a
bounded cycle regression.
The nontermination regressions run on an OS thread with a receive timeout rather
than an async timer. This is deliberate: an in-memory VFS future can remain
continuously ready, so
tokio::time::timeoutis not guaranteed to preempt amalformed loop.
Verification
On this branch, rebased onto
main:cargo nextest run --all-features --no-fail-fast: 599 passed, 5 skipped;PAGEDB_INVARIANT_CHECKS=1: 599 passed, 5 skipped;cargo fmt --all --check: pass;cargo clippy --all-targets --all-features -- -D warnings: pass;cargo check --no-default-features: pass;cargo check --target wasm32-unknown-unknown --features opfs --lib: pass;cargo doc --no-deps --all-features: no warnings.Performance
The only new work on a healthy point-read descent is operation-local cycle
detection, measured against the exact base with the existing authenticated
cold-node benchmark — same host, lockfile, release profile, benchmark source,
1,000-key multi-level tree, cold main-page cache per lookup, and 1,000
iterations.
An earlier implementation allocated a set for every multi-level read and
reproducibly exceeded the threshold by 21–25%. It was not retained. Keeping four
page ids inline brings the same workload back inside the gate without removing
any zero-page, repeated-page, or full-path validation.
Review focus
encodings — the shared constants are what enforce that;
next_leaf_afteranddelete_rangemust accept zero sibling links for freshleaves while rejecting disagreement from nonzero persisted links;
release error;
valid final one-child remainder;
operations are unaffected.
Risk
Cycle tracking adds bounded per-operation memory proportional to tree height or
chain length. Shallow descents stay inline; deeper descents and chain walks use
exact sets, and chain walks already pay page lookup and authentication costs
that dwarf one set insertion per page.
This changes malformed-data failure modes from panic, hang, or partial
publication into typed errors, and restores the already-documented sorted
bulk-load contract. It adds no new public surface beyond the error variants
those failures now carry.