Skip to content

fix(storage): harden B-tree and chain integrity - #21

Merged
farhan-syah merged 3 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/btree-structural-integrity
Jul 26, 2026
Merged

fix(storage): harden B-tree and chain integrity#21
farhan-syah merged 3 commits into
NodeDB-Lab:mainfrom
presempathy-awb:codex/upstream/btree-structural-integrity

Conversation

@presempathy-awb

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

Copy link
Copy Markdown
Contributor

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 Err left no new logical state
behind.

Four boundaries are hardened without changing the durable format:

  1. disk-owned lengths and pointers are checked before they control allocation or
    traversal;
  2. every root-to-leaf or linked-page walk has an explicit termination invariant;
  3. structurally impossible caller input is rejected before page allocation or
    cache mutation;
  4. fallible old-value cleanup is rolled back before an error reaches the caller.

No new dependencies, feature flags, public types, file-format fields, or
migration requirements.

Chains that never terminate

overflow::read_chain converts the declared total length through checked
platform 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 overflow panic and a typed error: durable metadata must not choose an
arbitrarily large allocation before a single chain page has been verified.

The same visited-page rule protects overflow::release and
overflow::collect_chain, so the cleanup and maintenance variants of a cycle
are rejected even where the read path would already have caught it, and
pager::freelist::read_chain rejects 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_range advances using the authoritative parent path rather than relying
on leaf sibling links alone. That matters before flush, because leaves created
by same-session splits live in fresh_leaves and intentionally have no sibling
links 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 existing
OverflowChainCycle. A generic corruption variant would tell an operator that
something is wrong without saying where to look.

The sibling/parent disagreement in delete_range is deliberately not a cycle
and gets its own LeafSiblingMismatch { leaf_page_id, right_sibling, parent_next }. Both fields encode "the next leaf", so both must name it; a
disagreement means one of them outlived the page it points at.

Rejection before mutation

Leaf::single_record_fits_encoded, internal::separator_entry_size, and
internal::separator_fits express the two format constraints a key must
satisfy: 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, and bulk_load apply this preflight before writing
overflow 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. A
preflight 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 of
AppendNotMonotonic, same invariant, different entry point. Unsorted input does
not 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 LeafValue is restored under the same key
before 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_append monotonicity

put_append's invariant now holds across sessions, not only within one. On the
first 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 get and put; an internal separator that
cannot 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::timeout is not guaranteed to preempt a
malformed loop.

Verification

On this branch, rebased onto main:

  • cargo nextest run --all-features --no-fail-fast: 599 passed, 5 skipped;
  • the same suite under 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.

base:   cold_tree_get 443 ns
head:   cold_tree_get 458 ns
change: +3.48% (+15 ns), against a 10% advisory threshold

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

  1. the fit preflight must stay aligned with the durable leaf and internal
    encodings — the shared constants are what enforce that;
  2. next_leaf_after and delete_range must accept zero sibling links for fresh
    leaves while rejecting disagreement from nonzero persisted links;
  3. rollback must restore the old logical value before returning the original
    release error;
  4. the bulk-load no-progress guard must reject only a non-shrinking group, not a
    valid final one-child remainder;
  5. cycle tracking must stay local to one traversal, so healthy repeated
    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.

presempathy-awb and others added 3 commits July 27, 2026 07:14
…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
farhan-syah force-pushed the codex/upstream/btree-structural-integrity branch from dc00f15 to 4e8ee7c Compare July 26, 2026 23:51
@farhan-syah
farhan-syah merged commit f9e6cfa 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