Skip to content

Commit

Permalink
Fix tree cache checkout issue and add a few invariance checks (#479)
Browse files Browse the repository at this point in the history
* test: test failed if use non-cached diff calc

* fix: tree diff calc retreat cache current vv

* feat(wasm): commit message & get pending ops length

* bk

* chore: add tree one doc fuzz

* fix: fix the problem and added a few checks

* chore: rm debugging code

* fix: encode snapshot when detached

---------

Co-authored-by: leeeon233 <[email protected]>
  • Loading branch information
zxch3n and Leeeon233 authored Sep 25, 2024
1 parent 261cdda commit 78aaa40
Show file tree
Hide file tree
Showing 15 changed files with 263 additions and 46 deletions.
9 changes: 5 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions crates/fuzz/fuzz/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

136 changes: 134 additions & 2 deletions crates/fuzz/src/one_doc_fuzzer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use loro::{ContainerType, Frontiers, LoroDoc};
use loro::{ContainerType, Frontiers, LoroDoc, LoroError, TreeID};
use tabled::TableIteratorExt;
use tracing::{info, info_span};

Expand Down Expand Up @@ -56,6 +56,7 @@ impl OneDocFuzzer {
ContainerType::List,
ContainerType::Map,
ContainerType::MovableList,
ContainerType::Tree,
];
*target %= valid_targets.len() as u8;
action.convert_to_inner(&valid_targets[*target as usize]);
Expand Down Expand Up @@ -157,7 +158,81 @@ impl OneDocFuzzer {
crate::container::TextActionInner::Mark(_) => {}
}
}
_ => {}
crate::actions::ActionInner::Tree(tree_action) => {
let tree = self.doc.get_tree("tree");
tree.enable_fractional_index(0);
let nodes = tree
.nodes()
.into_iter()
.filter(|x| !tree.is_node_deleted(*x).unwrap())
.collect::<Vec<_>>();
let node_num = nodes.len();
let crate::container::TreeAction { target, action } = tree_action;
if node_num == 0
|| node_num < 2
&& (matches!(
action,
crate::container::TreeActionInner::Move { .. }
| crate::container::TreeActionInner::Meta { .. }
))
{
*action = crate::container::TreeActionInner::Create { index: 0 };
}
match action {
crate::container::TreeActionInner::Create { index } => {
let id = tree.__internal__next_tree_id();
let len = tree.children_num(None).unwrap_or(0);
*index %= len + 1;
*target = (id.peer, id.counter);
}
crate::container::TreeActionInner::Delete => {
let target_index = target.0 as usize % node_num;
*target =
(nodes[target_index].peer, nodes[target_index].counter);
}
crate::container::TreeActionInner::Move { parent, index } => {
let target_index = target.0 as usize % node_num;
*target =
(nodes[target_index].peer, nodes[target_index].counter);
let mut parent_idx = parent.0 as usize % node_num;
while target_index == parent_idx {
parent_idx = (parent_idx + 1) % node_num;
}
*parent = (nodes[parent_idx].peer, nodes[parent_idx].counter);
*index %= tree
.children_num(TreeID::new(parent.0, parent.1))
.unwrap_or(0)
+ 1;
}
crate::container::TreeActionInner::MoveBefore {
target,
before: (p, c),
}
| crate::container::TreeActionInner::MoveAfter {
target,
after: (p, c),
} => {
let target_index = target.0 as usize % node_num;
*target =
(nodes[target_index].peer, nodes[target_index].counter);
let mut other_idx = *p as usize % node_num;
while target_index == other_idx {
other_idx = (other_idx + 1) % node_num;
}
*p = nodes[other_idx].peer;
*c = nodes[other_idx].counter;
}
crate::container::TreeActionInner::Meta { meta: (_, v) } => {
let target_index = target.0 as usize % node_num;
*target =
(nodes[target_index].peer, nodes[target_index].counter);
if matches!(v, FuzzValue::Container(_)) {
*v = FuzzValue::I32(0);
}
}
}
}
_ => unimplemented!(),
}
}
}
Expand Down Expand Up @@ -233,6 +308,63 @@ impl OneDocFuzzer {
crate::container::TextActionInner::Mark(_) => {}
}
}
crate::actions::ActionInner::Tree(tree_action) => {
let tree = self.doc.get_tree("tree");
let crate::container::TreeAction { target, action } = tree_action;
let target = TreeID {
peer: target.0,
counter: target.1,
};
match action {
crate::container::TreeActionInner::Create { index } => {
tree.create_at(None, *index).unwrap();
}
crate::container::TreeActionInner::Delete => {
tree.delete(target).unwrap();
}
crate::container::TreeActionInner::Move { parent, index } => {
let parent = TreeID {
peer: parent.0,
counter: parent.1,
};
if let Err(LoroError::TreeError(e)) =
tree.mov_to(target, Some(parent), *index)
{
// cycle move
tracing::warn!("move error {}", e);
}
}
crate::container::TreeActionInner::MoveBefore {
target,
before,
} => {
let target = TreeID {
peer: target.0,
counter: target.1,
};
let before = TreeID {
peer: before.0,
counter: before.1,
};
tree.mov_before(target, before).unwrap();
}
crate::container::TreeActionInner::MoveAfter { target, after } => {
let target = TreeID {
peer: target.0,
counter: target.1,
};
let after = TreeID {
peer: after.0,
counter: after.1,
};
tree.mov_after(target, after).unwrap();
}
crate::container::TreeActionInner::Meta { meta: (k, v) } => {
let meta = tree.get_meta(target).unwrap();
meta.insert(k, v.to_string()).unwrap();
}
}
}
_ => unimplemented!(),
},
_ => unreachable!(),
Expand Down
1 change: 1 addition & 0 deletions crates/loro-internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ arref = "0.1.0"
tracing = { version = "0.1" }
nonmax = "0.5.5"
ensure-cov = { workspace = true }
pretty_assertions = "1.4.1"


