Skip to content

Commit 58fee9f

Browse files
committed
Merge remote-tracking branch 'origin/main'
# Conflicts: # Cargo.toml # src/txn/db/mod.rs
2 parents 73720d9 + da042c3 commit 58fee9f

10 files changed

Lines changed: 992 additions & 45 deletions

File tree

.github/workflows/test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,4 +266,4 @@ jobs:
266266
with:
267267
tool: cargo-deny
268268
- name: Run cargo-deny
269-
run: cargo deny check --exclude-dev
269+
run: cargo deny --exclude-dev check

Cargo.lock

Lines changed: 20 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/btree/tree/core.rs

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,14 @@ pub struct BTree<V: Vfs> {
2323
pub(super) freed: Vec<u64>,
2424
pub(super) page_size: usize,
2525
/// Minimum `page_id` that may be recycled from `freed` within this session.
26-
/// Pages below this threshold were live in prior snapshots and may still be
27-
/// accessed by pinned readers; they must not be overwritten until the
28-
/// deferred-free queue allows it. Set to `next_page_id` when any reader is
29-
/// tracked; set to 0 when no readers are pinned (safe to recycle freely).
26+
/// Pages below this threshold existed before the session began: they are
27+
/// still referenced by the last durable header (a copy-on-write free does
28+
/// not unreference them on disk until the *next* header lands) and may
29+
/// also be pinned by reader snapshots. They must not be overwritten
30+
/// in-session; they re-enter circulation through the deferred-free queue
31+
/// once the free-list naming them is durable. Callers set this to the
32+
/// session-start `next_page_id`, so only pages bump-allocated within the
33+
/// session are recyclable immediately.
3034
pub(super) reuse_threshold: u64,
3135
/// Leaves modified during this write session but not yet promoted via
3236
/// `CoW` to a fresh page. Keyed by the leaf's current `page_id` as referenced
@@ -108,8 +112,9 @@ impl<V: Vfs> BTree<V> {
108112

109113
/// Set the reuse threshold. Any freed page with `page_id < threshold` will
110114
/// not be recycled within this session; it goes to `self.freed` for later
111-
/// deferred-queue promotion. Call with `next_page_id` when tracked readers
112-
/// are present; call with `0` when no readers are pinned.
115+
/// deferred-queue promotion. Call with the session-start `next_page_id`:
116+
/// pages below it are still referenced by the last durable header and must
117+
/// keep their on-disk bytes until the header that frees them is durable.
113118
pub fn set_reuse_threshold(&mut self, threshold: u64) {
114119
self.reuse_threshold = threshold;
115120
}
@@ -153,16 +158,28 @@ impl<V: Vfs> BTree<V> {
153158
// the reuse threshold: a page below it may still be live in a pinned
154159
// reader's snapshot, so it can't be reused until the durable free-list
155160
// clears it (it leaves via `drain_freed` at commit instead).
161+
//
162+
// Exception: a below-threshold page originally drawn from the shared
163+
// cache this session (recorded in `free_page_consumed`) is free per
164+
// the last durable header regardless of what this session wrote to it
165+
// since, so recycling it again in-session is crash-safe — a failed or
166+
// torn commit leaves the durable header referencing none of its
167+
// content. Without this, every cache-drawn page freed by a later
168+
// in-session split is burned for the rest of the txn and allocation
169+
// falls through to bump growth.
156170
if self.reuse_threshold == 0 {
157171
if let Some(id) = self.freed.pop() {
158172
return id;
159173
}
160-
} else if let Some(pos) = self
161-
.freed
162-
.iter()
163-
.rposition(|&id| id >= self.reuse_threshold)
164-
{
165-
return self.freed.remove(pos);
174+
} else {
175+
let consumed = self.free_page_consumed.as_ref().map(|c| c.lock());
176+
let pos = self.freed.iter().rposition(|&id| {
177+
id >= self.reuse_threshold || consumed.as_ref().is_some_and(|c| c.contains(&id))
178+
});
179+
drop(consumed);
180+
if let Some(pos) = pos {
181+
return self.freed.remove(pos);
182+
}
166183
}
167184
// Then draw from the shared cross-commit cache. It is loaded at txn
168185
// begin with *only* free-list pages below the reclamation floor — pages

src/btree/tree/navigate.rs

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ impl<V: Vfs> BTree<V> {
5050
};
5151
self.write_internal(new_root_page, &internal).await?;
5252
self.root_page_id = new_root_page;
53+
// The tree grew a level: every cached dirty-leaf parent path
54+
// still starts at the old root (or its split halves). Prepend
55+
// the new root so flush's spine walk reaches it — otherwise
56+
// the new root's child pointers are never rewritten when those
57+
// halves are CoW'd at flush, leaving the durable root pointing
58+
// at freed pages.
59+
for path in self.dirty_parent_paths.values_mut() {
60+
path.insert(0, new_root_page);
61+
}
5362
return Ok(());
5463
}
5564

@@ -77,12 +86,18 @@ impl<V: Vfs> BTree<V> {
7786
self.free_page(internal_page);
7887
// An internal page that other dirty leaves' parent paths
7988
// reference has been replaced and freed; remap them. The
80-
// split case promotes the old internal: keys ≤ promoted live
81-
// on `new_internal_page`, the rest on `right_internal_page`.
82-
// For path-remapping (used only to find ancestors to CoW)
83-
// either replacement is acceptable, so substitute with the
84-
// left replacement.
85-
self.remap_dirty_parent_paths(internal_page, new_internal_page);
89+
// split distributes the old internal's children across two
90+
// pages, so each path must be remapped to the half that
91+
// actually holds its next hop — substituting the wrong half
92+
// means flush's spine walk never visits the true parent, its
93+
// stale child pointer survives, and the durable tree ends up
94+
// referencing a freed leaf page.
95+
self.remap_dirty_parent_paths_split(
96+
internal_page,
97+
new_internal_page,
98+
right_internal_page,
99+
&right,
100+
);
86101
child_old = internal_page;
87102
left_replacement = new_internal_page;
88103
right_to_insert = right_internal_page;
@@ -122,8 +137,44 @@ impl<V: Vfs> BTree<V> {
122137
}
123138
}
124139

140+
/// Substitute every occurrence of `old` in cached dirty-leaf parent paths
141+
/// with whichever split half actually contains the path's next hop as a
142+
/// child. Used after [`propagate_split_up`](Self::propagate_split_up)
143+
/// splits an internal page that other dirty leaves' paths reference: the
144+
/// old page's children are distributed across `left_new` and `right_new`,
145+
/// and flush's spine walk only rewrites child pointers in internals the
146+
/// paths name — so each path must name the half its leaf descends from.
147+
/// The next hop is the path element after `old`, or the dirty leaf's own
148+
/// `page_id` when `old` is the path's last element.
149+
fn remap_dirty_parent_paths_split(
150+
&mut self,
151+
old: u64,
152+
left_new: u64,
153+
right_new: u64,
154+
right: &Internal,
155+
) {
156+
if self.dirty_parent_paths.is_empty() {
157+
return;
158+
}
159+
let right_children: std::collections::HashSet<u64> = std::iter::once(right.leftmost_child)
160+
.chain(right.entries.iter().map(|e| e.right_child))
161+
.collect();
162+
for (leaf_id, path) in &mut self.dirty_parent_paths {
163+
for i in 0..path.len() {
164+
if path[i] == old {
165+
let next_hop = path.get(i + 1).copied().unwrap_or(*leaf_id);
166+
path[i] = if right_children.contains(&next_hop) {
167+
right_new
168+
} else {
169+
left_new
170+
};
171+
}
172+
}
173+
}
174+
}
175+
125176
/// Substitute every occurrence of `old` with `new` in cached dirty-leaf
126-
/// parent paths. Used after a split or a spine `CoW` in
177+
/// parent paths. Used after a spine `CoW` in
127178
/// [`propagate_split_up`](Self::propagate_split_up) replaces an internal
128179
/// page that other dirty leaves' paths reference.
129180
fn remap_dirty_parent_paths(&mut self, old: u64, new: u64) {

src/pager/core.rs

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,7 @@ impl<V: Vfs> Pager<V> {
604604
/// AAD is constructed from on-disk header bytes, not from `Pager.active_epoch`
605605
/// or the configured cipher. This makes mixed-epoch and mixed-cipher coexistence
606606
/// work correctly without global invariants.
607+
#[allow(clippy::too_many_lines)]
607608
async fn read_page(
608609
&self,
609610
file: FileKey,
@@ -619,6 +620,14 @@ impl<V: Vfs> Pager<V> {
619620
if page.realm_id_bytes != Some(realm_id.0) {
620621
return Err(PagedbError::ChecksumFailure);
621622
}
623+
// Enforce the same kind binding a cold read's AAD enforces.
624+
// Without this, a stale pointer reading a recycled page under
625+
// the wrong kind succeeds while the page is warm and only
626+
// starts failing after eviction — hiding structural damage
627+
// until long after the write that caused it.
628+
if page.kind_byte != 0 && page.kind_byte != expected_kind.as_byte() {
629+
return Err(PagedbError::ChecksumFailure);
630+
}
622631
self.inner.record_hit(file);
623632
cache.pin((file, page_id));
624633
return Ok(PageGuard {
@@ -720,9 +729,17 @@ impl<V: Vfs> Pager<V> {
720729
Err(e) => return Err(e),
721730
}
722731
}
732+
tracing::error!(
733+
page_id,
734+
?file,
735+
expected_kind = ?expected_kind,
736+
realm = ?realm_id.0,
737+
"page AEAD/MAC verification failed on read"
738+
);
723739
Err(last_err.unwrap_or(PagedbError::ChecksumFailure))
724740
}
725741

742+
#[allow(clippy::too_many_lines)]
726743
async fn flush_file(
727744
&self,
728745
file: FileKey,
@@ -744,8 +761,11 @@ impl<V: Vfs> Pager<V> {
744761
let flush_epoch = self.active_epoch.load(AtomOrd::SeqCst);
745762

746763
// Serial gather: snapshot each dirty page's plaintext + kind under the
747-
// cache lock. Cheap memcpy; no AEAD work happens here.
764+
// cache lock. Cheap memcpy; no AEAD work happens here. The gathered
765+
// `Arc<Page>` is retained per pid so the dirty-clear below can detect
766+
// pages replaced by a concurrent writer during the (slow) seal+write.
748767
let mut prepared: Vec<(u64, PageKind, Vec<u8>)> = Vec::with_capacity(dirty_ids.len());
768+
let mut gathered: Vec<(u64, Arc<Page>)> = Vec::with_capacity(dirty_ids.len());
749769
for pid in &dirty_ids {
750770
let page = self
751771
.inner
@@ -755,6 +775,7 @@ impl<V: Vfs> Pager<V> {
755775
.ok_or_else(|| {
756776
PagedbError::Io(std::io::Error::other("dirty page missing from cache"))
757777
})?;
778+
gathered.push((*pid, page.clone()));
758779
let kind = if page.kind_byte != 0 {
759780
PageKind::from_byte(page.kind_byte)?
760781
} else {
@@ -765,6 +786,7 @@ impl<V: Vfs> Pager<V> {
765786
let mut wire = vec![0u8; page_size];
766787
let plain = body(&page.bytes);
767788
wire[HEADER_LEN..HEADER_LEN + plain.len()].copy_from_slice(plain);
789+
tracing::trace!(page_id = *pid, ?kind, ?file, "flush: writing page");
768790
prepared.push((*pid, kind, wire));
769791
}
770792

@@ -843,10 +865,25 @@ impl<V: Vfs> Pager<V> {
843865
f.sync().await?;
844866
}
845867

846-
// Clear dirty flags.
868+
// Clear dirty flags — but ONLY for pages still holding the exact
869+
// `Arc<Page>` we gathered. A concurrent writer replaces the Arc on
870+
// every write; unconditionally clearing here would wipe its dirty
871+
// flag while its content never reached disk (lost update → stale
872+
// page on next cold read → AEAD/kind mismatch).
873+
// A replaced page keeps its flag and flushes on the next cycle.
847874
let mut cache = self.inner.cache_for_key(file).lock();
848-
for pid in dirty_ids {
849-
cache.clear_dirty((file, pid));
875+
for (pid, snapshot) in gathered {
876+
match cache.get((file, pid)) {
877+
Some(current) if Arc::ptr_eq(&current, &snapshot) => {
878+
cache.clear_dirty((file, pid));
879+
}
880+
_ => {
881+
tracing::debug!(
882+
page_id = pid,
883+
"page re-dirtied during flush; keeping dirty for next cycle"
884+
);
885+
}
886+
}
850887
}
851888
Ok(())
852889
}

0 commit comments

Comments
 (0)