Skip to content

Commit d8ff86a

Browse files
committed
refactor(errors): unify handle-mode gating and corruption reporting
Replace the scattered ad hoc mode checks (ReadOnly/Unsupported/IdentityForked returned inconsistently from begin_write, create_segment, rekey_db, compaction, and apply_incremental) with a single PagedbError::WrongMode variant and a DbModeCapabilities matrix that every gate resolves against through Db::require_mode, so an embedder gets one variant naming both the attempted operation and the mode that would have worked. Fold FooterUnverifiable and StagingMissing down to the identity they can actually authenticate (drop the unreliable embedder-visible name), add PageUnverifiable so a per-page AEAD failure names the failing page instead of collapsing into a bare ChecksumFailure, and drop the never-raised FreeListExhausted/SegmentTombstoneStalled variants. Classify std::io::Error at the one boundary every VFS call crosses so device exhaustion surfaces as NoSpace instead of hiding inside a generic Io error. Add a RichlyReported diagnostics guard so a site-specific corruption capture is not duplicated by the generic constructor-level one. Db::open (bootstrap-or-reopen decided from what is actually on disk) becomes the only production entry point; the open_internal*/open_existing* family that raced or skipped that decision moves behind #[cfg(test)]. OpenOptions gains a cipher selector for stores being created. Tighten apply_incremental's visibility window so reader admission stays closed continuously from the tombstone pin scan through snapshot publication, and add one-shot fault injection for the nonce-anchor commit/refresh path and two more rekey resume points, with tests exercising each interruption. Update every bench, benchmark, and integration test to the new API and error shapes.
1 parent f729a92 commit d8ff86a

65 files changed

Lines changed: 1679 additions & 511 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

benches/authenticated_node_read.rs

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
//!
44
//! Every timed lookup descends a multi-level tree with an empty main-page
55
//! cache, so each internal and leaf node costs a real decrypt plus the
6-
//! envelope/body kind agreement check in `BTree::read_node_guard`. The cache
7-
//! eviction that creates those conditions is deliberately *outside* the timed
8-
//! region — measuring it would charge cache bookkeeping to the read path.
6+
//! envelope/body kind agreement check in `BTree::read_node_guard`. The store is
7+
//! built once; each iteration opens its own handle over those same bytes, and a
8+
//! fresh handle starts with an empty buffer pool. Opening and retiring handles
9+
//! is deliberately *outside* the timed region — measuring it would charge
10+
//! handle setup to the read path.
911
//!
1012
//! Runs on `MemVfs`: the figure is CPU + AEAD for an authenticated cold
1113
//! descent, not the cost of reaching real storage.
@@ -28,60 +30,85 @@ use pagedb::{Db, RealmId};
2830
const PAGE: usize = 4096;
2931
const KEY_COUNT: usize = 1_000;
3032
const REALM: RealmId = RealmId::new([0x51; 16]);
33+
const KEK: [u8; 32] = [0xA5; 32];
3134
const VALUE: &[u8] = b"authenticated-node-read";
3235

3336
thread_local! {
34-
/// Built once and reused: this workload only reads, so rebuilding the tree
35-
/// per iteration would add setup noise without changing what is measured.
36-
static DB: RefCell<Option<Rc<Db<MemVfs>>>> = const { RefCell::new(None) };
37+
/// The populated store's bytes, built once: this workload only reads, so
38+
/// rebuilding the tree per iteration would add setup noise without changing
39+
/// what is measured.
40+
static STORE: RefCell<Option<MemVfs>> = const { RefCell::new(None) };
3741
}
3842

3943
fn key(index: usize) -> Vec<u8> {
4044
format!("node:{index:08}").into_bytes()
4145
}
4246

