Skip to content

Commit

Permalink
feat: add a prev_root check when committing
Browse files Browse the repository at this point in the history
  • Loading branch information
@rphmeier
rphmeier committed Jan 29, 2025
1 parent 0d92ceb commit 4b2faee
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 38 deletions.
29 changes: 24 additions & 5 deletions nomt/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,16 +340,18 @@ impl<T: HashAlgorithm> Nomt<T> {
None
};

let prev_root = live_overlay
.parent_root()
.unwrap_or_else(|| self.root().into_inner());

Session {
store,
merkle_updater: self.merkle_update_pool.begin::<T>(
self.page_cache.clone(),
self.page_pool.clone(),
self.store.clone(),
live_overlay.clone(),
live_overlay
.parent_root()
.unwrap_or_else(|| self.root().into_inner()),
prev_root,
),
metrics: self.metrics.clone(),
rollback_delta,
Expand All @@ -358,6 +360,7 @@ impl<T: HashAlgorithm> Nomt<T> {
access_guard: params
.take_global_guard
.then(|| RwLock::read_arc(&self.access_lock)),
prev_root: Root(prev_root),
_marker: std::marker::PhantomData,
}
}
Expand Down Expand Up @@ -500,6 +503,7 @@ pub struct Session<T: HashAlgorithm> {
overlay: LiveOverlay,
witness_mode: WitnessMode,
access_guard: Option<ArcRwLockReadGuard<parking_lot::RawRwLock, ()>>,
prev_root: Root,
_marker: std::marker::PhantomData<T>,
}

