Skip to content

Commit

Permalink
fix: Concurrent write and remove should reconcile correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
matheus23 committed Apr 25, 2024
1 parent cd112ac commit 0d17d90
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 36 deletions.
4 changes: 2 additions & 2 deletions wnfs/src/private/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1453,13 +1453,13 @@ impl PrivateDirectory {
}
(PrivateNode::File(_), PrivateNode::Dir(_)) => {
// a directory wins over a file
*our_link = other_link.clone();
our_link.clone_from(other_link);
}
// file vs. file and dir vs. dir cases
_ => {
// We tie-break as usual
if ord == Ordering::Greater {
*our_link = other_link.clone();
our_link.clone_from(other_link);
}
}
}
Expand Down
62 changes: 28 additions & 34 deletions wnfs/src/public/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -884,38 +884,17 @@ impl PublicDirectory {
Reconciliation::FastForward
}
None => {
let file_tie_breaks = self.merge(other, store).await?;
let mut file_tie_breaks = BTreeSet::new();
self.reconcile_helper(other, store, &[], &mut file_tie_breaks)
.await?;
Reconciliation::Merged { file_tie_breaks }
}
})
}

/// Merge this node with given other node, ignoring whether the
/// other node is actually ahead in history or not.
///
/// Prefer using `reconcile`, if you don't know what the difference is!
///
/// Returns the set of file paths where tie breaks were used to resolve
/// conflicts. This means that for each path there exists a file that has been
/// overwritten with another version.
///
/// It's possible to walk the history backwards to find which version of each
/// file has been overwritten & merge the two file versions of each file together
/// in an application-specific way and create another history entry.
pub async fn merge<'a>(
self: &'a mut Arc<Self>,
other: &'a Arc<Self>,
store: &'a impl BlockStore,
) -> Result<BTreeSet<Vec<String>>> {
let mut file_tie_breaks = BTreeSet::new();
self.merge_helper(other, store, &[], &mut file_tie_breaks)
.await?;
Ok(file_tie_breaks)
}

#[cfg_attr(not(target_arch = "wasm32"), async_recursion)]
#[cfg_attr(target_arch = "wasm32", async_recursion(?Send))]
async fn merge_helper<'a>(
async fn reconcile_helper<'a>(
self: &'a mut Arc<Self>,
other: &'a Arc<Self>,
store: &'a impl BlockStore,
Expand Down Expand Up @@ -947,6 +926,21 @@ impl PublicDirectory {
}
Entry::Occupied(mut occupied) => {
let our_node = occupied.get_mut().resolve_value_mut(store).await?;

match our_node.causal_compare(other_node, store).await? {
Some(Ordering::Equal) => {
continue;
}
Some(Ordering::Less) => {
our_node.clone_from(other_node);
continue;
}
Some(Ordering::Greater) => {
continue;
}
None => {}
};

match (our_node, other_node) {
(PublicNode::File(our_file), PublicNode::File(other_file)) => {
if our_file.merge(other_file, store).await? {
Expand All @@ -966,7 +960,7 @@ impl PublicDirectory {
(PublicNode::Dir(dir), PublicNode::Dir(other_dir)) => {
let mut path = current_path.to_vec();
path.push(name.clone());
dir.merge_helper(other_dir, store, &path, file_tie_breaks)
dir.reconcile_helper(other_dir, store, &path, file_tie_breaks)
.await?;
}
}
Expand Down Expand Up @@ -1626,7 +1620,7 @@ mod proptests {

root1.mkdir(&path, time, store).await.unwrap();

root0.merge(root1, store).await.unwrap();
root0.reconcile(root1, store).await.unwrap();

let node = root0
.get_node(&path, store)
Expand All @@ -1653,9 +1647,9 @@ mod proptests {
let root1 = convert_fs(fs1, time, store).await.unwrap();

let mut merge_one_way = Arc::clone(&root0);
merge_one_way.merge(&root1, store).await.unwrap();
merge_one_way.reconcile(&root1, store).await.unwrap();
let mut merge_other_way = Arc::clone(&root1);
merge_other_way.merge(&root0, store).await.unwrap();
merge_other_way.reconcile(&root0, store).await.unwrap();

let cid_one_way = merge_one_way.store(store).await.unwrap();
let cid_other_way = merge_other_way.store(store).await.unwrap();
Expand All @@ -1680,13 +1674,13 @@ mod proptests {
let root2 = convert_fs(fs2, time, store).await.unwrap();

let mut merge_0_1_then_2 = Arc::clone(&root0);
merge_0_1_then_2.merge(&root1, store).await.unwrap();
merge_0_1_then_2.merge(&root2, store).await.unwrap();
merge_0_1_then_2.reconcile(&root1, store).await.unwrap();
merge_0_1_then_2.reconcile(&root2, store).await.unwrap();

let mut merge_1_2 = Arc::clone(&root1);
merge_1_2.merge(&root2, store).await.unwrap();
merge_1_2.reconcile(&root2, store).await.unwrap();
let mut merge_0_with_1_2 = Arc::clone(&root0);
merge_0_with_1_2.merge(&merge_1_2, store).await.unwrap();
merge_0_with_1_2.reconcile(&merge_1_2, store).await.unwrap();

let cid_one_way = merge_0_1_then_2.store(store).await.unwrap();
let cid_other_way = merge_0_with_1_2.store(store).await.unwrap();
Expand All @@ -1712,7 +1706,7 @@ mod proptests {
let mut root = convert_fs(fs0, time, store).await.unwrap();
let root1 = convert_fs(fs1, time, store).await.unwrap();

root.merge(&root1, store).await.unwrap();
root.reconcile(&root1, store).await.unwrap();

for dir in all_dirs {
let exists = root.get_node(&dir, store).await.unwrap().is_some();
Expand Down

0 comments on commit 0d17d90

Please sign in to comment.