43-
fn shared_db() -> Rc<Db<MemVfs>> {
44-
DB.with(|cell| {
47+
fn populated_store() -> MemVfs {
48+
STORE.with(|cell| {
4549
if cell.borrow().is_none() {
46-
let db = block_on(async {
50+
let vfs = MemVfs::new();
51+
block_on(async {
4752
// Commit history is irrelevant here and would only add
4853
// unrelated catalog writes to the setup.
4954
let options =
5055
OpenOptions::default().with_commit_history_retain(RetainPolicy::Disabled);
51-
let db =
52-
Db::open_internal_with_options(MemVfs::new(), [0xA5; 32], PAGE, REALM, options)
53-
.await
54-
.expect("open bench store");
56+
let db = Db::open(vfs.clone(), KEK, PAGE, REALM, options)
57+
.await
58+
.expect("open bench store");
5559
let mut write = db.begin_write().await.expect("begin txn");
5660
for index in 0..KEY_COUNT {
5761
write.put(&key(index), VALUE).await.expect("insert");
5862
}
5963
write.commit().await.expect("commit");
60-
db
64+
// The writer handle drops with this block, releasing its
65+
// sentinel so the read-only handles below can attach.
6166
});
62-
*cell.borrow_mut() = Some(Rc::new(db));
67+
*cell.borrow_mut() = Some(vfs);
6368
}
64-
cell.borrow().as_ref().expect("db initialised").clone()
69+
cell.borrow().as_ref().expect("store initialised").clone()
70+
})
71+
}
72+
73+
/// A handle whose buffer pool is empty, over an already-populated store.
74+
fn cold_handle(store: &MemVfs) -> Db<MemVfs> {
75+
block_on(async {
76+
Db::open_read_only(store.clone(), KEK, PAGE, REALM, OpenOptions::default())
77+
.await
78+
.expect("open cold read-only handle")
6579
})
6680
}
6781

6882
#[bench(group = "pager/authenticated-node-read")]
6983
fn cold_tree_get(b: &mut Bencher) {
70-
let db = shared_db();
84+
let store = populated_store();
85+
// `Rc` so the timed region can lift the handle out of the cell *before* it
86+
// awaits: a `RefCell` borrow held across an await point is what
87+
// `clippy::await_holding_refcell_ref` rejects, and cloning the `Rc` costs a
88+
// refcount bump rather than moving the handle's teardown into the
89+
// measurement.
90+
let handle: RefCell<Option<Rc<Db<MemVfs>>>> = RefCell::new(None);
7191
let mut index = 0usize;
7292

7393
b.iter_with_setup(
7494
|| {
75-
// Untimed: drop the warm pages and pre-build the lookup key, so the
76-
// timed region is descent plus authentication only.
95+
// Untimed: retire the previous iteration's warm handle, open a cold
96+
// one, and pre-build the lookup key, so the timed region is descent
97+
// plus authentication only.
7798
let lookup_key = key(index % KEY_COUNT);
7899
index = index.wrapping_add(1);
79-
db.evict_main_pages(REALM);
100+
handle.borrow_mut().take();
101+
*handle.borrow_mut() = Some(Rc::new(cold_handle(&store)));
80102
lookup_key
81103
},
82104
|lookup_key| {
83105
with_rt(|rt| {
84106
rt.block_on(async {
107+
let db = handle
108+
.borrow()
109+
.as_ref()
110+
.map(Rc::clone)
111+
.expect("handle opened in setup");
85112
let read = db.begin_read().await.expect("begin read");
86113
read.get(&lookup_key).await.expect("get")
87114
})

benches/compaction.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use fluxbench::bench;
1414
use fluxbench::prelude::*;
1515

1616
use pagedb::vfs::memory::MemVfs;
17-
use pagedb::{Db, RealmId};
17+
use pagedb::{Db, OpenOptions, RealmId};
1818

1919
const PAGE: usize = 4096;
2020
const KEK: [u8; 32] = [7; 32];
@@ -33,7 +33,7 @@ fn prepared_dense_repack() -> Db<MemVfs> {
3333
// Retire the previous iteration's store here, where it is not measured.
3434
drain_parked();
3535

36-
let db = Db::open_internal(MemVfs::new(), KEK, PAGE, REALM)
36+
let db = Db::open(MemVfs::new(), KEK, PAGE, REALM, OpenOptions::default())
3737
.await
3838
.expect("open bench store");
3939
let value = [0x2A; VALUE_LEN];

benches/segment.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use fluxbench::{bench, compare, synthetic};
1818
use tokio::sync::Mutex as AsyncMutex;
1919

2020
use pagedb::vfs::memory::MemVfs;
21-
use pagedb::{Db, RealmId, SegmentKind, SegmentPageKind};
21+
use pagedb::{Db, OpenOptions, RealmId, SegmentKind, SegmentPageKind};
2222

2323
const PAGE: usize = 4096;
2424
/// Payload bytes per page (must fit inside `page_size - envelope_overhead`).
@@ -35,9 +35,15 @@ fn shared_db() -> Arc<AsyncMutex<Db<MemVfs>>> {
3535
if cell.borrow().is_none() {
3636
let db = with_rt(|rt| {
3737
rt.block_on(async {
38-
Db::open_internal(MemVfs::new(), [9u8; 32], PAGE, RealmId::new([1; 16]))
39-
.await
40-
.unwrap()
38+
Db::open(
39+
MemVfs::new(),
40+
[9u8; 32],
41+
PAGE,
42+
RealmId::new([1; 16]),
43+
OpenOptions::default(),
44+
)
45+
.await
46+
.unwrap()
4147
})
4248
});
4349
*cell.borrow_mut() = Some(Arc::new(AsyncMutex::new(db)));

benchmarks/engine-comparison/btree.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,12 @@ thread_local! {
5757
}
5858

5959
async fn open_mem(cipher: CipherId, seed: u8) -> SharedDb<MemVfs> {
60-
let db = Db::open_internal_with_options_and_cipher(
60+
let db = Db::open(
6161
MemVfs::new(),
6262
[seed; 32],
6363
PAGE,
6464
RealmId::new([seed; 16]),
65-
bench_opts(),
66-
cipher,
65+
bench_opts().with_cipher(cipher),
6766
)
6867
.await
6968
.unwrap();
@@ -73,13 +72,12 @@ async fn open_mem(cipher: CipherId, seed: u8) -> SharedDb<MemVfs> {
7372
async fn open_file(cipher: CipherId, seed: u8) -> (SharedDb<TokioVfs>, tempfile::TempDir) {
7473
let dir = tempfile::TempDir::new().unwrap();
7574
let vfs = TokioVfs::new(dir.path());
76-
let db = Db::open_internal_with_options_and_cipher(
75+
let db = Db::open(
7776
vfs,
7877
[seed; 32],
7978
PAGE,
8079
RealmId::new([seed; 16]),
81-
bench_opts(),
82-
cipher,
80+
bench_opts().with_cipher(cipher),
8381
)
8482
.await
8583
.unwrap();

benchmarks/engine-comparison/comparison.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ fn open_pagedb_fresh() -> PagedbBench {
9494
.with_anchor_budget(10_000_000);
9595
let db = with_rt(|rt| {
9696
rt.block_on(async {
97-
Db::open_internal_with_options(vfs, [0xAB; 32], 4096, RealmId::new([1; 16]), opts)
97+
Db::open(vfs, [0xAB; 32], 4096, RealmId::new([1; 16]), opts)
9898
.await
9999
.unwrap()
100100
})

src/compaction/full.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,11 @@ pub async fn compact_now<V: Vfs + Clone>(db: &Db<V>) -> Result<CompactStats> {
3535
#[allow(clippy::too_many_lines)]
3636
async fn compact_now_inner<V: Vfs + Clone>(db: &Db<V>) -> Result<CompactStats> {
3737
db.ensure_usable()?;
38-
if !matches!(db.mode, crate::txn::mode::DbMode::Standalone) {
39-
return Err(PagedbError::Unsupported);
40-
}
38+
db.require_mode(
39+
"compaction",
40+
crate::txn::mode::DbMode::Standalone,
41+
crate::txn::db::DbModeCapabilities::allows_store_maintenance,
42+
)?;
4143

4244
let mut result = CompactStats::default();
4345

src/compaction/step.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use super::types::{CompactBudget, CompactProgress};
2424
/// be safely chunked). Compaction runs atomically to completion and always
2525
/// returns `more_work = false`; to reclaim periodically, call this again later.
2626
///
27-
/// Returns `PagedbError::Unsupported` if the handle is not in `Standalone` mode.
27+
/// Returns `PagedbError::WrongMode` if the handle is not in `Standalone` mode.
2828
pub async fn compact_step<V: Vfs + Clone>(
2929
db: &Db<V>,
3030
budget: CompactBudget,

src/diag.rs

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,57 @@
1515
1616
#[cfg(all(feature = "diagnostics", not(target_arch = "wasm32")))]
1717
mod imp {
18+
use std::cell::Cell;
19+
use std::marker::PhantomData;
20+
1821
use crate::RealmId;
1922
use crate::errors::CorruptionDetail;
2023

24+
thread_local! {
25+
/// Live [`RichlyReported`] guards on this thread. A depth rather than a
26+
/// flag: nesting must not leave the suppression stuck on when the inner
27+
/// guard drops.
28+
static RICH_REPORTS_IN_FLIGHT: Cell<u32> = const { Cell::new(0) };
29+
}
30+
31+
/// Evidence that a site-specific report already covers the corruption about
32+
/// to be constructed.
33+
///
34+
/// Every rich capture in this module returns one. Bind it across the
35+
/// `PagedbError::corruption(...)` call that reports the same failure and
36+
/// [`corruption_captured`] stands down for that construction, so one
37+
/// failure files one report — the site's, which carries forensics (the
38+
/// failing page, the fsck hint, the store path) that the constructor, which
39+
/// sees only a `CorruptionDetail`, cannot know.
40+
///
41+
/// This is the mechanism, not a special case: a second site that grows its
42+
/// own `DomainContext` gets the same suppression by returning this type
43+
/// from its capture function, with nothing to rediscover.
44+
///
45+
/// Deliberately not `Send`. The guard covers the construction immediately
46+
/// following it and nothing else; holding it across an `.await` would
47+
/// silence unrelated corruption raised by whatever ran in between, and the
48+
/// compiler rejects that wherever the surrounding future must be `Send`.
49+
#[must_use = "bind the guard so it outlives the corruption() call it covers"]
50+
pub struct RichlyReported {
51+
_not_send: PhantomData<*const ()>,
52+
}
53+
54+
impl RichlyReported {
55+
fn begin() -> Self {
56+
RICH_REPORTS_IN_FLIGHT.with(|depth| depth.set(depth.get().saturating_add(1)));
57+
Self {
58+
_not_send: PhantomData,
59+
}
60+
}
61+
}
62+
63+
impl Drop for RichlyReported {
64+
fn drop(&mut self) {
65+
RICH_REPORTS_IN_FLIGHT.with(|depth| depth.set(depth.get().saturating_sub(1)));
66+
}
67+
}
68+
2169
/// Breadcrumb: a store was (re)opened — marks epoch boundaries in the trail,
2270
/// the dimension along which freed-page use-after-free surfaces.
2371
pub fn reopened(latest_commit: u64) {
@@ -98,13 +146,18 @@ mod imp {
98146
/// authenticate. Records the failing page, expected binding, and realm so
99147
/// the failure is diagnosable from the report alone; the host application
100148
/// preserves the store bytes (it owns the real path behind the VFS).
149+
///
150+
/// Returns a [`RichlyReported`] guard: bind it across the
151+
/// `PagedbError::corruption(...)` that turns this same failure into an
152+
/// error, so the caller gets the precise variant while this report — not
153+
/// the constructor's generic one — is what gets filed.
101154
pub fn page_read_verify_failed(
102155
main_db_path: &str,
103156
page_id: u64,
104157
file: &str,
105158
binding: &str,
106159
realm: &RealmId,
107-
) {
160+
) -> RichlyReported {
108161
let ctx = PageReadVerifyFailure {
109162
page_id,
110163
file: file.to_owned(),
@@ -119,6 +172,7 @@ mod imp {
119172
.domain(&ctx)
120173
.with_backtrace()
121174
.emit();
175+
RichlyReported::begin()
122176
}
123177

124178
/// Forensic context for a [`CorruptionDetail`] captured at the moment
@@ -268,6 +322,12 @@ mod imp {
268322
/// reporting layer instead of only the one AEAD failure site this module
269323
/// originally covered.
270324
pub fn corruption_captured(detail: &CorruptionDetail) {
325+
// A site-specific capture is already in flight for this exact failure
326+
// and carries strictly more than this one could. Filing both would mean
327+
// two reports for one event, the second of them less useful.
328+
if RICH_REPORTS_IN_FLIGHT.with(Cell::get) > 0 {
329+
return;
330+
}
271331
let (kind, grouping_key) = classify(detail);
272332
let ctx = CorruptionConstructed {
273333
kind,
@@ -288,6 +348,10 @@ mod imp {
288348
mod imp {
289349
use crate::RealmId;
290350

351+
/// No-op counterpart of the recording build's suppression guard, so call
352+
/// sites bind the same value under every cfg.
353+
pub struct RichlyReported;
354+
291355
pub fn reopened(_latest_commit: u64) {}
292356
pub fn committed(_commit_id: u64, _freed_pages: usize) {}
293357
pub fn lock_acquired(_mode: &str, _path: &str) {}
@@ -299,12 +363,16 @@ mod imp {
299363
_file: &str,
300364
_binding: &str,
301365
_realm: &RealmId,
302-
) {
366+
) -> RichlyReported {
367+
RichlyReported
303368
}
304369

305370
pub fn corruption_captured(_detail: &crate::errors::CorruptionDetail) {}
306371
}
307372

373+
// `RichlyReported` is deliberately not re-exported: a call site binds it as the
374+
// return value of a rich capture (`let _report = diag::page_read_verify_failed(…)`)
375+
// and never names the type, so exporting it would only be an unused path.
308376
pub use imp::{
309377
committed, corruption_captured, flushed, lock_acquired, lock_rejected, page_read_verify_failed,
310378
reopened,

0 commit comments

Comments
 (0)