Expand Down Expand Up @@ -601,6 +605,7 @@ impl<T: HashAlgorithm> Session<T> {
merkle_output,
rollback_delta,
parent_overlay: self.overlay,
prev_root: self.prev_root,
take_global_guard: self.access_guard.is_some(),
}
}
Expand All @@ -618,6 +623,7 @@ pub struct FinishedSession {
merkle_output: merkle::Output,
rollback_delta: Option<rollback::Delta>,
parent_overlay: LiveOverlay,
prev_root: Root,
// INTERNAL: whether to take a write guard while committing. always true except during rollback.
take_global_guard: bool,
}
Expand Down Expand Up @@ -647,6 +653,7 @@ impl FinishedSession {
let values = self.value_transaction.into_iter().collect();

self.parent_overlay.finish(
self.prev_root.into_inner(),
self.merkle_output.root,
updated_pages,
values,
Expand All @@ -662,12 +669,17 @@ impl FinishedSession {
/// The changeset may be invalidated if another competing session, overlay, or rollback was
/// committed.
pub fn commit<T: HashAlgorithm>(self, nomt: &Nomt<T>) -> Result<(), anyhow::Error> {
// TODO: do a prev_root check or something to ensure continuity?

let _write_guard = self.take_global_guard.then(|| nomt.access_lock.write());

{
let mut shared = nomt.shared.lock();
if shared.root != self.prev_root {
anyhow::bail!(
"Changeset no longer valid (expected previous root {:?}, got {:?})",
self.prev_root,
shared.root
);
}
shared.root = Root(self.merkle_output.root);
shared.last_commit_marker = None;
}
Expand Down Expand Up @@ -720,6 +732,13 @@ impl Overlay {

{
let mut shared = nomt.shared.lock();
if shared.root != self.prev_root() {
anyhow::bail!(
"Changeset no longer valid (expected previous root {:?}, got {:?})",
self.prev_root(),
shared.root
);
}
shared.root = root;
shared.last_commit_marker = Some(marker);
}
Expand Down
104 changes: 71 additions & 33 deletions nomt/src/overlay.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ impl Overlay {
Root(self.inner.root)
}

/// Get the previous root of this overlay.
pub fn prev_root(&self) -> Root {
Root(self.inner.prev_root)
}

/// Check whether the parent of this overlay matches the provided marker.
/// If the provided marker is `None`, then this checks that this overlay doesn't have a parent.
pub(super) fn parent_matches_marker(&self, marker: Option<&OverlayMarker>) -> bool {
Expand Down Expand Up @@ -74,6 +79,7 @@ impl Overlay {
}

struct OverlayInner {
prev_root: Node,
root: Node,
index: Index,
data: Arc<Data>,
Expand Down Expand Up @@ -362,6 +368,7 @@ impl LiveOverlay {
/// Finish this overlay and transform it into a frozen [`Overlay`].
pub(super) fn finish(
self,
prev_root: Node,
root: Node,
page_changes: HashMap<PageId, DirtyPage>,
value_changes: HashMap<KeyPath, ValueChange>,
Expand Down Expand Up @@ -392,6 +399,7 @@ impl LiveOverlay {
Overlay {
inner: Arc::new(OverlayInner {
index,
prev_root,
root,
data: Arc::new(Data {
pages: page_changes,
Expand Down Expand Up @@ -448,18 +456,25 @@ mod tests {

#[test]
fn not_ancestor() {
let a =
LiveOverlay::new(None)
.unwrap()
.finish([1; 32], HashMap::new(), HashMap::new(), None);
let a1 =
LiveOverlay::new(None)
.unwrap()
.finish([2; 32], HashMap::new(), HashMap::new(), None);
let a = LiveOverlay::new(None).unwrap().finish(
[0; 32],
[1; 32],
HashMap::new(),
HashMap::new(),
None,
);
let a1 = LiveOverlay::new(None).unwrap().finish(
[1; 32],
[2; 32],
HashMap::new(),
HashMap::new(),
None,
);

let mut ancestors = VecDeque::new();
ancestors.push_front(a);
let b = LiveOverlay::new(&ancestors).unwrap().finish(
[2; 32],
[3; 32],
HashMap::new(),
HashMap::new(),
Expand All @@ -476,21 +491,26 @@ mod tests {

#[test]
fn incomplete_ancestors() {
let a =
LiveOverlay::new(None)
.unwrap()
.finish([1; 32], HashMap::new(), HashMap::new(), None);
let a = LiveOverlay::new(None).unwrap().finish(
[0; 32],
[1; 32],
HashMap::new(),
HashMap::new(),
None,
);

let mut ancestors = VecDeque::new();
ancestors.push_front(a);
let b = LiveOverlay::new(&ancestors).unwrap().finish(
[1; 32],
[2; 32],
HashMap::new(),
HashMap::new(),
None,
);
ancestors.push_front(b);
let c = LiveOverlay::new(&ancestors).unwrap().finish(
[2; 32],
[3; 32],
HashMap::new(),
HashMap::new(),
Expand Down Expand Up @@ -520,14 +540,18 @@ mod tests {

#[test]
fn drop_propagation() {
let a =
LiveOverlay::new(None)
.unwrap()
.finish([1; 32], HashMap::new(), HashMap::new(), None);
let a = LiveOverlay::new(None).unwrap().finish(
[0; 32],
[1; 32],
HashMap::new(),
HashMap::new(),
None,
);

let mut ancestors = VecDeque::new();
ancestors.push_front(a);
let b = LiveOverlay::new(&ancestors).unwrap().finish(
[1; 32],
[2; 32],
HashMap::new(),
HashMap::new(),
Expand All @@ -541,14 +565,18 @@ mod tests {

#[test]
fn commit_overrides_drop_propagation() {
let a =
LiveOverlay::new(None)
.unwrap()
.finish([1; 32], HashMap::new(), HashMap::new(), None);
let a = LiveOverlay::new(None).unwrap().finish(
[0; 32],
[1; 32],
HashMap::new(),
HashMap::new(),
None,
);

let mut ancestors = VecDeque::new();
ancestors.push_front(a);
let b = LiveOverlay::new(&ancestors).unwrap().finish(
[1; 32],
[2; 32],
HashMap::new(),
HashMap::new(),
Expand All @@ -563,21 +591,26 @@ mod tests {

#[test]
fn committed_ancestors_considered_complete() {
let a =
LiveOverlay::new(None)
.unwrap()
.finish([1; 32], HashMap::new(), HashMap::new(), None);
let a = LiveOverlay::new(None).unwrap().finish(
[0; 32],
[1; 32],
HashMap::new(),
HashMap::new(),
None,
);

let mut ancestors = VecDeque::new();
ancestors.push_front(a);
let b = LiveOverlay::new(&ancestors).unwrap().finish(
[1; 32],
[2; 32],
HashMap::new(),
HashMap::new(),
None,
);
ancestors.push_front(b);
let c = LiveOverlay::new(&ancestors).unwrap().finish(
[2; 32],
[3; 32],
HashMap::new(),
HashMap::new(),
Expand Down Expand Up @@ -615,13 +648,13 @@ mod tests {
let value_map = vec![(key1, value1a)].into_iter().collect();
let a = LiveOverlay::new(None)
.unwrap()
.finish([1; 32], page_map, value_map, None);
.finish([0; 32], [1; 32], page_map, value_map, None);

let page_map = vec![(ROOT_PAGE_ID, page1b)].into_iter().collect();
let value_map = vec![(key1, value1b)].into_iter().collect();
let b = LiveOverlay::new(Some(&a))
.unwrap()
.finish([2; 32], page_map, value_map, None);
.finish([1; 32], [2; 32], page_map, value_map, None);

let c = LiveOverlay::new([&b, &a]).unwrap();

Expand Down Expand Up @@ -681,7 +714,7 @@ mod tests {
let value_map = [(key, value)].into_iter().collect();
let overlay = LiveOverlay::new(&ancestors)
.unwrap()
.finish([1; 32], page_map, value_map, None);
.finish([0; 32], [1; 32], page_map, value_map, None);
ancestors.push_front(overlay);
}

Expand Down Expand Up @@ -721,12 +754,14 @@ mod tests {
);

let a = LiveOverlay::new(None).unwrap().finish(
[0; 32],
[1; 32],
vec![(ROOT_PAGE_ID, page)].into_iter().collect(),
HashMap::new(),
None,
);
let b = LiveOverlay::new([&a]).unwrap().finish(
[1; 32],
[2; 32],
vec![(ROOT_PAGE_ID, page2)].into_iter().collect(),
HashMap::new(),
Expand Down Expand Up @@ -787,20 +822,23 @@ mod tests {
.collect();
let a = LiveOverlay::new(None)
.unwrap()
.finish([1; 32], page_map, value_map, None);
.finish([0; 32], [1; 32], page_map, value_map, None);

let page_map = vec![(page_id_2.clone(), page_2b)].into_iter().collect();
let value_map = vec![(key_2, val_2b.clone())].into_iter().collect();
let b = LiveOverlay::new([&a])
.unwrap()
.finish([1; 32], page_map, value_map, None);
.finish([0; 32], [1; 32], page_map, value_map, None);

a.mark_committed();

let c =
LiveOverlay::new([&b])
.unwrap()
.finish([1; 32], HashMap::new(), HashMap::new(), None);
let c = LiveOverlay::new([&b]).unwrap().finish(
[0; 32],
[1; 32],
HashMap::new(),
HashMap::new(),
None,
);

// ensure everything from seqn 0 has been pruned.
assert_eq!(c.inner.index.pages_by_seqn[0].0, 1);
Expand Down
Loading

0 comments on commit 4b2faee

Please sign in to comment.