[dev-dependencies]
Expand Down
19 changes: 11 additions & 8 deletions crates/loro-internal/src/diff_calc/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use fractional_index::FractionalIndex;
use fxhash::FxHashMap;
use itertools::Itertools;
use loro_common::{ContainerID, IdFull, IdLp, IdSpan, Lamport, PeerID, TreeID, ID};
use tracing::trace;

use crate::{
container::{idx::ContainerIdx, tree::tree_op::TreeOp},
Expand Down Expand Up @@ -50,7 +51,7 @@ impl DiffCalculatorTrait for TreeDiffCalculator {

fn apply_change(
&mut self,
oplog: &OpLog,
_oplog: &OpLog,
op: crate::op::RichOp,
_vv: Option<&crate::VersionVector>,
) {
Expand Down Expand Up @@ -193,11 +194,6 @@ impl TreeDiffCalculator {
}

// forward and apply
let current_frontiers = tree_cache.current_vv.to_frontiers(&oplog.dag);
let forward_min_lamport = self
.get_min_lamport_by_frontiers(&current_frontiers, oplog)
.min(min_lamport);

let max_lamport = self.get_max_lamport_by_frontiers(&to_frontiers, oplog);
let mut forward_ops = vec![];
let group = h
Expand All @@ -207,7 +203,7 @@ impl TreeDiffCalculator {
.unwrap();
for (idlp, op) in group.ops().range(
IdLp {
lamport: forward_min_lamport,
lamport: 0,
peer: 0,
}..=IdLp {
lamport: max_lamport,
Expand Down Expand Up @@ -246,7 +242,6 @@ impl TreeDiffCalculator {
let mark = h.ensure_importing_caches_exist();
let tree_ops = h.get_tree(&self.container, mark).unwrap();
let mut tree_cache = tree_ops.tree().lock().unwrap();

let s = tracing::span!(tracing::Level::INFO, "checkout_diff");
let _e = s.enter();
let to_frontiers = to.to_frontiers(&oplog.dag);
Expand Down Expand Up @@ -293,6 +288,14 @@ impl TreeDiffCalculator {
// we need to know whether old_parent is deleted
let is_parent_deleted = tree_cache.is_parent_deleted(op.op.parent_id());
let is_old_parent_deleted = tree_cache.is_parent_deleted(old_parent);
if op.op.target().id() == op.id.id() {
assert_eq!(
old_parent,
TreeParentId::Unexist,
"old_parent = {:?} instead",
&old_parent
);
}
let this_diff = TreeDeltaItem::new(
op.op.target(),
old_parent,
Expand Down
13 changes: 13 additions & 0 deletions crates/loro-internal/src/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,19 @@ impl<'a> ExportMode<'a> {
version: Cow::Borrowed(frontiers),
}
}

pub fn updates_till(vv: &VersionVector) -> ExportMode<'static> {
let mut spans = Vec::with_capacity(vv.len());
for (peer, counter) in vv.iter() {
if *counter > 0 {
spans.push(IdSpan::new(*peer, 0, *counter));
}
}

ExportMode::UpdatesInRange {
spans: Cow::Owned(spans),
}
}
}

const MAGIC_BYTES: [u8; 4] = *b"loro";
Expand Down
23 changes: 19 additions & 4 deletions crates/loro-internal/src/encoding/fast_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ impl OpLog {
}

pub(crate) fn encode_snapshot<W: std::io::Write>(doc: &LoroDoc, w: &mut W) {
// events should be emitted before encode snapshot
assert!(doc.drop_pending_events().is_empty());
let old_state_frontiers = doc.state_frontiers();
let was_detached = doc.is_detached();
let mut state = doc.app_state().try_lock().unwrap();
let oplog = doc.oplog().try_lock().unwrap();
let is_gc = state.store.gc_store().is_some();
Expand All @@ -169,18 +173,23 @@ pub(crate) fn encode_snapshot<W: std::io::Write>(doc: &LoroDoc, w: &mut W) {
return;
}
assert!(!state.is_in_txn());
assert_eq!(oplog.frontiers(), &state.frontiers);

let oplog_bytes = oplog.encode_change_store();
state.ensure_all_alive_containers();

if oplog.is_trimmed() {
assert_eq!(
oplog.trimmed_frontiers(),
state.store.trimmed_frontiers().unwrap()
);
}

if was_detached {
let latest = oplog.frontiers().clone();
drop(oplog);
drop(state);
doc.checkout_without_emitting(&latest).unwrap();
state = doc.app_state().try_lock().unwrap();
}

state.ensure_all_alive_containers();
let state_bytes = state.store.encode();
_encode_snapshot(
Snapshot {
Expand All @@ -190,6 +199,12 @@ pub(crate) fn encode_snapshot<W: std::io::Write>(doc: &LoroDoc, w: &mut W) {
},
w,
);

if was_detached {
drop(state);
doc.checkout_without_emitting(&old_state_frontiers).unwrap();
doc.drop_pending_events();
}
}

pub(crate) fn encode_snapshot_at<W: std::io::Write>(
Expand Down
Loading

0 comments on commit 78aaa40

Please sign in to comment.