From be17aed3dca7413cbd2ecbb93b2f1b633102c03c Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Thu, 1 Oct 2026 05:42:54 +0800 Subject: [PATCH 1/2] fix: Propagate unreadable lazy history errors See context/arena-parent-links.md for first-reader errors and infallible compatibility. Co-Authored-By: GPT-6 --- .changeset/fallible-unparsable-history.md | 5 + Cargo.lock | 1 + context/arena-parent-links.md | 141 +++++---- crates/loro-internal/src/dag.rs | 289 ++++++++++-------- crates/loro-internal/src/dag/iter.rs | 11 +- crates/loro-internal/src/diff_calc.rs | 53 +++- .../src/encoding/fast_snapshot.rs | 2 +- .../src/encoding/shallow_snapshot.rs | 43 ++- crates/loro-internal/src/loro.rs | 262 +++++++++++----- crates/loro-internal/src/oplog.rs | 274 +++++++++++------ .../loro-internal/src/oplog/change_store.rs | 244 ++++++++++----- crates/loro-internal/src/oplog/loro_dag.rs | 171 ++++++++--- crates/loro-internal/src/version.rs | 18 +- crates/loro-wasm/src/lib.rs | 20 +- .../fixtures/unparsable-history.snapshot.bin | Bin 0 -> 9532 bytes .../tests/unparsable_history.test.ts | 70 +++++ crates/loro/Cargo.toml | 1 + crates/loro/src/lib.rs | 38 +++ crates/loro/tests/perf_history_lazy_load.rs | 68 +++++ .../tests/unregistered_container_parent.rs | 266 ++++++++++++++++ 20 files changed, 1476 insertions(+), 501 deletions(-) create mode 100644 .changeset/fallible-unparsable-history.md create mode 100644 crates/loro-wasm/tests/fixtures/unparsable-history.snapshot.bin create mode 100644 crates/loro-wasm/tests/unparsable_history.test.ts create mode 100644 crates/loro/tests/perf_history_lazy_load.rs diff --git a/.changeset/fallible-unparsable-history.md b/.changeset/fallible-unparsable-history.md new file mode 100644 index 000000000..62c460cc4 --- /dev/null +++ b/.changeset/fallible-unparsable-history.md @@ -0,0 +1,5 @@ +--- +"loro-crdt": patch +--- + +Return decode errors when fallible history operations first read an unreadable change block, preserving document state instead of panicking during lazy DAG loading. diff --git a/Cargo.lock b/Cargo.lock index c2b079b9e..1e8cb0c3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1606,6 +1606,7 @@ dependencies = [ "serde_json", "serial_test", "tracing", + "xxhash-rust", ] [[package]] diff --git a/context/arena-parent-links.md b/context/arena-parent-links.md index 0d0d4665c..866815f4a 100644 --- a/context/arena-parent-links.md +++ b/context/arena-parent-links.md @@ -1,6 +1,6 @@ # Arena Parent Links -Verified against code 2026-09-30. +Verified against code 2026-10-01. `SharedArena` (`crates/loro-internal/src/arena.rs`) stores each container's parent. Liveness (`DocState::is_deleted`), paths (`DocState::get_path`, @@ -105,58 +105,93 @@ No path leads to it, so callers treat it like a deleted container: ## A block that cannot be parsed -Snapshot import validates the KV checksums (`ChangeStore::import_all`), so a -lazily loaded block that fails to parse is forged or truncated external input, -not an internal invariant. The resolver runs under the state lock from queries -that return `bool` or `Option` (`is_deleted`, `has_container`, `get_path`), so -it has no `Err` to return, and a panic there unwinds under the locks and traps -the WASM instance. Until 2026-09-30 it panicked. - -Every reader of the change store that fails to decode or parse a block records -it in `ChangeStore::parse_failures` (a leaf lock; the first failure is kept) and -answers "no such change", as the readers other than the resolver always did. -The resolver answers `CreatorOp::Corrupt`, which the arena treats like `Absent` -for that one lookup: the container reads as deleted, the ID as not a container. - -That answer may be wrong, so `OpLog::check_history_parsable` turns the record -into `DecodeError("cannot parse change block ...")`. **Only public entry points -that return a `Result` call it:** - -- `LoroDoc::checkout`, `diff` and `revert_to` (through `diff_events`), and - `import` (every path, through - `import_changes_and_apply_delta_to_state_if_needed`), before they start. -- `LoroDoc::export` (and so `fork_at` and `merge`), before it starts and again - when it is done, so an export that is the first to read the block fails - instead of returning bytes without the block's changes. - -Do not move the check into `_checkout_without_emitting` or the exporters. -`undo` (`calc_diff`), `checkout_to_latest`, and `fork` on a detached doc call -them and `unwrap` the result, so a check there turns the record into a panic -on those paths (the first version of this fix did). `fork` uses -`export_inner(.., false)` / `fork_at_inner(.., false)` for the same reason. - -Limits: - -- The paths without the check behave as before the record existed. They can - still panic when they need the broken block itself: `AppDag` loads its nodes - from the change store and panics with "unparsed vv don't match with change - store" (`loro_dag.rs`, `ensure_lazy_load_node`). -- A read that returns before any failure was recorded is not undone: an - `import` or `checkout` that is the first to read the block finishes on the - partial history. Only `export` checks again at the end. -- Local edits still succeed after a failure was recorded, but they cannot be - exported from this document any more. What can be salvaged is the current - state (`get_deep_value`). - -Tests, in `change_store.rs`: -`the_creator_resolver_reports_an_unparsable_block_instead_of_panicking`, -`every_reader_records_an_unparsable_block`, -`a_doc_with_an_unparsable_block_neither_panics_nor_exports_partial_history` -(a truncated block; the container is one that a healthy history finds), -`an_export_that_finds_an_unparsable_block_fails_instead_of_skipping_it`, and -`a_recorded_parse_failure_does_not_panic_where_no_error_can_be_returned` (the -record is set by hand on a healthy doc, to separate the check from the DAG -panic above). +Snapshot import validates the KV checksums (`ChangeStore::import_all`), but +loads full-snapshot history lazily. A forged or truncated block with recomputed +checksums can therefore be accepted before its contents are read. Honest data +does not produce such a block; a decode/parse failure is invalid external input, +not an impossible internal invariant. + +Every change-store reader that fails to decode or parse records the block in +`ChangeStore::parse_failures` (a leaf lock; the first failure is kept). Legacy +`Option` readers answer "no such change". The creator resolver answers +`CreatorOp::Corrupt`, which the arena treats like `Absent` for that one lookup: +`is_deleted`, `has_container`, and `get_path` cannot return an error, and the +container reads as deleted or absent. This avoids panicking under their state +lock, but does not establish that the container is actually absent. + +`OpLog::check_history_parsable` converts the record to +`DecodeError("cannot parse change block ...")`. Public fallible operations check +already recorded failures, and their history reads also propagate failures +**on the first read**, without requiring an earlier query to record them: + +- `AppDag::ensure_lazy_load_node` returns `LoroResult<()>`. If the change store + cannot provide a node, it checks the recorded parse failure and returns it. + If there is no parse failure, an id promised by `unparsed_vv` but absent from + the store is still an impossible internal inconsistency: the original + "unparsed vv don't match with change store" assertion remains. +- DAG version conversion, replay-base selection, causal iteration, and diff + calculation use fallible reads. `checkout`, `diff`, `revert_to`, `fork_at`, + binary/JSON/batch imports, and shallow/state-only/snapshot-at exports propagate + the error before applying state. Attached imports enable their existing + rollback journal when old history is still unparsed, including register-only + imports that otherwise need no journal. Initial shallow-snapshot import also + returns the error if reading its root fails, then resets state and op log. +- `ChangeStore::try_iter_changes` validates the requested range before handing + changes to a consumer. Export stops at an unreadable block rather than + feeding a gap into a scratch history or returning partial bytes; the public + export's final history check returns the recorded error. +- Query signatures that previously returned `Option` or plain values remain + available. Use `try_frontiers_to_vv`, `try_vv_to_frontiers`, + `try_minimize_frontiers`, `try_find_id_spans_between`, `try_get_change` + (`OpLog::try_get_change_at`), and `try_get_changed_containers_in` to receive + decode errors. `cmp_frontiers` keeps its `FrontiersNotIncluded` error type, + which can now carry the decode-error message; `travel_change_ancestors` + returns `ChangeTravelError::HistoryUnreadable(LoroError::DecodeError(..))`. + WASM queries that already return `JsResult` use these fallible readers too, + including `findIdSpansBetween`, `frontiersToVV`, `vvToFrontiers`, + `getChangeAt`, `getChangeAtLamport`, `getOpsInChange`, and + `getChangedContainersIn`; their JS names and return types stay the same. + +A failed fallible operation leaves materialized values, document versions, and +attached/detached status unchanged; loading valid blocks may populate history +caches. The successful DAG lazy-load path does no additional block reads or +parse-failure checks: the error-record lookup is inside the missing-node branch. +The release benchmark `crates/loro/tests/perf_history_lazy_load.rs` measures +fresh snapshot import, cold historical checkout, and concurrent update import. + +Limits and compatibility: + +- Reads that need no history do not eagerly validate every block. For example, + full snapshot import and byte-faithful full snapshot export can copy an + unreadable block before any failure has been recorded. A query that can use + the latest version directly may also succeed without reading it. +- The legacy helpers used by `undo`, `checkout_to_latest`, and `fork` retain + their infallible boundaries. Their fallible reads record the failure first, + then their existing unwrap boundary may panic if they need the broken history. + A detached `fork` fails + outside the source document's locks, leaving fallible reads on the source + able to report the record. Other infallible paths can still unwind under + locks or trap WASM. Do not add blanket record checks to their shared internal + helpers: a recorded failure alone must not break an operation that can finish + using healthy history. +- Local edits and `get_deep_value` can still use the current state after a + failure is recorded. Fallible history operations and public exports then + reject the unreadable history. + +Regression tests in `change_store.rs` cover every store reader, the creator +resolver, first DAG reads on tree history, and infallible compatibility. The +internal missing-node assertion is tested in `loro_dag.rs`. +`crates/loro/tests/unregistered_container_parent.rs` +rewrites snapshot bytes through the public KV API and repairs both checksum +layers. Each fallible entry point gets its own fresh document, with the first, +middle, and last peer-1 block truncated; it must return "cannot parse change +block" without a panic, preserve the value/version/status, and leave locks +usable. It also tests initial shallow-import rollback. The healthy-doc test +`a_recorded_parse_failure_does_not_panic_where_no_error_can_be_returned` keeps +manual error records separate from actual unreadable blocks. +`crates/loro-wasm/tests/unparsable_history.test.ts` uses the same corrupted +snapshot as a fixture and checks JS exceptions, unchanged values/versions, +and a usable WASM instance after each first read. ## Cost of looking up an ID diff --git a/crates/loro-internal/src/dag.rs b/crates/loro-internal/src/dag.rs index cf4538560..ff194495c 100644 --- a/crates/loro-internal/src/dag.rs +++ b/crates/loro-internal/src/dag.rs @@ -12,10 +12,11 @@ use std::{ fmt::Debug, }; -use loro_common::IdSpanVector; +use loro_common::{IdSpanVector, LoroError, LoroResult}; use rle::{HasLength, Sliceable}; use rustc_hash::{FxHashMap, FxHashSet}; mod iter; +pub(crate) use iter::DagCausalIter; mod mermaid; #[cfg(feature = "test_utils")] mod test; @@ -31,7 +32,7 @@ use crate::{ }; use self::{ - iter::{iter_dag, iter_dag_with_vv, DagCausalIter, DagIterator, DagIteratorVV}, + iter::{iter_dag, iter_dag_with_vv, DagIterator, DagIteratorVV}, mermaid::dag_to_mermaid, }; @@ -53,6 +54,9 @@ pub(crate) trait Dag: Debug { type Node: DagNode; fn get(&self, id: ID) -> Option; + fn try_get(&self, id: ID) -> LoroResult> { + Ok(self.get(id)) + } #[allow(unused)] fn frontier(&self) -> &Frontiers; fn vv(&self) -> &VersionVector; @@ -95,13 +99,34 @@ pub(crate) trait DagUtils: Dag { /// one it can find itself). Finding the single-head cut walks the whole /// DAG, so callers that usually have a better answer should not pay for /// it up front. - fn find_meet_and_mode(&self, a_id: &Frontiers, b_id: &Frontiers) -> (MeetAsBase, DiffMode); - fn latest_single_head_critical_version(&self, a_id: &Frontiers, b_id: &Frontiers) -> Frontiers; + #[cfg(test)] + fn find_meet_and_mode(&self, a_id: &Frontiers, b_id: &Frontiers) -> (MeetAsBase, DiffMode) { + self.try_find_meet_and_mode(a_id, b_id).unwrap() + } + fn try_find_meet_and_mode( + &self, + a_id: &Frontiers, + b_id: &Frontiers, + ) -> LoroResult<(MeetAsBase, DiffMode)>; + #[cfg(test)] + fn latest_single_head_critical_version(&self, a_id: &Frontiers, b_id: &Frontiers) -> Frontiers { + self.try_latest_single_head_critical_version(a_id, b_id) + .unwrap() + } + fn try_latest_single_head_critical_version( + &self, + a_id: &Frontiers, + b_id: &Frontiers, + ) -> LoroResult; /// Slow, should probably only use on dev #[allow(unused)] fn get_vv(&self, id: ID) -> VersionVector; #[allow(unused)] - fn find_path(&self, from: &Frontiers, to: &Frontiers) -> VersionVectorDiff; + fn find_path(&self, from: &Frontiers, to: &Frontiers) -> VersionVectorDiff { + self.try_find_path(from, to).unwrap() + } + fn try_find_path(&self, from: &Frontiers, to: &Frontiers) -> LoroResult; + #[cfg(test)] fn iter_causal(&self, from: Frontiers, target: IdSpanVector) -> DagCausalIter<'_, Self> where Self: Sized; @@ -135,13 +160,21 @@ impl DagUtils for T { } #[inline] - fn find_meet_and_mode(&self, a_id: &Frontiers, b_id: &Frontiers) -> (MeetAsBase, DiffMode) { - find_meet_and_mode(&|id| self.get(id), a_id, b_id) + fn try_find_meet_and_mode( + &self, + a_id: &Frontiers, + b_id: &Frontiers, + ) -> LoroResult<(MeetAsBase, DiffMode)> { + find_meet_and_mode(&|id| self.try_get(id), a_id, b_id) } #[inline] - fn latest_single_head_critical_version(&self, a_id: &Frontiers, b_id: &Frontiers) -> Frontiers { - latest_single_head_critical_version(&|id| self.get(id), a_id, b_id) + fn try_latest_single_head_critical_version( + &self, + a_id: &Frontiers, + b_id: &Frontiers, + ) -> LoroResult { + latest_single_head_critical_version(&|id| self.try_get(id), a_id, b_id) } #[inline] @@ -149,18 +182,20 @@ impl DagUtils for T { get_version_vector(&|id| self.get(id), id) } - fn find_path(&self, from: &Frontiers, to: &Frontiers) -> VersionVectorDiff { + fn try_find_path(&self, from: &Frontiers, to: &Frontiers) -> LoroResult { let mut ans = VersionVectorDiff::default(); if from == to { - return ans; + return Ok(ans); } if from.len() == 1 && to.len() == 1 { let from = from.as_single().unwrap(); let to = to.as_single().unwrap(); if from.peer == to.peer { - let from_span = self.get(from).unwrap(); - let to_span = self.get(to).unwrap(); + let from_span = self + .try_get(from)? + .ok_or(LoroError::FrontiersNotFound(from))?; + let to_span = self.try_get(to)?.ok_or(LoroError::FrontiersNotFound(to))?; if from_span.id_start() == to_span.id_start() { if from.counter < to.counter { ans.forward.insert( @@ -173,7 +208,7 @@ impl DagUtils for T { CounterSpan::new(to.counter + 1, from.counter + 1), ); } - return ans; + return Ok(ans); } if from_span.deps().len() == 1 @@ -183,7 +218,7 @@ impl DagUtils for T { from.peer, CounterSpan::new(to.counter + 1, from.counter + 1), ); - return ans; + return Ok(ans); } if to_span.deps().len() == 1 @@ -193,13 +228,13 @@ impl DagUtils for T { from.peer, CounterSpan::new(from.counter + 1, to.counter + 1), ); - return ans; + return Ok(ans); } } } _walk_to_meet( - &|v| self.get(v), + &|v| self.try_get(v), from, to, &mut |span, node_type| match node_type { @@ -211,9 +246,9 @@ impl DagUtils for T { } }, true, - ); + )?; - ans + Ok(ans) } #[inline(always)] @@ -225,6 +260,7 @@ impl DagUtils for T { } #[inline(always)] + #[cfg(test)] fn iter_causal(&self, from: Frontiers, target: IdSpanVector) -> DagCausalIter<'_, Self> where Self: Sized, @@ -342,19 +378,21 @@ enum NodeType { impl<'a> OrdIdSpan<'a> { #[inline] - fn from_dag_node(id: ID, get: &'a F) -> Option> + fn from_dag_node(id: ID, get: &'a F) -> LoroResult>> where D: DagNode + 'a, - F: Fn(ID) -> Option, + F: Fn(ID) -> LoroResult>, { - let span = get(id)?; + let Some(span) = get(id)? else { + return Ok(None); + }; let span_id = span.id_start(); - Some(OrdIdSpan { + Ok(Some(OrdIdSpan { id: span_id, lamport: span.lamport(), deps: Cow::Owned(span.deps().clone()), len: (id.counter - span_id.counter) as usize + 1, - }) + })) } #[inline] @@ -373,13 +411,13 @@ fn find_meet_and_mode<'a, F, D>( get: &'a F, a_id: &Frontiers, b_id: &Frontiers, -) -> (MeetAsBase, DiffMode) +) -> LoroResult<(MeetAsBase, DiffMode)> where D: DagNode + 'a, - F: Fn(ID) -> Option, + F: Fn(ID) -> LoroResult>, { if b_id.is_empty() { - return (MeetAsBase::Valid(Default::default()), DiffMode::Checkout); + return Ok((MeetAsBase::Valid(Default::default()), DiffMode::Checkout)); } _find_meet_and_mode(get, a_id, b_id) @@ -392,19 +430,19 @@ fn _walk_to_meet<'a, F, D, G>( b_ids: &Frontiers, notify: &mut G, find_path: bool, -) -> FxHashMap +) -> LoroResult> where D: DagNode + 'a, - F: Fn(ID) -> Option, + F: Fn(ID) -> LoroResult>, G: FnMut(IdSpan, NodeType), { let mut ans: FxHashMap = Default::default(); let mut queue: BinaryHeap<(OrdIdSpan, NodeType)> = BinaryHeap::new(); for id in a_ids.iter() { - queue.push((OrdIdSpan::from_dag_node(id, get).unwrap(), NodeType::A)); + queue.push((OrdIdSpan::from_dag_node(id, get)?.unwrap(), NodeType::A)); } for id in b_ids.iter() { - queue.push((OrdIdSpan::from_dag_node(id, get).unwrap(), NodeType::B)); + queue.push((OrdIdSpan::from_dag_node(id, get)?.unwrap(), NodeType::B)); } let mut visited: HashMap = FxHashMap::default(); // invariants in this method: @@ -502,7 +540,7 @@ where } for dep_id in node.deps.as_ref().iter() { - queue.push((OrdIdSpan::from_dag_node(dep_id, get).unwrap(), node_type)); + queue.push((OrdIdSpan::from_dag_node(dep_id, get)?.unwrap(), node_type)); } if node_type != NodeType::Shared { @@ -535,44 +573,46 @@ where } } - ans + Ok(ans) } /// Resolves each frontier id to the span of its containing node, truncated at /// that id. Returns `None` when an id is unavailable (trimmed history). -fn ids_to_ord_id_spans<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( +fn ids_to_ord_id_spans<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( ids: &Frontiers, get: &'a F, -) -> Option>> { +) -> LoroResult>>> { let mut ans = Vec::with_capacity(ids.len()); for id in ids.iter() { - if let Some(node) = OrdIdSpan::from_dag_node(id, get) { + if let Some(node) = OrdIdSpan::from_dag_node(id, get)? { ans.push(node); } else { - return None; + return Ok(None); } } - Some(ans) + Ok(Some(ans)) } /// The parent set of a span's first op: the node's explicit deps plus the /// implicit same-peer predecessor when it is not already covered by one of them. -fn deps_to_ord_id_spans<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( +fn deps_to_ord_id_spans<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( node: &OrdIdSpan<'a>, get: &'a F, -) -> Option>> { - let mut deps = ids_to_ord_id_spans(node.deps.as_ref(), get)?; +) -> LoroResult>>> { + let Some(mut deps) = ids_to_ord_id_spans(node.deps.as_ref(), get)? else { + return Ok(None); + }; if node.id.counter > 0 { let prev = node.id.inc(-1); - if let Some(prev) = OrdIdSpan::from_dag_node(prev, get) { + if let Some(prev) = OrdIdSpan::from_dag_node(prev, get)? { if !deps.iter().any(|dep| dep.contains_id(prev.id_last())) { deps.push(prev); } } } - Some(deps) + Ok(Some(deps)) } /// The latest single-head critical version (Eg-walker §3.5/§3.6) of the @@ -590,18 +630,18 @@ fn deps_to_ord_id_spans<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( /// endpoint is concurrent with everything below the current level, so no /// later singleton can be valid — bail out to the empty version, which is /// the old behaviour. -fn latest_single_head_critical_version<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( +fn latest_single_head_critical_version<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( get: &'a F, left: &Frontiers, right: &Frontiers, -) -> Frontiers { +) -> LoroResult { let mut queue: BinaryHeap = BinaryHeap::new(); - let Some(spans) = ids_to_ord_id_spans(left, get) else { - return Default::default(); + let Some(spans) = ids_to_ord_id_spans(left, get)? else { + return Ok(Default::default()); }; queue.extend(spans); - let Some(spans) = ids_to_ord_id_spans(right, get) else { - return Default::default(); + let Some(spans) = ids_to_ord_id_spans(right, get)? else { + return Ok(Default::default()); }; queue.extend(spans); @@ -615,7 +655,7 @@ fn latest_single_head_critical_version<'a, D: DagNode + 'a, F: Fn(ID) -> Option< } if queue.is_empty() { - return node.id_last().into(); + return Ok(node.id_last().into()); } if let Some(other) = queue.peek() { @@ -636,17 +676,17 @@ fn latest_single_head_critical_version<'a, D: DagNode + 'a, F: Fn(ID) -> Option< } } - match deps_to_ord_id_spans(&node, get) { + match deps_to_ord_id_spans(&node, get)? { Some(deps) if !deps.is_empty() => { for dep in deps { queue.push(dep); } } - _ => return Default::default(), + _ => return Ok(Default::default()), } } - Default::default() + Ok(Default::default()) } /// Finds the replay base and diff mode for the transition `left -> right`. @@ -671,65 +711,65 @@ fn _find_meet_and_mode<'a, F, D>( get: &'a F, left: &Frontiers, right: &Frontiers, -) -> (MeetAsBase, DiffMode) +) -> LoroResult<(MeetAsBase, DiffMode)> where D: DagNode + 'a, - F: Fn(ID) -> Option, + F: Fn(ID) -> LoroResult>, { if right.is_empty() { - return (MeetAsBase::Valid(Default::default()), DiffMode::Checkout); + return Ok((MeetAsBase::Valid(Default::default()), DiffMode::Checkout)); } if left.is_empty() { if right.len() == 1 { let mut node_id = right.as_single().unwrap(); - let mut node = get(node_id).unwrap(); + let mut node = get(node_id)?.unwrap(); while node.deps().len() == 1 { node_id = node.deps().as_single().unwrap(); - let Some(next) = get(node_id) else { - return ( + let Some(next) = get(node_id)? else { + return Ok(( MeetAsBase::Valid(Default::default()), DiffMode::ImportGreaterUpdates, - ); + )); }; node = next; } if node.deps().is_empty() { - return (MeetAsBase::Valid(Default::default()), DiffMode::Linear); + return Ok((MeetAsBase::Valid(Default::default()), DiffMode::Linear)); } } - return ( + return Ok(( MeetAsBase::Valid(Default::default()), DiffMode::ImportGreaterUpdates, - ); + )); } if left.len() == 1 && right.len() == 1 { let left = left.as_single().unwrap(); let right = right.as_single().unwrap(); if left.peer == right.peer { - let left_span = get(left).unwrap(); - let right_span = get(right).unwrap(); + let left_span = get(left)?.unwrap(); + let right_span = get(right)?.unwrap(); if left_span.id_start() == right_span.id_start() { if left.counter < right.counter { - return (MeetAsBase::Valid(left.into()), DiffMode::Linear); + return Ok((MeetAsBase::Valid(left.into()), DiffMode::Linear)); } else { - return (MeetAsBase::Valid(right.into()), DiffMode::Checkout); + return Ok((MeetAsBase::Valid(right.into()), DiffMode::Checkout)); } } if left_span.deps().len() == 1 && right_span.contains_id(left_span.deps().as_single().unwrap()) { - return (MeetAsBase::Valid(right.into()), DiffMode::Checkout); + return Ok((MeetAsBase::Valid(right.into()), DiffMode::Checkout)); } if right_span.deps().len() == 1 && left_span.contains_id(right_span.deps().as_single().unwrap()) { - return (MeetAsBase::Valid(left.into()), DiffMode::Linear); + return Ok((MeetAsBase::Valid(left.into()), DiffMode::Linear)); } } } @@ -740,21 +780,21 @@ where let mut has_unresolved_unmatched_branch = false; let mut ans: Frontiers = Default::default(); - fn shrink_ancestor_frontiers<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( + fn shrink_ancestor_frontiers<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( ids: &Frontiers, get: &'a F, - ) -> Frontiers { + ) -> LoroResult { if ids.len() <= 1 { - return ids.clone(); + return Ok(ids.clone()); } - let mut ids = ids_to_ord_id_spans(ids, get).expect("meet candidates should be in dag"); + let mut ids = ids_to_ord_id_spans(ids, get)?.expect("meet candidates should be in dag"); ids.sort_unstable(); let mut frontiers = Vec::with_capacity(ids.len()); for id in ids.iter().rev() { let mut should_insert = true; for frontier in frontiers.iter().rev() { - if contains_in_ancestors(get, *frontier, id) { + if contains_in_ancestors(get, *frontier, id)? { should_insert = false; break; } @@ -765,19 +805,22 @@ where } } - frontiers.into_iter().collect() + Ok(frontiers.into_iter().collect()) } - fn has_trimmed_history_deps<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( + fn has_trimmed_history_deps<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( ids: &Frontiers, get: &'a F, - ) -> bool { - ids.iter().any(|id| { - let Some(node) = OrdIdSpan::from_dag_node(id, get) else { - return true; + ) -> LoroResult { + for id in ids.iter() { + let Some(node) = OrdIdSpan::from_dag_node(id, get)? else { + return Ok(true); }; - ids_to_ord_id_spans(node.deps.as_ref(), get).is_none() - }) + if ids_to_ord_id_spans(node.deps.as_ref(), get)?.is_none() { + return Ok(true); + } + } + Ok(false) } /// Whether every tip in `tips` is contained in the ancestors of `frontiers` @@ -785,21 +828,21 @@ where /// frontier with a shared visited set and prunes by the lowest lamport among /// the still-unproven tips, so the ancestor region is traversed at most once /// per call instead of once per (tip, frontier) pair. - fn all_tips_covered_by_ancestors<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( + fn all_tips_covered_by_ancestors<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( get: &'a F, frontiers: &Frontiers, tips: &FxHashSet, - ) -> bool { + ) -> LoroResult { let mut remaining = Vec::with_capacity(tips.len()); for id in tips.iter() { - let Some(span) = OrdIdSpan::from_dag_node(*id, get) else { - return false; + let Some(span) = OrdIdSpan::from_dag_node(*id, get)? else { + return Ok(false); }; remaining.push(span); } if remaining.is_empty() { - return true; + return Ok(true); } let mut min_lamport = remaining @@ -810,7 +853,7 @@ where let mut visited = FxHashSet::default(); let mut pending = Vec::new(); for frontier in frontiers.iter() { - if let Some(node) = OrdIdSpan::from_dag_node(frontier, get) { + if let Some(node) = OrdIdSpan::from_dag_node(frontier, get)? { pending.push(node); } } @@ -819,7 +862,7 @@ where let len_before = remaining.len(); remaining.retain(|tip| !node.contains_id(tip.id_last())); if remaining.is_empty() { - return true; + return Ok(true); } if remaining.len() != len_before { @@ -840,14 +883,14 @@ where continue; } - if let Some(deps) = deps_to_ord_id_spans(&node, get) { + if let Some(deps) = deps_to_ord_id_spans(&node, get)? { for dep in deps { pending.push(dep); } } } - false + Ok(false) } /// Verifies the `ImportGreaterUpdates` contract for a multi-head `left`: @@ -866,15 +909,15 @@ where /// through their new parents. The common healthy shape — a change whose /// deps equal the left frontier — passes with a set comparison and no /// graph walk. Trimmed history fails conservatively. - fn new_region_after_all_left_heads<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( + fn new_region_after_all_left_heads<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( get: &'a F, left: &Frontiers, right: &Frontiers, - ) -> bool { + ) -> LoroResult { let left_ids: FxHashSet = left.iter().collect(); - let is_old = |id: ID| -> bool { + let is_old = |id: ID| -> LoroResult { if left_ids.contains(&id) { - return true; + return Ok(true); } let mut single = FxHashSet::default(); single.insert(id); @@ -884,11 +927,11 @@ where let mut visited: FxHashSet = FxHashSet::default(); let mut stack: Vec = Vec::new(); for id in right.iter() { - if is_old(id) { + if is_old(id)? { continue; } - let Some(span) = OrdIdSpan::from_dag_node(id, get) else { - return false; + let Some(span) = OrdIdSpan::from_dag_node(id, get)? else { + return Ok(false); }; stack.push(span); } @@ -898,14 +941,14 @@ where continue; } - let Some(parents) = deps_to_ord_id_spans(&span, get) else { - return false; + let Some(parents) = deps_to_ord_id_spans(&span, get)? else { + return Ok(false); }; let mut entry_point = true; let mut old_parents = Frontiers::default(); for parent in parents { let pid = parent.id_last(); - if is_old(pid) { + if is_old(pid)? { old_parents.push(pid); } else { entry_point = false; @@ -918,29 +961,29 @@ where continue; } - if !all_tips_covered_by_ancestors(get, &old_parents, &left_ids) { - return false; + if !all_tips_covered_by_ancestors(get, &old_parents, &left_ids)? { + return Ok(false); } } } - true + Ok(true) } - fn contains_in_ancestors<'a, D: DagNode + 'a, F: Fn(ID) -> Option>( + fn contains_in_ancestors<'a, D: DagNode + 'a, F: Fn(ID) -> LoroResult>>( get: &'a F, frontier: ID, target: &OrdIdSpan<'_>, - ) -> bool { + ) -> LoroResult { let mut visited = FxHashSet::default(); let mut pending = Vec::new(); - let Some(node) = OrdIdSpan::from_dag_node(frontier, get) else { - return false; + let Some(node) = OrdIdSpan::from_dag_node(frontier, get)? else { + return Ok(false); }; pending.push(node); while let Some(node) = pending.pop() { if node.contains_id(target.id_last()) { - return true; + return Ok(true); } if node.lamport_last() < target.lamport_last() { @@ -951,14 +994,14 @@ where continue; } - if let Some(deps) = deps_to_ord_id_spans(&node, get) { + if let Some(deps) = deps_to_ord_id_spans(&node, get)? { for dep in deps { pending.push(dep); } } } - false + Ok(false) } // The third tuple item carries the dependency tips at which this path first @@ -966,7 +1009,7 @@ where // side, those tips tell us whether it was a real concurrent branch or merely // a redundant route into an ancestor we already found. let mut queue: BinaryHeap<(OrdIdSpan, NodeType, Vec)> = BinaryHeap::new(); - for span in ids_to_ord_id_spans(left, get).unwrap() { + for span in ids_to_ord_id_spans(left, get)?.unwrap() { let branch_tips = if left.len() > 1 { vec![span.id_last()] } else { @@ -975,7 +1018,7 @@ where queue.push((span, NodeType::A, branch_tips)); } - for span in ids_to_ord_id_spans(right, get).unwrap() { + for span in ids_to_ord_id_spans(right, get)?.unwrap() { let branch_tips = if right.len() > 1 { vec![span.id_last()] } else { @@ -1049,7 +1092,7 @@ where } } - if let Some(deps) = deps_to_ord_id_spans(&node, get) { + if let Some(deps) = deps_to_ord_id_spans(&node, get)? { if !deps.is_empty() { let starts_new_branches = branch_tips.is_empty() && deps.len() > 1; for dep in deps { @@ -1086,20 +1129,20 @@ where } } - ans = shrink_ancestor_frontiers(&ans, get); + ans = shrink_ancestor_frontiers(&ans, get)?; // A branch can look unmatched merely because a shared node was found first // and was therefore not expanded. In that case another queued path may still // walk into one of the shared node's ancestors. That path is redundant, not a // concurrent branch. Only fall back when an unmatched tip is not causally // covered by the meet candidates we found. let has_uncovered_unmatched_branch = has_unresolved_unmatched_branch - || !all_tips_covered_by_ancestors(get, &ans, &unmatched_branches); + || !all_tips_covered_by_ancestors(get, &ans, &unmatched_branches)?; // A genuine concurrent branch invalidates the meet as a replay base. The // caller has to retreat to a critical version; which one is its choice, so // it can try a cheap multi-head cut before paying for the full-DAG // single-head descent. let mut needs_critical_retreat = - has_uncovered_unmatched_branch && !has_trimmed_history_deps(&ans, get); + has_uncovered_unmatched_branch && !has_trimmed_history_deps(&ans, get)?; if has_uncovered_unmatched_branch { is_right_greater = false; @@ -1110,7 +1153,7 @@ where is_right_greater = true; } - if is_right_greater && left.len() > 1 && !new_region_after_all_left_heads(get, left, right) { + if is_right_greater && left.len() > 1 && !new_region_after_all_left_heads(get, left, right)? { // Version inclusion holds, but part of the new region entered old // history through a strict subset of the left heads and is therefore // concurrent with the others. `ImportGreaterUpdates` promises "no @@ -1144,7 +1187,7 @@ where } else { MeetAsBase::Valid(ans) }; - (meet, mode) + Ok((meet, mode)) } pub fn remove_included_frontiers(frontiers: &mut VersionVector, new_change_deps: &[ID]) { @@ -1745,13 +1788,7 @@ mod tests { let root = node(1, 0, 1, 0, Frontiers::default()); let x = node(2, 0, 1, 1, ID::new(1, 0).into()); let y = node(3, 0, 1, 1, ID::new(1, 0).into()); - let merge = node( - 4, - 0, - 1, - 2, - Frontiers::from([ID::new(2, 0), ID::new(3, 0)]), - ); + let merge = node(4, 0, 1, 2, Frontiers::from([ID::new(2, 0), ID::new(3, 0)])); let dag = TestDag::new(vec![root, x, y, merge], ID::new(4, 0).into()); let left = Frontiers::from([ID::new(2, 0), ID::new(3, 0)]); diff --git a/crates/loro-internal/src/dag/iter.rs b/crates/loro-internal/src/dag/iter.rs index 4c824b5b8..d81ed19b9 100644 --- a/crates/loro-internal/src/dag/iter.rs +++ b/crates/loro-internal/src/dag/iter.rs @@ -197,7 +197,12 @@ pub(crate) struct IterReturn { } impl<'a, T: DagNode, D: Dag + Debug> DagCausalIter<'a, D> { + #[cfg(test)] pub fn new(dag: &'a D, from: Frontiers, target: IdSpanVector) -> Self { + Self::try_new(dag, from, target).unwrap() + } + + pub fn try_new(dag: &'a D, from: Frontiers, target: IdSpanVector) -> LoroResult { let mut out_degrees: FxHashMap = FxHashMap::default(); let mut succ: BTreeMap> = BTreeMap::default(); let mut stack = Vec::new(); @@ -213,7 +218,7 @@ impl<'a, T: DagNode, D: Dag + Debug> DagCausalIter<'a, D> { // if out_degree is 0, then it can be iterated directly while let Some(id) = q.pop() { let client = id.peer; - let node = dag.get(id).unwrap(); + let node = dag.try_get(id)?.ok_or(LoroError::FrontiersNotFound(id))?; let deps = node.deps(); out_degrees.insert( id, @@ -273,14 +278,14 @@ impl<'a, T: DagNode, D: Dag + Debug> DagCausalIter<'a, D> { true }); - Self { + Ok(Self { dag, frontier: from, target, out_degrees, succ, stack, - } + }) } } diff --git a/crates/loro-internal/src/diff_calc.rs b/crates/loro-internal/src/diff_calc.rs index 825ddcf19..07e6591b8 100644 --- a/crates/loro-internal/src/diff_calc.rs +++ b/crates/loro-internal/src/diff_calc.rs @@ -122,9 +122,9 @@ fn changed_containers_between( oplog: &OpLog, before: &VersionVector, after: &VersionVector, -) -> FxHashSet { +) -> loro_common::LoroResult> { let (retreat, forward) = before.diff_iter(after); - oplog.containers_in_spans(retreat.chain(forward)) + oplog.try_containers_in_spans(retreat.chain(forward)) } impl DiffCalculator { @@ -160,6 +160,26 @@ impl DiffCalculator { /// It's the expected diff mode inferred from the two version, which can reflect the direction of the /// change). pub(crate) fn calc_diff_internal( + &mut self, + oplog: &OpLog, + before: &VersionVector, + before_frontiers: &Frontiers, + after: &VersionVector, + after_frontiers: &Frontiers, + container_filter: Option<&dyn Fn(ContainerIdx) -> bool>, + ) -> (Vec, DiffMode) { + self.try_calc_diff_internal( + oplog, + before, + before_frontiers, + after, + after_frontiers, + container_filter, + ) + .unwrap() + } + + pub(crate) fn try_calc_diff_internal( &mut self, oplog: &super::oplog::OpLog, before: &crate::VersionVector, @@ -167,9 +187,9 @@ impl DiffCalculator { after: &crate::VersionVector, after_frontiers: &Frontiers, container_filter: Option<&dyn Fn(ContainerIdx) -> bool>, - ) -> (Vec, DiffMode) { + ) -> loro_common::LoroResult<(Vec, DiffMode)> { if before == after { - return (Vec::new(), DiffMode::Linear); + return Ok((Vec::new(), DiffMode::Linear)); } let s = tracing::span!(tracing::Level::INFO, "DiffCalc", ?before, ?after,); @@ -185,14 +205,20 @@ impl DiffCalculator { concurrent_containers, }, iter, - ) = oplog.iter_from_replay_base_causally(before, before_frontiers, after, after_frontiers); + ) = oplog.try_iter_from_replay_base_causally( + before, + before_frontiers, + after, + after_frontiers, + )?; // A conservative replay base may be much older than `before`. The causal replay // still needs that common history as position context, but containers // whose ops are present on both sides cannot contribute to the diff. // Without this filter, every such List/Text/MovableList can trigger its // own full-history safety rebuild below. - let changed_containers = - (&replay_base != before).then(|| changed_containers_between(oplog, before, after)); + let changed_containers = (&replay_base != before) + .then(|| changed_containers_between(oplog, before, after)) + .transpose()?; // Two distinct mode values live in this function — do not conflate them: // - `origin_diff_mode` describes the DIRECTION of the transition // (Checkout can go backwards; the other modes imply `after ⊇ before`). @@ -219,7 +245,8 @@ impl DiffCalculator { let affected_set = { loro_common::debug!("replay_base: {:?} mode={:?}", &replay_base, calc_mode); let mut started_set = FxHashSet::default(); - for (change, (start_counter, end_counter), vv) in iter { + for item in iter { + let (change, (start_counter, end_counter), vv) = item?; let iter_start = change .ops .binary_search_by(|op| op.ctr_last().cmp(&start_counter)) @@ -430,10 +457,10 @@ impl DiffCalculator { } } - ( + Ok(( ans.into_values().map(|x| x.1).collect_vec(), origin_diff_mode, - ) + )) } // TODO: we may remove depth info @@ -2340,7 +2367,7 @@ fn causal_existing_peer_import_uses_current_version_as_replay_base() { assert_eq!(base.vv, before); assert_eq!(base.diff_mode, DiffMode::ImportGreaterUpdates); assert_eq!( - changed_containers_between(&oplog, &before, &after), + changed_containers_between(&oplog, &before, &after).unwrap(), [expected_idx].into_iter().collect() ); @@ -2398,7 +2425,7 @@ fn conservative_replay_only_builds_calculators_for_changed_containers() { ); assert_eq!(base.diff_mode, DiffMode::Import); assert_eq!( - changed_containers_between(&oplog, &before, &after), + changed_containers_between(&oplog, &before, &after).unwrap(), [expected_idx, list_idx].into_iter().collect() ); @@ -2461,7 +2488,7 @@ fn conservative_checkout_replay_filters_to_retreat_changed_containers() { ); assert_eq!(base.diff_mode, DiffMode::Checkout); assert_eq!( - changed_containers_between(&oplog, &before, &after), + changed_containers_between(&oplog, &before, &after).unwrap(), [expected_idx].into_iter().collect() ); diff --git a/crates/loro-internal/src/encoding/fast_snapshot.rs b/crates/loro-internal/src/encoding/fast_snapshot.rs index 526fe2c1f..d9faf8050 100644 --- a/crates/loro-internal/src/encoding/fast_snapshot.rs +++ b/crates/loro-internal/src/encoding/fast_snapshot.rs @@ -275,7 +275,7 @@ pub(crate) fn decode_snapshot_inner( impl OpLog { pub(super) fn decode_change_store(&mut self, bytes: bytes::Bytes) -> LoroResult<()> { let v = self.change_store().import_all(bytes)?; - self.dag.set_version_by_fast_snapshot_import(v); + self.dag.set_version_by_fast_snapshot_import(v)?; self.refresh_visible_op_count(); Ok(()) } diff --git a/crates/loro-internal/src/encoding/shallow_snapshot.rs b/crates/loro-internal/src/encoding/shallow_snapshot.rs index 074c60fd3..6162ade77 100644 --- a/crates/loro-internal/src/encoding/shallow_snapshot.rs +++ b/crates/loro-internal/src/encoding/shallow_snapshot.rs @@ -307,7 +307,7 @@ pub(crate) fn export_shallow_snapshot_inner( cached_root: CachedShallowRoot, ) -> Result<(Snapshot, Frontiers), LoroEncodeError> { let oplog = doc.oplog().lock(); - let start_from = calc_shallow_doc_start(&oplog, start_from, oplog.frontiers()); + let start_from = calc_shallow_doc_start(&oplog, start_from, oplog.frontiers())?; // `root_vv` is the version of the state at the shallow root; `start_vv` // additionally excludes the frontier ops themselves because the retained // history must include them. @@ -324,14 +324,14 @@ pub(crate) fn export_shallow_snapshot_inner( if !start_from.is_empty() { assert!(start_from.len() == 1); let id = start_from.as_single().unwrap(); - let node = oplog.dag.get(id).unwrap(); + let node = oplog.dag.try_get(id)?.unwrap(); if id.counter == node.cnt { - let vv = oplog.dag().frontiers_to_vv(&node.deps).unwrap(); + let vv = oplog.dag().try_frontiers_to_vv(&node.deps)?.unwrap(); assert_eq!(vv, start_vv); } else { let vv = oplog .dag() - .frontiers_to_vv(&Frontiers::from(id.inc(-1))) + .try_frontiers_to_vv(&Frontiers::from(id.inc(-1)))? .unwrap(); assert_eq!(vv, start_vv); } @@ -820,7 +820,7 @@ pub(crate) fn export_state_only_snapshot( w: &mut W, ) -> Result { let oplog = doc.oplog().lock(); - let start_from = calc_shallow_doc_start(&oplog, target_frontiers, target_frontiers); + let start_from = calc_shallow_doc_start(&oplog, target_frontiers, target_frontiers)?; let root_vv = frontiers_to_vv_for_export(&oplog, &start_from, "export_state_only_snapshot")?; let mut start_vv = root_vv.clone(); for id in start_from.iter() { @@ -899,7 +899,7 @@ fn frontiers_to_vv_for_export( frontiers: &Frontiers, context: &str, ) -> Result { - oplog.dag().frontiers_to_vv(frontiers).ok_or_else(|| { + oplog.dag().try_frontiers_to_vv(frontiers)?.ok_or_else(|| { LoroEncodeError::FrontiersNotFound(format!( "{context}: unreachable frontiers {frontiers:?}" )) @@ -941,7 +941,7 @@ fn calc_shallow_doc_start( oplog: &crate::OpLog, frontiers: &Frontiers, retained_to: &Frontiers, -) -> Frontiers { +) -> Result { if frontiers.is_empty() { return clamp_to_shallow_root(oplog, Frontiers::default()); } @@ -950,7 +950,7 @@ fn calc_shallow_doc_start( // version exists, e.g. for independent heads with no common history. let root = oplog .dag() - .latest_single_head_critical_version(frontiers, retained_to); + .try_latest_single_head_critical_version(frontiers, retained_to)?; let mut ans = Frontiers::new(); for id in root.iter() { @@ -979,19 +979,22 @@ fn calc_shallow_doc_start( clamp_to_shallow_root(oplog, ans) } -fn clamp_to_shallow_root(oplog: &crate::OpLog, frontiers: Frontiers) -> Frontiers { +fn clamp_to_shallow_root( + oplog: &crate::OpLog, + frontiers: Frontiers, +) -> Result { if oplog.shallow_since_vv().is_empty() { - return frontiers; + return Ok(frontiers); } - let Some(vv) = oplog.dag().frontiers_to_vv(&frontiers) else { - return oplog.shallow_since_frontiers().clone(); + let Some(vv) = oplog.dag().try_frontiers_to_vv(&frontiers)? else { + return Ok(oplog.shallow_since_frontiers().clone()); }; if vv.includes_vv(&oplog.shallow_since_vv().to_vv()) { - frontiers + Ok(frontiers) } else { - oplog.shallow_since_frontiers().clone() + Ok(oplog.shallow_since_frontiers().clone()) } } @@ -1076,7 +1079,11 @@ pub(crate) fn encode_snapshot_at( "encode_snapshot_at: state is unexpectedly still in a transaction", )); } - let Some(oplog_bytes) = oplog.fork_changes_up_to(frontiers) else { + let oplog_bytes = match oplog.fork_changes_up_to(frontiers) { + Ok(bytes) => bytes, + Err(err) => break 'block Err(err.into()), + }; + let Some(oplog_bytes) = oplog_bytes else { break 'block Err(LoroEncodeError::FrontiersNotFound(format!( "frontiers: {:?} when export in SnapshotAt mode", frontiers @@ -1096,7 +1103,11 @@ pub(crate) fn encode_snapshot_at( } } - let Some(version) = oplog.dag.frontiers_to_vv(frontiers) else { + let version = match oplog.dag.try_frontiers_to_vv(frontiers) { + Ok(version) => version, + Err(err) => break 'block Err(err.into()), + }; + let Some(version) = version else { break 'block Err(LoroEncodeError::FrontiersNotFound(format!( "frontiers: {:?} when export in SnapshotAt mode", frontiers diff --git a/crates/loro-internal/src/loro.rs b/crates/loro-internal/src/loro.rs index 6d86ab69d..500adc503 100644 --- a/crates/loro-internal/src/loro.rs +++ b/crates/loro-internal/src/loro.rs @@ -32,7 +32,7 @@ use crate::{ subscription::{LocalUpdateCallback, Observer, Subscriber}, undo::DiffBatch, utils::subscription::{SubscriberSetWithQueue, Subscription}, - version::{shrink_frontiers, Frontiers, ImVersionVector, VersionRange, VersionVectorDiff}, + version::{Frontiers, ImVersionVector, VersionRange, VersionVectorDiff}, ChangeMeta, DocDiff, HandlerTrait, InternalString, ListHandler, LoroDoc, LoroError, MapHandler, VersionVector, }; @@ -707,7 +707,7 @@ impl LoroDoc { let result = f(&mut oplog); if &old_vv != oplog.vv() { let mut diff = DiffCalculator::new(false); - let (diff, diff_mode) = diff.calc_diff_internal( + let computed = diff.try_calc_diff_internal( &oplog, &old_vv, &old_frontiers, @@ -715,6 +715,13 @@ impl LoroDoc { oplog.dag.get_frontiers(), None, ); + let (diff, diff_mode) = match computed { + Ok(diff) => diff, + Err(err) => { + oplog.rollback_owned_import(owns_rollback, &mut self.state.lock()); + return Err(err); + } + }; let mut state = self.state.lock(); if let Err(e) = state.apply_diff( InternalDocDiff { @@ -782,7 +789,13 @@ impl LoroDoc { } }; - let preflight = oplog.preflight_import_changes(&changes); + let preflight = match oplog.preflight_import_changes(&changes) { + Ok(preflight) => preflight, + Err(err) => { + oplog.rollback_arena(arena_checkpoint, &mut self.state.lock()); + return Err(err); + } + }; if preflight.has_deps_before_shallow_root && (self.is_detached() || !preflight.applies_to_dag) { @@ -841,7 +854,8 @@ impl LoroDoc { !state.store.contains_id(&id) }) }); - let rollback_enabled = preflight.needs_state_apply_rollback; + let rollback_enabled = + preflight.needs_state_apply_rollback || oplog.dag.has_unparsed_history(); if rollback_enabled { oplog.begin_import_rollback_with_arena(arena_checkpoint); } @@ -869,37 +883,54 @@ impl LoroDoc { *end == old_end.max(component_end) }) }); - let (diff, diff_mode) = if let Some((component_vv, _)) = isolated_batch { - #[cfg(test)] - ISOLATED_FAST_PATH_HITS.with(|hits| hits.set(hits.get() + 1)); - let component_frontiers = oplog.dag.vv_to_frontiers(&component_vv); - let (diff, _) = calc.calc_diff_internal( - &oplog, - &VersionVector::default(), - &Frontiers::default(), - &component_vv, - &component_frontiers, - None, - ); - (diff, DiffMode::Import) - } else { - calc.calc_diff_internal( - &oplog, - &old_vv, - &old_frontiers, - oplog.vv(), - oplog.dag.get_frontiers(), - None, - ) + let computed = (|| -> LoroResult<_> { + let (diff, diff_mode) = if let Some((component_vv, _)) = isolated_batch { + #[cfg(test)] + ISOLATED_FAST_PATH_HITS.with(|hits| hits.set(hits.get() + 1)); + let component_frontiers = oplog.dag.try_vv_to_frontiers(&component_vv)?; + let (diff, _) = calc.try_calc_diff_internal( + &oplog, + &VersionVector::default(), + &Frontiers::default(), + &component_vv, + &component_frontiers, + None, + )?; + (diff, DiffMode::Import) + } else { + calc.try_calc_diff_internal( + &oplog, + &old_vv, + &old_frontiers, + oplog.vv(), + oplog.dag.get_frontiers(), + None, + )? + }; + oplog.check_history_parsable()?; + Ok((diff, diff_mode)) + })(); + let (diff, diff_mode) = match computed { + Ok(diff) => diff, + Err(err) => { + oplog.rollback_owned_import(rollback_enabled, &mut self.state.lock()); + return Err(err); + } }; let mut state = self.state.lock(); - let diff = recalc_in_checkout_mode_if_needed( + let diff = match recalc_in_checkout_mode_if_needed( &mut state, &oplog, &old_vv, &old_frontiers, diff, - ); + ) { + Ok(diff) => diff, + Err(err) => { + oplog.rollback_owned_import(rollback_enabled, &mut state); + return Err(err); + } + }; if let Err(e) = state.apply_diff( InternalDocDiff { origin, @@ -2206,7 +2237,11 @@ impl LoroDoc { a: &Frontiers, b: &Frontiers, ) -> Result, FrontiersNotIncluded> { - self.oplog().lock().cmp_frontiers(a, b) + let oplog = self.oplog.lock(); + oplog + .check_history_parsable() + .map_err(FrontiersNotIncluded::unreadable)?; + oplog.cmp_frontiers(a, b) } pub fn subscribe_root(&self, callback: Subscriber) -> Subscription { @@ -2535,7 +2570,7 @@ impl LoroDoc { } let frontiers = if to_shrink_frontiers { - shrink_frontiers(frontiers, &oplog.dag).map_err(LoroError::FrontiersNotFound)? + crate::version::try_shrink_frontiers(frontiers, &oplog.dag)? } else { frontiers.clone() }; @@ -2552,24 +2587,33 @@ impl LoroDoc { } } - let before = oplog.dag.frontiers_to_vv(&state.frontiers).ok_or_else(|| { - LoroError::NotFoundError( - format!( - "Cannot find the current state version {:?}", - state.frontiers + let before = oplog + .dag + .try_frontiers_to_vv(&state.frontiers)? + .ok_or_else(|| { + LoroError::NotFoundError( + format!( + "Cannot find the current state version {:?}", + state.frontiers + ) + .into_boxed_str(), ) - .into_boxed_str(), - ) - })?; - let Some(after) = &oplog.dag.frontiers_to_vv(&frontiers) else { + })?; + let Some(after) = &oplog.dag.try_frontiers_to_vv(&frontiers)? else { return Err(LoroError::NotFoundError( format!("Cannot find the specified version {:?}", frontiers).into_boxed_str(), )); }; + let (diff, diff_mode) = calc.try_calc_diff_internal( + &oplog, + &before, + &state.frontiers, + after, + &frontiers, + None, + )?; self.set_detached(true); - let (diff, diff_mode) = - calc.calc_diff_internal(&oplog, &before, &state.frontiers, after, &frontiers, None); state.apply_diff( InternalDocDiff { origin, @@ -2599,6 +2643,34 @@ impl LoroDoc { self.oplog.lock().dag.frontiers_to_vv(frontiers) } + /// Converts frontiers to a version vector, reporting unreadable history. + pub fn try_frontiers_to_vv(&self, frontiers: &Frontiers) -> LoroResult> { + let oplog = self.oplog.lock(); + oplog.check_history_parsable()?; + oplog.dag.try_frontiers_to_vv(frontiers) + } + + /// Converts a version vector to frontiers, reporting unreadable history. + pub fn try_vv_to_frontiers(&self, vv: &VersionVector) -> LoroResult { + let oplog = self.oplog.lock(); + oplog.check_history_parsable()?; + oplog.dag.try_vv_to_frontiers(vv) + } + + /// Removes redundant frontiers, reporting unreadable history or an absent id. + pub fn try_minimize_frontiers(&self, frontiers: &Frontiers) -> LoroResult { + let oplog = self.oplog.lock(); + oplog.check_history_parsable()?; + crate::version::try_shrink_frontiers(frontiers, &oplog.dag) + } + + /// Reads change metadata, distinguishing unreadable history from an absent id. + pub fn try_get_change(&self, id: ID) -> LoroResult> { + let oplog = self.oplog.lock(); + let change = oplog.try_get_change_at(id)?; + Ok(change.as_deref().map(ChangeMeta::from_change)) + } + /// Import ops from other doc. /// /// After `a.merge(b)` and `b.merge(a)`, `a` and `b` will have the same content if they are in attached mode. @@ -3020,6 +3092,22 @@ impl LoroDoc { self.oplog().lock().dag.find_path(from, to) } + /// Finds operation spans between versions, reporting unreadable history. + pub fn try_find_id_spans_between( + &self, + from: &Frontiers, + to: &Frontiers, + ) -> LoroResult { + let oplog = self.oplog.lock(); + oplog.check_history_parsable()?; + for id in from.iter().chain(to.iter()) { + if !oplog.vv().includes_id(id) { + return Err(LoroError::FrontiersNotFound(id)); + } + } + oplog.dag.try_find_path(from, to) + } + /// Subscribe to the first commit from a peer. Operations performed on the `LoroDoc` within this callback /// will be merged into the current commit. /// @@ -3181,6 +3269,8 @@ fn isolated_scalar_root_batch( #[derive(Debug, thiserror::Error)] pub enum ChangeTravelError { + #[error(transparent)] + HistoryUnreadable(#[from] LoroError), #[error("Target id not found {0:?}")] TargetIdNotFound(ID), #[error("The shallow history of the doc doesn't include the target version")] @@ -3218,43 +3308,49 @@ impl LoroDoc { } } - for id in ids { - let op_log = &self.oplog().lock(); - if !op_log.vv().includes_id(*id) { - return Err(ChangeTravelError::TargetIdNotFound(*id)); - } - if op_log.dag.shallow_since_vv().includes_id(*id) { - return Err(ChangeTravelError::TargetVersionNotIncluded); + let ans = (|| { + for id in ids { + let op_log = &self.oplog().lock(); + op_log.check_history_parsable()?; + if !op_log.vv().includes_id(*id) { + return Err(ChangeTravelError::TargetIdNotFound(*id)); + } + if op_log.dag.shallow_since_vv().includes_id(*id) { + return Err(ChangeTravelError::TargetVersionNotIncluded); + } } - } - let mut visited = FxHashSet::default(); - let mut pending: BinaryHeap = BinaryHeap::new(); - for id in ids { - pending.push(PendingNode(ChangeMeta::from_change( - &self.oplog().lock().get_change_at(*id).unwrap(), - ))); - } - while let Some(PendingNode(node)) = pending.pop() { - let deps = node.deps.clone(); - if f(node).is_break() { - break; + let mut visited = FxHashSet::default(); + let mut pending: BinaryHeap = BinaryHeap::new(); + for id in ids { + let change = self + .oplog() + .lock() + .try_get_change_at(*id)? + .ok_or(ChangeTravelError::TargetIdNotFound(*id))?; + pending.push(PendingNode(ChangeMeta::from_change(&change))); } - - for dep in deps.iter() { - let Some(dep_node) = self.oplog().lock().get_change_at(dep) else { - continue; - }; - if visited.contains(&dep_node.id) { - continue; + while let Some(PendingNode(node)) = pending.pop() { + let deps = node.deps.clone(); + if f(node).is_break() { + break; } - visited.insert(dep_node.id); - pending.push(PendingNode(ChangeMeta::from_change(&dep_node))); + for dep in deps.iter() { + let Some(dep_node) = self.oplog().lock().try_get_change_at(dep)? else { + continue; + }; + if visited.contains(&dep_node.id) { + continue; + } + + visited.insert(dep_node.id); + pending.push(PendingNode(ChangeMeta::from_change(&dep_node))); + } } - } - let ans = Ok(()); + Ok(()) + })(); self.renew_txn_if_auto_commit(options); ans } @@ -3287,6 +3383,18 @@ impl LoroDoc { }) } + /// Gets modified container IDs, reporting unreadable history. + pub fn try_get_changed_containers_in( + &self, + id: ID, + len: usize, + ) -> LoroResult> { + self.oplog.lock().check_history_parsable()?; + let containers = self.get_changed_containers_in(id, len); + self.oplog.lock().check_history_parsable()?; + Ok(containers) + } + pub fn delete_root_container(&self, cid: ContainerID) { if !cid.is_root() { return; @@ -3397,21 +3505,21 @@ fn recalc_in_checkout_mode_if_needed( old_vv: &VersionVector, old_frontiers: &Frontiers, diff: Vec, -) -> Vec { +) -> LoroResult> { if !state.needs_checkout_diff(&diff) { - return diff; + return Ok(diff); } - DiffCalculator::new(true) - .calc_diff_internal( + Ok(DiffCalculator::new(true) + .try_calc_diff_internal( oplog, old_vv, old_frontiers, oplog.vv(), oplog.dag.get_frontiers(), None, - ) - .0 + )? + .0) } impl BatchImportGuard<'_> { diff --git a/crates/loro-internal/src/oplog.rs b/crates/loro-internal/src/oplog.rs index 1db21511a..ba7ecdd75 100644 --- a/crates/loro-internal/src/oplog.rs +++ b/crates/loro-internal/src/oplog.rs @@ -255,6 +255,17 @@ impl OpLog { Some(ans) } + pub fn try_get_change_with_lamport_lte( + &self, + peer: PeerID, + lamport: Lamport, + ) -> Result, LoroError> { + self.check_history_parsable()?; + let change = self.get_change_with_lamport_lte(peer, lamport); + self.check_history_parsable()?; + Ok(change) + } + pub fn get_timestamp_of_version(&self, f: &Frontiers) -> Timestamp { let mut timestamp = Timestamp::default(); for id in f.iter() { @@ -373,14 +384,17 @@ impl OpLog { } } - pub(crate) fn preflight_import_changes(&self, changes: &[Change]) -> ImportChangesPreflight { + pub(crate) fn preflight_import_changes( + &self, + changes: &[Change], + ) -> Result { let mut ans = ImportChangesPreflight::default(); for change in changes { if change.ctr_end() <= self.vv().get(&change.id.peer).copied().unwrap_or(0) { continue; } - if self.dag.import_deps_before_shallow_root(&change.deps) { + if self.dag.try_import_deps_before_shallow_root(&change.deps)? { ans.has_deps_before_shallow_root = true; continue; } @@ -397,7 +411,7 @@ impl OpLog { if self .dag - .get_change_lamport_from_deps(&change.deps) + .try_get_change_lamport_from_deps(&change.deps)? .is_some() { ans.applies_to_dag = true; @@ -428,7 +442,7 @@ impl OpLog { ans.needs_state_apply_rollback = true; } - ans + Ok(ans) } /// Undo the open import rollback scope, including the arena. @@ -675,6 +689,13 @@ impl OpLog { self.change_store.get_change(id) } + pub fn try_get_change_at(&self, id: ID) -> Result, LoroError> { + self.check_history_parsable()?; + let change = self.get_change_at(id); + self.check_history_parsable()?; + Ok(change) + } + pub(crate) fn set_uncommitted_change(&mut self, change: Change) { self.uncommitted_change = Some(change); } @@ -709,6 +730,14 @@ impl OpLog { Some(convert_change_to_remote(&self.arena, &change)) } + pub fn try_get_remote_change_at( + &self, + id: ID, + ) -> Result>>, LoroError> { + let change = self.try_get_change_at(id)?; + Ok(change.map(|change| convert_change_to_remote(&self.arena, &change))) + } + pub(crate) fn import_unknown_lamport_pending_changes( &mut self, remote_changes: Vec, @@ -752,12 +781,18 @@ impl OpLog { self.change_store.export_blocks_in_range(spans, w) } - pub(crate) fn fork_changes_up_to(&self, frontiers: &Frontiers) -> Option { - let vv = self.dag.frontiers_to_vv(frontiers)?; - Some( - self.change_store - .fork_changes_up_to(self.dag.shallow_since_vv(), frontiers, &vv), - ) + pub(crate) fn fork_changes_up_to( + &self, + frontiers: &Frontiers, + ) -> Result, LoroError> { + let Some(vv) = self.dag.try_frontiers_to_vv(frontiers)? else { + return Ok(None); + }; + Ok(Some(self.change_store.fork_changes_up_to( + self.dag.shallow_since_vv(), + frontiers, + &vv, + )?)) } #[inline(always)] @@ -769,16 +804,16 @@ impl OpLog { /// /// Only `op.container` is needed, so this scans the borrowed changes /// directly instead of materializing a cloned RichOp per op. - pub(crate) fn containers_in_spans( + pub(crate) fn try_containers_in_spans( &self, spans: impl Iterator, - ) -> FxHashSet { + ) -> Result, LoroError> { let mut containers = FxHashSet::default(); // Consecutive ops overwhelmingly share a container; skip the hash // probe when it hasn't changed. let mut last_container = None; for span in spans { - for change in self.change_store.iter_changes(span) { + for change in self.change_store.try_iter_changes(span)? { let start_counter = span.counter.min().max(change.id.counter); let end_counter = span.counter.norm_end(); let start = change @@ -798,7 +833,7 @@ impl OpLog { } } - containers + Ok(containers) } /// For `to ⊇ from`: the old-parent frontiers of every entry change of the @@ -823,11 +858,11 @@ impl OpLog { from: &VersionVector, from_frontiers: &Frontiers, to: &VersionVector, - ) -> Option> { + ) -> Result>, LoroError> { let mut uncovered = Vec::new(); for (peer, span) in from.diff(to).forward.iter() { let id_span = IdSpan::new(*peer, span.start, span.end); - for change in self.change_store.iter_changes(id_span) { + for change in self.change_store.try_iter_changes(id_span)? { let start = change.id.counter.max(span.start); let parents = if start > change.id.counter { // The change straddles `from`; the new suffix only has @@ -856,14 +891,16 @@ impl OpLog { continue; } - let parents_vv = self.dag.frontiers_to_vv(&parents)?; + let Some(parents_vv) = self.dag.try_frontiers_to_vv(&parents)? else { + return Ok(None); + }; if !parents_vv.includes_vv(from) { uncovered.push(parents); } } } - Some(uncovered) + Ok(Some(uncovered)) } /// Decide whether an import whose new region `to − from` is concurrent @@ -891,11 +928,13 @@ impl OpLog { from: &VersionVector, to: &VersionVector, entry_parents: &[Frontiers], - ) -> Option> { + ) -> Result>, LoroError> { // ⋂ Events(parents) as a version vector: per-peer minimum. let mut causal_past = from.clone(); for parents in entry_parents { - let parents_vv = self.dag.frontiers_to_vv(parents)?; + let Some(parents_vv) = self.dag.try_frontiers_to_vv(parents)? else { + return Ok(None); + }; causal_past.retain(|peer, end| { let bound = parents_vv.get(peer).copied().unwrap_or(0); *end = (*end).min(bound); @@ -909,25 +948,25 @@ impl OpLog { // that trimmed history we cannot see which containers it touched, so // the decision has to stay with the DAG. if !causal_past.includes_vv(&self.dag.shallow_since_vv().to_vv()) { - return None; + return Ok(None); } let concurrent_old = causal_past.diff(from).forward; - let old_containers = self.containers_in_spans( + let old_containers = self.try_containers_in_spans( concurrent_old .iter() .map(|(peer, span)| IdSpan::new(*peer, span.start, span.end)), - ); + )?; if old_containers.is_empty() { - return Some(old_containers); + return Ok(Some(old_containers)); } let new_region = from.diff(to).forward; - let new_containers = self.containers_in_spans( + let new_containers = self.try_containers_in_spans( new_region .iter() .map(|(peer, span)| IdSpan::new(*peer, span.start, span.end)), - ); + )?; let harmless = old_containers .iter() .filter(|idx| new_containers.contains(idx)) @@ -937,7 +976,7 @@ impl OpLog { ContainerType::Counter => true, _ => false, }); - harmless.then_some(old_containers) + Ok(harmless.then_some(old_containers)) } /// The latest critical version below `from ∩ to`: the greatest causally @@ -965,31 +1004,42 @@ impl OpLog { /// that moves its peer's cut into it. The scan is thus bounded by the /// replay it enables, which walks the same region and computes the same /// contexts, so it needs no budget of its own. + #[cfg(test)] pub(crate) fn latest_critical_version_below_meet( &self, from: &VersionVector, to: &VersionVector, merged: &VersionVector, ) -> CriticalVersionSearch { + self.try_latest_critical_version_below_meet(from, to, merged) + .unwrap() + } + + pub(crate) fn try_latest_critical_version_below_meet( + &self, + from: &VersionVector, + to: &VersionVector, + merged: &VersionVector, + ) -> Result { use CriticalVersionSearch::*; // An intersection of two causally closed sets is causally closed, so // `min(from, to)` is a version, and it is the meet of the two. let mut v = from.intersection(to); if v.is_empty() { - return NoneBelowMeet; + return Ok(NoneBelowMeet); } let mut pending: Vec = v.diff_iter(merged).1.collect(); while let Some(span) = pending.pop() { - for change in self.change_store.iter_changes(span) { + for change in self.change_store.try_iter_changes(span)? { // The context of the change's first op above `v`, as // `iter_from_replay_base_causally` computes it for the // replay: the recorded deps plus the same-peer prefix. - let Some(mut ctx) = self.dag.frontiers_to_vv(&change.deps) else { + let Some(mut ctx) = self.dag.try_frontiers_to_vv(&change.deps)? else { // The deps reach below trimmed history; nothing can be // proved from here. - return Unknown; + return Ok(Unknown); }; let cut = v.get(&change.id.peer).copied().unwrap_or(0); let first_above = change.id.counter.max(cut); @@ -999,14 +1049,14 @@ impl OpLog { let above = v.clone(); v.intersect_with(&ctx); if v.is_empty() { - return NoneBelowMeet; + return Ok(NoneBelowMeet); } pending.extend(v.diff_iter(&above).1); } } } - Found(v) + Ok(Found(v)) } /// Iterates causally over all changes between the replay base (the meet @@ -1047,6 +1097,35 @@ impl OpLog { ), > + '_, ) { + let (base, iter) = self + .try_iter_from_replay_base_causally(from, from_frontiers, to, to_frontiers) + .unwrap(); + (base, iter.map(Result::unwrap)) + } + + #[allow(clippy::type_complexity)] + pub(crate) fn try_iter_from_replay_base_causally( + &self, + from: &VersionVector, + from_frontiers: &Frontiers, + to: &VersionVector, + to_frontiers: &Frontiers, + ) -> Result< + ( + ReplayBase, + impl Iterator< + Item = Result< + ( + BlockChangeRef, + (Counter, Counter), + Rc>, + ), + LoroError, + >, + > + '_, + ), + LoroError, + > { let mut merged_vv = from.clone(); merged_vv.merge(to); loro_common::debug!("to_frontiers={:?} vv={:?}", &to_frontiers, to); @@ -1058,10 +1137,10 @@ impl OpLog { // to a critical version; with container knowledge we can often // prove the concurrency harmless and replay from `from` // without touching the DAG at all. - if let Some(entry_parents) = self.uncovered_entry_parents(from, from_frontiers, to) { + if let Some(entry_parents) = self.uncovered_entry_parents(from, from_frontiers, to)? { if !entry_parents.is_empty() { if let Some(containers) = - self.register_only_concurrency(from, to, &entry_parents) + self.register_only_concurrency(from, to, &entry_parents)? { concurrent_containers = Some(containers); register_only_base = Some(from_frontiers.clone()); @@ -1073,7 +1152,8 @@ impl OpLog { let (meet, mut diff_mode) = if let Some(base) = register_only_base { (MeetAsBase::Valid(base), DiffMode::ImportGreaterUpdates) } else { - self.dag.find_meet_and_mode(from_frontiers, to_frontiers) + self.dag + .try_find_meet_and_mode(from_frontiers, to_frontiers)? }; if diff_mode == DiffMode::Checkout && to > from { diff_mode = DiffMode::Import; @@ -1093,9 +1173,9 @@ impl OpLog { #[cfg(test)] CRITICAL_BASE_FALLBACK_COUNT.with(|c| c.set(c.get() + 1)); self.dag - .latest_single_head_critical_version(from_frontiers, to_frontiers) + .try_latest_single_head_critical_version(from_frontiers, to_frontiers) }; - match self.latest_critical_version_below_meet(from, to, &merged_vv) { + match self.try_latest_critical_version_below_meet(from, to, &merged_vv)? { CriticalVersionSearch::Found(v) => { // Only replay from the version whose criticality was // proved. A shallow doc cannot replay from below its @@ -1106,17 +1186,18 @@ impl OpLog { // against) — a version that fails to round-trip // through frontiers is not one. Both are left to the // descent, as before. - let f = self.dag.vv_to_frontiers(&v); + let f = self.dag.try_vv_to_frontiers(&v)?; let seed = self .dag - .frontiers_to_vv(self.dag.shallow_since_frontiers()) + .try_frontiers_to_vv(self.dag.shallow_since_frontiers())? .unwrap(); - if v.includes_vv(&seed) && self.dag.frontiers_to_vv(&f).as_ref() == Some(&v) + if v.includes_vv(&seed) + && self.dag.try_frontiers_to_vv(&f)?.as_ref() == Some(&v) { replay_base_is_critical = true; f } else { - descend() + descend()? } } CriticalVersionSearch::NoneBelowMeet => { @@ -1127,12 +1208,15 @@ impl OpLog { // walk. Frontiers::default() } - CriticalVersionSearch::Unknown => descend(), + CriticalVersionSearch::Unknown => descend()?, } } }; - let mut replay_base_vv = self.dag.frontiers_to_vv(&replay_base_frontiers).unwrap(); + let mut replay_base_vv = self + .dag + .try_frontiers_to_vv(&replay_base_frontiers)? + .unwrap(); replay_base_is_critical |= replay_base_vv.is_empty(); if !replay_base_vv.includes_vv(&shallow_since_vv) { // The replay base cannot point before shallow history because those @@ -1140,18 +1224,18 @@ impl OpLog { replay_base_frontiers = self.dag.shallow_since_frontiers().clone(); replay_base_vv = self .dag - .frontiers_to_vv(&replay_base_frontiers) + .try_frontiers_to_vv(&replay_base_frontiers)? .unwrap_or(shallow_since_vv); replay_base_is_critical = false; } // go from the replay base to merged_vv let diff = replay_base_vv.diff(&merged_vv).forward; - let mut iter = self.dag.iter_causal(replay_base_frontiers, diff); + let mut iter = crate::dag::DagCausalIter::try_new(&self.dag, replay_base_frontiers, diff)?; let mut node = iter.next(); let mut cur_cnt = 0; let vv = Rc::new(RefCell::new(VersionVector::default())); - ( + Ok(( ReplayBase { vv: replay_base_vv.clone(), diff_mode, @@ -1159,37 +1243,52 @@ impl OpLog { concurrent_containers, }, std::iter::from_fn(move || { - if let Some(inner) = &node { - let mut inner_vv = vv.borrow_mut(); - // FIXME: PERF: it looks slow for large vv, like 10000+ entries - inner_vv.clear(); - self.dag.ensure_vv_for(&inner.data); - inner_vv.extend_to_include_vv(inner.data.vv.get().unwrap().iter()); - let peer = inner.data.peer; - let cnt = inner - .data - .cnt - .max(cur_cnt) - .max(replay_base_vv.get(&peer).copied().unwrap_or(0)); - let dag_node_end = (inner.data.cnt + inner.data.len as Counter) - .min(merged_vv.get(&peer).copied().unwrap_or(0)); - let change = self.change_store.get_change(ID::new(peer, cnt)).unwrap(); - - if change.ctr_end() < dag_node_end { - cur_cnt = change.ctr_end(); - } else { - node = iter.next(); - cur_cnt = 0; - } + let result = (|| -> Result<_, LoroError> { + if let Some(inner) = &node { + let mut inner_vv = vv.borrow_mut(); + // FIXME: PERF: it looks slow for large vv, like 10000+ entries + inner_vv.clear(); + self.dag.try_ensure_vv_for(&inner.data)?; + inner_vv.extend_to_include_vv(inner.data.vv.get().unwrap().iter()); + let peer = inner.data.peer; + let cnt = inner + .data + .cnt + .max(cur_cnt) + .max(replay_base_vv.get(&peer).copied().unwrap_or(0)); + let dag_node_end = (inner.data.cnt + inner.data.len as Counter) + .min(merged_vv.get(&peer).copied().unwrap_or(0)); + let change = match self.change_store.get_change(ID::new(peer, cnt)) { + Some(change) => change, + None => { + self.check_history_parsable()?; + panic!("DAG node must have a change"); + } + }; + + if change.ctr_end() < dag_node_end { + cur_cnt = change.ctr_end(); + } else { + node = iter.next(); + cur_cnt = 0; + } - inner_vv.extend_to_include_end_id(change.id); + inner_vv.extend_to_include_end_id(change.id); - Some((change, (cnt, dag_node_end), vv.clone())) - } else { - None + Ok(Some((change, (cnt, dag_node_end), vv.clone()))) + } else { + Ok(None) + } + })(); + match result { + Ok(item) => item.map(Ok), + Err(err) => { + node = None; + Some(Err(err)) + } } }), - ) + )) } pub fn len_changes(&self) -> usize { @@ -1284,7 +1383,10 @@ impl OpLog { let elem = *elems .entry(r.elem_id) .or_insert_with(|| self.resolve_movable_list_elem(r.elem_id)); - if !self.is_visible_movable_list_elem(&r, elem) { + if elem.is_none() { + self.check_history_parsable()?; + } + if !self.is_visible_movable_list_elem(&r, elem)? { return Err(LoroError::DecodeError( format!( "Movable list op {} targets element {}, which is not in the list's \ @@ -1326,31 +1428,33 @@ impl OpLog { &self, r: &MovableListElemRef, elem: Option, - ) -> bool { + ) -> Result { if r.elem_id.lamport >= r.op_lamport { - return false; + return Ok(false); } let Some(elem) = elem else { // History before a shallow root is trimmed. An op after the root can // only see pre-root elements that are still alive at the root. - return !self.dag.shallow_since_vv().is_empty() + return Ok(!self.dag.shallow_since_vv().is_empty() && self.with_history_cache(|h| { h.shallow_root_has_movable_list_elem(r.container, r.elem_id) - }); + })); }; if !elem.is_insert || elem.container != r.container { - return false; + return Ok(false); } // Earlier ops of the op's own DAG node are by the same peer, so the node's // start version covers every other peer's part of its causal history. let target = elem.target; - (target.peer == r.op_id.peer && target.counter < r.op_id.counter) - || self - .dag - .get(r.op_id) - .is_some_and(|node| self.dag.ensure_vv_for(&node).includes_id(target)) + if target.peer == r.op_id.peer && target.counter < r.op_id.counter { + return Ok(true); + } + let Some(node) = self.dag.try_get(r.op_id)? else { + return Ok(false); + }; + Ok(self.dag.try_ensure_vv_for(&node)?.includes_id(target)) } #[allow(unused)] diff --git a/crates/loro-internal/src/oplog/change_store.rs b/crates/loro-internal/src/oplog/change_store.rs index 08c7b28b9..b619f76eb 100644 --- a/crates/loro-internal/src/oplog/change_store.rs +++ b/crates/loro-internal/src/oplog/change_store.rs @@ -15,7 +15,6 @@ use crate::{ }; use block_encode::decode_block_range; use bytes::Bytes; -use itertools::Itertools; use loro_common::{ ContainerID, Counter, HasCounterSpan, HasId, HasIdSpan, HasLamportSpan, IdLp, IdSpan, Lamport, LoroError, LoroResult, PeerID, ID, @@ -370,7 +369,10 @@ impl ChangeStore { for span in layer { // PERF: this can be optimized by reusing the current encoded blocks // In the current method, it needs to parse and re-encode the blocks - for c in self.iter_changes(span) { + let Ok(changes) = self.try_iter_changes(span) else { + return; + }; + for c in changes { let start = ((span.counter.start - c.id.counter).max(0) as usize).min(c.atom_len()); let end = ((span.counter.end - c.id.counter).max(0) as usize).min(c.atom_len()); @@ -710,76 +712,63 @@ impl ChangeStore { } pub(crate) fn iter_blocks(&self, id_span: IdSpan) -> Vec<(Arc, usize, usize)> { + self.try_iter_blocks(id_span).unwrap_or_default() + } + + fn try_iter_blocks( + &self, + id_span: IdSpan, + ) -> LoroResult, usize, usize)>> { if id_span.counter.start == id_span.counter.end { - return vec![]; + return Ok(vec![]); } - assert!(id_span.counter.start < id_span.counter.end); - self.ensure_block_loaded_in_range( + self.try_ensure_block_loaded_in_range( Bound::Included(id_span.id_start()), Bound::Excluded(id_span.id_end()), - ); + )?; let mut inner = self.inner.lock(); - let next_back = inner.mem_parsed_kv.range(..=id_span.id_start()).next_back(); - match next_back { - None => { - return vec![]; - } - Some(next_back) => { - if next_back.0.peer != id_span.peer { - return vec![]; - } + let Some((start_id, _)) = inner.mem_parsed_kv.range(..=id_span.id_start()).next_back() + else { + return Ok(vec![]); + }; + if start_id.peer != id_span.peer { + return Ok(vec![]); + } + let start_counter = start_id.counter; + let mut blocks = Vec::new(); + for (block_id, block) in inner.mem_parsed_kv.range_mut( + ID::new(id_span.peer, start_counter)..ID::new(id_span.peer, id_span.counter.end), + ) { + if block.counter_range.1 < id_span.counter.start { + continue; + } + if let Err(err) = block.ensure_changes(&self.arena) { + warn!(?block_id, ?err, "failed to parse change block"); + self.parse_failures.record(*block_id, &err); + return Err(self.corrupt_block_error().unwrap_err()); + } + let changes = block.content.try_changes().unwrap(); + let (start, end) = if id_span.counter.start <= block.counter_range.0 + && id_span.counter.end >= block.counter_range.1 + { + (0, changes.len()) + } else { + let start = block + .get_change_index_by_counter(id_span.counter.start) + .unwrap_or_else(|x| x); + let end = match block.get_change_index_by_counter(id_span.counter.end - 1) { + Ok(end) => end + 1, + Err(0) => continue, + Err(end) => end, + }; + (start, end) + }; + if start != end { + blocks.push((block.clone(), start, end)); } } - let start_counter = next_back.map(|(id, _)| id.counter).unwrap_or(0); - let ans = inner - .mem_parsed_kv - .range_mut( - ID::new(id_span.peer, start_counter)..ID::new(id_span.peer, id_span.counter.end), - ) - .filter_map(|(_id, block)| { - if block.counter_range.1 < id_span.counter.start { - return None; - } - - if let Err(err) = block.ensure_changes(&self.arena) { - warn!(block_id = ?_id, ?err, "failed to parse change block"); - self.parse_failures.record(*_id, &err); - return None; - } - let changes = block.content.try_changes().unwrap(); - let start; - let end; - if id_span.counter.start <= block.counter_range.0 - && id_span.counter.end >= block.counter_range.1 - { - start = 0; - end = changes.len(); - } else { - start = block - .get_change_index_by_counter(id_span.counter.start) - .unwrap_or_else(|x| x); - - match block.get_change_index_by_counter(id_span.counter.end - 1) { - Ok(e) => { - end = e + 1; - } - Err(0) => return None, - Err(e) => { - end = e; - } - } - } - if start == end { - return None; - } - - Some((block.clone(), start, end)) - }) - // TODO: PERF avoid alloc - .collect_vec(); - - ans + Ok(blocks) } pub fn iter_changes(&self, id_span: IdSpan) -> impl Iterator + '_ { @@ -813,6 +802,21 @@ impl ChangeStore { }) } + /// Parse the range before handing any changes to a consumer: a missing middle + /// block must not become a counter gap in an export's scratch store. + pub(crate) fn try_iter_changes( + &self, + id_span: IdSpan, + ) -> LoroResult> { + let blocks = self.try_iter_blocks(id_span)?; + Ok(blocks.into_iter().flat_map(|(block, start, end)| { + (start..end).map(move |change_index| BlockChangeRef { + change_index, + block: block.clone(), + }) + })) + } + #[allow(dead_code)] pub(crate) fn get_blocks_in_range(&self, id_span: IdSpan) -> VecDeque> { let mut inner = self.inner.lock(); @@ -939,7 +943,10 @@ impl ChangeStore { // PERF: this can be optimized by reusing the current encoded blocks // In the current method, it needs to parse and re-encode the blocks - for c in self.iter_changes(span) { + let Ok(changes) = self.try_iter_changes(span) else { + return; + }; + for c in changes { let start = ((start_vv.get(&c.id.peer).copied().unwrap_or(0) - c.id.counter).max(0) as usize) .min(c.atom_len()); @@ -962,7 +969,7 @@ impl ChangeStore { start_vv: &ImVersionVector, frontiers: &Frontiers, vv: &VersionVector, - ) -> Bytes { + ) -> LoroResult { let new_store = ChangeStore::new_mem(&self.arena, self.merge_interval.clone()); for mut span in vv.sub_iter_im(start_vv) { let counter_lower_bound = start_vv.get(&span.peer).copied().unwrap_or(0); @@ -974,7 +981,7 @@ impl ChangeStore { // PERF: this can be optimized by reusing the current encoded blocks // In the current method, it needs to parse and re-encode the blocks - for c in self.iter_changes(span) { + for c in self.try_iter_changes(span)? { let start = ((start_vv.get(&c.id.peer).copied().unwrap_or(0) - c.id.counter).max(0) as usize) .min(c.atom_len()); @@ -988,7 +995,7 @@ impl ChangeStore { } } - new_store.encode_all(vv, frontiers) + Ok(new_store.encode_all(vv, frontiers)) } } @@ -1695,6 +1702,23 @@ mod mut_inner_kv { /// This is fast because we don't actually parse the content. // TODO: PERF: This method feels slow. pub(super) fn ensure_block_loaded_in_range(&self, start: Bound, end: Bound) { + let _ = self.load_blocks_in_range(start, end, false); + } + + pub(super) fn try_ensure_block_loaded_in_range( + &self, + start: Bound, + end: Bound, + ) -> LoroResult<()> { + self.load_blocks_in_range(start, end, true) + } + + fn load_blocks_in_range( + &self, + start: Bound, + end: Bound, + stop_on_error: bool, + ) -> LoroResult<()> { let mut whether_need_scan_backward = match start { Bound::Included(id) => Some(id), Bound::Excluded(id) => Some(id.inc(1)), @@ -1729,6 +1753,9 @@ mod mut_inner_kv { Err(err) => { warn!(?id, ?err, "failed to decode external change block"); self.parse_failures.record(id, &err); + if stop_on_error { + return Err(self.corrupt_block_error().unwrap_err()); + } continue; } }; @@ -1737,24 +1764,33 @@ mod mut_inner_kv { } if let Some(start_id) = whether_need_scan_backward { - self.ensure_id_lte(start_id); + if stop_on_error { + self.try_ensure_id_lte(start_id)?; + } else { + self.ensure_id_lte(start_id); + } } + Ok(()) } pub(super) fn ensure_id_lte(&self, id: ID) { + let _ = self.try_ensure_id_lte(id); + } + + pub(super) fn try_ensure_id_lte(&self, id: ID) -> LoroResult<()> { let kv = self.external_kv.lock(); let mut inner = self.inner.lock(); let Some((next_back_id, next_back_bytes)) = kv .scan(Bound::Unbounded, Bound::Included(&id.to_bytes())) .rfind(|(id, _)| id.len() == 12) else { - return; + return Ok(()); }; let next_back_id = ID::from_bytes(&next_back_id); if next_back_id.peer == id.peer { if inner.mem_parsed_kv.contains_key(&next_back_id) { - return; + return Ok(()); } let block = match ChangesBlock::from_bytes(next_back_bytes) { @@ -1766,11 +1802,12 @@ mod mut_inner_kv { "failed to decode external change block" ); self.parse_failures.record(next_back_id, &err); - return; + return Err(self.corrupt_block_error().unwrap_err()); } }; inner.mem_parsed_kv.insert(next_back_id, Arc::new(block)); } + Ok(()) } } } @@ -2523,6 +2560,69 @@ mod test { assert_eq!(doc.get_deep_value(), value); } + #[test] + fn the_first_dag_reader_reports_unparsable_tree_history_without_changing_state() { + let (snapshot, _, peer) = snapshot_with_a_container_only_the_history_knows(); + for api in ["checkout", "diff", "revert", "frontiers", "travel", "cmp"] { + let doc = LoroDoc::new(); + doc.import(&snapshot).unwrap(); + truncate_block(&doc.oplog().lock().change_store, ID::new(peer, 0)); + assert!(doc.oplog().lock().check_history_parsable().is_ok()); + let value = doc.get_deep_value(); + let state_frontiers = doc.state_frontiers(); + let mid = Frontiers::from(ID::new(peer, 10)); + let result = match api { + "checkout" => doc.checkout(&mid).map_err(|err| err.to_string()), + "diff" => doc + .diff(&mid, &state_frontiers) + .map(|_| ()) + .map_err(|err| err.to_string()), + "revert" => doc.revert_to(&mid).map_err(|err| err.to_string()), + "frontiers" => doc + .try_frontiers_to_vv(&mid) + .map(|_| ()) + .map_err(|err| err.to_string()), + "travel" => doc + .travel_change_ancestors(&[ID::new(peer, 10)], &mut |_| { + std::ops::ControlFlow::Continue(()) + }) + .map_err(|err| err.to_string()), + "cmp" => doc + .cmp_frontiers(&mid, &state_frontiers) + .map(|_| ()) + .map_err(|err| err.to_string()), + _ => unreachable!(), + }; + assert!( + result.unwrap_err().contains("cannot parse change block"), + "{api}" + ); + assert_eq!(doc.get_deep_value(), value, "{api}"); + assert_eq!(doc.state_frontiers(), state_frontiers, "{api}"); + assert!(matches!( + doc.oplog().lock().check_history_parsable(), + Err(LoroError::DecodeError(_)) + )); + } + } + + #[test] + fn a_detached_fork_records_unparsable_history_before_its_infallible_boundary_panics() { + let (snapshot, _, peer) = snapshot_with_a_container_only_the_history_knows(); + let doc = LoroDoc::new(); + doc.import(&snapshot).unwrap(); + truncate_block(&doc.oplog().lock().change_store, ID::new(peer, 0)); + let value = doc.get_deep_value(); + doc.detach(); + assert!(std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| doc.fork())).is_err()); + assert_eq!(doc.get_deep_value(), value); + assert!(doc + .checkout(&doc.oplog_frontiers()) + .unwrap_err() + .to_string() + .contains("cannot parse change block")); + } + #[test] fn a_recorded_parse_failure_does_not_panic_where_no_error_can_be_returned() { // `undo`, `checkout_to_latest` and a detached `fork` run the same checkout and export diff --git a/crates/loro-internal/src/oplog/loro_dag.rs b/crates/loro-internal/src/oplog/loro_dag.rs index 4c7684d49..451728af3 100644 --- a/crates/loro-internal/src/oplog/loro_dag.rs +++ b/crates/loro-internal/src/oplog/loro_dag.rs @@ -4,7 +4,9 @@ use crate::id::{Counter, ID}; use crate::span::{HasId, HasLamport}; use crate::sync::Mutex; use crate::version::{shrink_frontiers, Frontiers, ImVersionVector, VersionVector}; -use loro_common::{HasCounter, HasCounterSpan, HasIdSpan, HasLamportSpan, PeerID}; +use loro_common::{ + HasCounter, HasCounterSpan, HasIdSpan, HasLamportSpan, LoroError, LoroResult, PeerID, +}; use once_cell::sync::OnceCell; use rle::{HasIndex, HasLength, Mergable, Sliceable}; use rustc_hash::FxHashSet; @@ -596,9 +598,9 @@ impl AppDag { } } - fn ensure_lazy_load_node(&self, id: ID) { + fn ensure_lazy_load_node(&self, id: ID) -> LoroResult<()> { if self.shallow_since_vv.includes_id(id) { - return; + return Ok(()); } loop { @@ -609,7 +611,7 @@ impl AppDag { unparsed_vv.get(&id.peer).copied().unwrap_or(0) }; if unparsed_end <= id.counter { - return; + return Ok(()); } let last_unparsed_id = ID::new(id.peer, unparsed_end - 1); @@ -617,6 +619,10 @@ impl AppDag { .change_store .get_dag_nodes_that_contains(last_unparsed_id) else { + // An unreadable external block is recorded by ChangeStore; return its + // decode error. A parsable store missing an id promised by unparsed_vv + // is still an impossible internal inconsistency and must fail fast. + self.change_store.corrupt_block_error()?; panic!("unparsed vv don't match with change store. Id:{id} is not in change store") }; @@ -624,11 +630,18 @@ impl AppDag { } } + pub(crate) fn has_unparsed_history(&self) -> bool { + !self.unparsed_vv.lock().is_empty() + } + pub fn total_parsed_dag_node(&self) -> usize { self.map.lock().len() } - pub(crate) fn set_version_by_fast_snapshot_import(&mut self, v: BatchDecodeInfo) { + pub(crate) fn set_version_by_fast_snapshot_import( + &mut self, + v: BatchDecodeInfo, + ) -> LoroResult<()> { assert!(self.vv.is_empty()); *self.unparsed_vv.lock() = v.vv.clone(); self.vv = v.vv; @@ -637,13 +650,14 @@ impl AppDag { if !f.is_empty() { assert!(f.len() == 1); let id = f.as_single().unwrap(); - let node = self.get(id).unwrap(); + let node = self.try_get(id)?.unwrap(); assert!(node.cnt == id.counter); self.shallow_root_frontiers_deps = node.deps.clone(); } self.shallow_since_frontiers = f; self.shallow_since_vv = ImVersionVector::from_vv(&vv); } + Ok(()) } /// This method is slow and should only be used for debugging and testing. @@ -669,7 +683,7 @@ impl AppDag { let init_counter = self.shallow_since_vv.get(peer).copied().unwrap_or(0); while end_cnt > init_counter { let cnt = end_cnt - 1; - self.ensure_lazy_load_node(ID::new(*peer, cnt)); + self.ensure_lazy_load_node(ID::new(*peer, cnt)).unwrap(); end_cnt = self.unparsed_vv.lock().get(peer).copied().unwrap_or(0); } } @@ -794,12 +808,16 @@ impl AppDag { } pub(crate) fn import_deps_before_shallow_root(&self, deps: &Frontiers) -> bool { + self.try_import_deps_before_shallow_root(deps).unwrap() + } + + pub(crate) fn try_import_deps_before_shallow_root(&self, deps: &Frontiers) -> LoroResult { if self.shallow_since_vv.is_empty() { - return false; + return Ok(false); } if deps.is_empty() { - return true; + return Ok(true); } // Deps equal to the root's own deps describe a change CONCURRENT with @@ -810,12 +828,12 @@ impl AppDag { // be parked as pending and then panic in `calc_unknown_lamport_change`. // Reject it like any other pre-root update. if deps == &self.shallow_root_frontiers_deps { - return true; + return Ok(true); } let shallow_vv = VersionVector::from_im_vv(&self.shallow_since_vv); - if let Some(vv) = self.frontiers_to_vv(deps) { - return !vv.includes_vv(&shallow_vv); + if let Some(vv) = self.try_frontiers_to_vv(deps)? { + return Ok(!vv.includes_vv(&shallow_vv)); } // Import only needs to reject updates whose causal source is older than @@ -826,20 +844,20 @@ impl AppDag { .iter() .any(|id| self.shallow_since_frontiers.contains(&id)) { - return false; + return Ok(false); } - deps.iter().any(|id| self.shallow_since_vv.includes_id(id)) + Ok(deps.iter().any(|id| self.shallow_since_vv.includes_id(id))) } /// Travel the ancestors of the given id, and call the callback for each node /// /// It will travel the ancestors in the reverse order (from the greatest lamport to the smallest) - pub(crate) fn travel_ancestors( + pub(crate) fn try_travel_ancestors( &self, id: ID, f: &mut dyn FnMut(&AppDagNode) -> ControlFlow<()>, - ) { + ) -> LoroResult<()> { struct PendingNode(AppDagNode); impl PartialEq for PendingNode { fn eq(&self, other: &Self) -> bool { @@ -863,14 +881,16 @@ impl AppDag { let mut visited = FxHashSet::default(); let mut pending: BinaryHeap = BinaryHeap::new(); - pending.push(PendingNode(self.get(id).unwrap())); + pending.push(PendingNode( + self.try_get(id)?.ok_or(LoroError::FrontiersNotFound(id))?, + )); while let Some(PendingNode(node)) = pending.pop() { if f(&node).is_break() { break; } for dep in node.deps.iter() { - let Some(dep_node) = self.get(dep) else { + let Some(dep_node) = self.try_get(dep)? else { continue; }; if visited.contains(&dep_node.id_start()) { @@ -881,6 +901,7 @@ impl AppDag { pending.push(PendingNode(dep_node)); } } + Ok(()) } pub(crate) fn update_version_on_new_local_op( @@ -1066,24 +1087,28 @@ impl Dag for AppDag { } fn get(&self, id: ID) -> Option { - self.ensure_lazy_load_node(id); + self.try_get(id).unwrap() + } + + fn try_get(&self, id: ID) -> LoroResult> { + self.ensure_lazy_load_node(id)?; let binding = self.map.lock(); if let Some(x) = binding.range(..=id).next_back() { if x.1.contains_id(id) { // PERF: do we need to optimize clone like this? // by adding another layer of Arc? - return Some(x.1.clone()); + return Ok(Some(x.1.clone())); } } if let Some(node) = &self.pending_txn_node { if node.peer == id.peer && node.cnt <= id.counter { assert!(node.cnt + node.len as Counter > id.counter); - return Some(node.clone()); + return Ok(Some(node.clone())); } } - None + Ok(None) } fn vv(&self) -> &VersionVector { @@ -1108,6 +1133,13 @@ impl AppDag { } pub(crate) fn ensure_vv_for(&self, target_node: &AppDagNode) -> ImVersionVector { + self.try_ensure_vv_for(target_node).unwrap() + } + + pub(crate) fn try_ensure_vv_for( + &self, + target_node: &AppDagNode, + ) -> LoroResult { if target_node.vv.get().is_none() { // Iterative DFS. When the DAG contains a diamond, a dep can end // up on the stack multiple times (once for each ancestor that @@ -1129,7 +1161,7 @@ impl AppDag { } else { let mut all_deps_processed = true; for id in top_node.deps.iter() { - let Some(node) = self.get(id) else { + let Some(node) = self.try_get(id)? else { if self.shallow_since_vv.includes_id(id) { continue; } @@ -1151,7 +1183,7 @@ impl AppDag { } for id in top_node.deps.iter() { - let Some(node) = self.get(id) else { + let Some(node) = self.try_get(id)? else { if self.shallow_since_vv.includes_id(id) { ans_vv.extend_to_include_vv(self.shallow_since_vv.iter()); continue; @@ -1177,7 +1209,7 @@ impl AppDag { } } - target_node.vv.get().unwrap().clone() + Ok(target_node.vv.get().unwrap().clone()) } /// Compare the causal order of two versions. @@ -1193,44 +1225,63 @@ impl AppDag { } pub fn get_lamport(&self, id: &ID) -> Option { - self.get(*id).and_then(|node| { + self.try_get_lamport(id).unwrap() + } + + pub fn try_get_lamport(&self, id: &ID) -> LoroResult> { + Ok(self.try_get(*id)?.and_then(|node| { assert!(id.counter >= node.cnt); if node.cnt + node.len as Counter > id.counter { Some(node.lamport + (id.counter - node.cnt) as Lamport) } else { None } - }) + })) } pub fn get_change_lamport_from_deps(&self, deps: &Frontiers) -> Option { + self.try_get_change_lamport_from_deps(deps).unwrap() + } + + pub fn try_get_change_lamport_from_deps( + &self, + deps: &Frontiers, + ) -> LoroResult> { let mut lamport = 0; for id in deps.iter() { - let x = self.get_lamport(&id)?; + let Some(x) = self.try_get_lamport(&id)? else { + return Ok(None); + }; lamport = lamport.max(x + 1); } - Some(lamport) + Ok(Some(lamport)) } /// Convert a frontiers to a version vector /// /// If the frontiers version is not found in the dag, return None pub fn frontiers_to_vv(&self, frontiers: &Frontiers) -> Option { + self.try_frontiers_to_vv(frontiers).unwrap() + } + + pub fn try_frontiers_to_vv(&self, frontiers: &Frontiers) -> LoroResult> { if frontiers == &self.shallow_root_frontiers_deps { let vv = VersionVector::from_im_vv(&self.shallow_since_vv); - return Some(vv); + return Ok(Some(vv)); } let mut vv: VersionVector = Default::default(); for id in frontiers.iter() { - let x = self.get(id)?; - let target_vv = self.ensure_vv_for(&x); + let Some(x) = self.try_get(id)? else { + return Ok(None); + }; + let target_vv = self.try_ensure_vv_for(&x)?; vv.extend_to_include_vv(target_vv.iter()); vv.extend_to_include_last_id(id); } - Some(vv) + Ok(Some(vv)) } #[allow(unused)] @@ -1294,8 +1345,12 @@ impl AppDag { } pub fn vv_to_frontiers(&self, vv: &VersionVector) -> Frontiers { + self.try_vv_to_frontiers(vv).unwrap() + } + + pub fn try_vv_to_frontiers(&self, vv: &VersionVector) -> LoroResult { if vv.is_empty() { - return Default::default(); + return Ok(Default::default()); } let this = vv; @@ -1318,10 +1373,10 @@ impl AppDag { .collect(); if last_ids.is_empty() { - return self.shallow_since_frontiers.clone(); + return Ok(self.shallow_since_frontiers.clone()); } - shrink_frontiers(&last_ids, self).unwrap() + crate::version::try_shrink_frontiers(&last_ids, self) } pub(crate) fn frontiers_to_next_lamport(&self, frontiers: &Frontiers) -> Lamport { @@ -1376,17 +1431,44 @@ impl AppDag { a: &Frontiers, b: &Frontiers, ) -> Result, FrontiersNotIncluded> { - let a = self.frontiers_to_vv(a).ok_or(FrontiersNotIncluded)?; - let b = self.frontiers_to_vv(b).ok_or(FrontiersNotIncluded)?; + let a = self + .try_frontiers_to_vv(a) + .map_err(FrontiersNotIncluded::unreadable)? + .ok_or(FrontiersNotIncluded)?; + let b = self + .try_frontiers_to_vv(b) + .map_err(FrontiersNotIncluded::unreadable)? + .ok_or(FrontiersNotIncluded)?; Ok(a.partial_cmp(&b)) } } #[derive(Debug, PartialEq, Eq)] -pub struct FrontiersNotIncluded; +pub struct FrontiersNotIncluded { + history_error: Option>, +} + +// Retain the unit-like constructor used by existing callers and pattern matches. +#[allow(non_upper_case_globals)] +pub const FrontiersNotIncluded: FrontiersNotIncluded = FrontiersNotIncluded { + history_error: None, +}; + +impl FrontiersNotIncluded { + pub(crate) fn unreadable(error: LoroError) -> Self { + Self { + history_error: Some(error.to_string().into_boxed_str()), + } + } +} + impl Display for FrontiersNotIncluded { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("The given Frontiers are not included by the doc") + f.write_str( + self.history_error + .as_deref() + .unwrap_or("The given Frontiers are not included by the doc"), + ) } } @@ -1426,6 +1508,15 @@ mod ensure_vv_for_tests { dag } + #[test] + #[should_panic(expected = "unparsed vv don't match with change store")] + fn a_missing_node_without_a_parse_failure_remains_an_internal_invariant() { + let store = ChangeStore::new_mem(&SharedArena::new(), Arc::new(AtomicI64::new(0))); + let dag = AppDag::new(store); + dag.unparsed_vv.lock().insert(1, 1); + let _ = dag.try_get(ID::new(1, 0)); + } + /// Regression for loro-dev/loro#929: when computing the vv for a node /// whose DAG fan-in contains a shared ancestor reached through multiple /// paths, the iterative DFS used to push the shared ancestor onto the diff --git a/crates/loro-internal/src/version.rs b/crates/loro-internal/src/version.rs index 3efbe1498..3c3edb362 100644 --- a/crates/loro-internal/src/version.rs +++ b/crates/loro-internal/src/version.rs @@ -994,6 +994,16 @@ impl VersionVector { /// Use minimal set of ids to represent the frontiers #[tracing::instrument(skip(dag))] pub fn shrink_frontiers(last_ids: &Frontiers, dag: &AppDag) -> Result { + try_shrink_frontiers(last_ids, dag).map_err(|err| match err { + loro_common::LoroError::FrontiersNotFound(id) => id, + err => panic!("{err}"), + }) +} + +pub(crate) fn try_shrink_frontiers( + last_ids: &Frontiers, + dag: &AppDag, +) -> loro_common::LoroResult { // it only keep the ids of ops that are concurrent to each other if last_ids.len() <= 1 { @@ -1010,8 +1020,8 @@ pub fn shrink_frontiers(last_ids: &Frontiers, dag: &AppDag) -> Result Result Result) -> JsResult { let frontiers = ids_to_frontiers(frontiers)?; - let borrow_mut = &self.doc; - let oplog = borrow_mut.oplog().lock(); - oplog - .dag() - .frontiers_to_vv(&frontiers) + self.doc + .try_frontiers_to_vv(&frontiers)? .map(VersionVector) .ok_or_else(|| JsError::new("Frontiers not found").into()) } @@ -2155,7 +2153,7 @@ impl LoroDoc { /// ``` #[wasm_bindgen(js_name = "vvToFrontiers")] pub fn vv_to_frontiers(&self, vv: &VersionVector) -> JsResult { - let f = self.doc.oplog().lock().dag().vv_to_frontiers(&vv.0); + let f = self.doc.try_vv_to_frontiers(&vv.0)?; frontiers_to_ids(&f) } @@ -2266,7 +2264,7 @@ impl LoroDoc { let id = js_id_to_id(id)?; Ok(self .doc - .get_changed_containers_in(id, len) + .try_get_changed_containers_in(id, len)? .into_iter() .map(|cid| { let v: JsValue = (&cid).into(); diff --git a/crates/loro-wasm/tests/fixtures/unparsable-history.snapshot.bin b/crates/loro-wasm/tests/fixtures/unparsable-history.snapshot.bin new file mode 100644 index 0000000000000000000000000000000000000000..6588f0079d955d98c4801ddee705ff0e59e1a815 GIT binary patch literal 9532 zcmeI2dvI0N6~^~I_ueD~?gb&|fCh*rB&82}bDv43cC_QG?X)@(|L~7ahr-Zlr&Jtk zX_#@Q2`Uhw8UY0apG3u~U=W{xA|ggw0Rt#LTCHO10};YQtq=OGwLdhu1CnGCMxCC) z{aa`6v)A2s@B4k{oE;}kzGSlLaI`Nj+iF4=4KwCv=be9^377tS*tpZDZL!9zGmXXg zH{o(?t+A%0%$jpo*zp_Cgj3t{2NUwl8AGh``Ww$PhfYFEe=lr3gTk0_H1xZPQ^S`| zth+Sy`(Isbc7|=Z)y@hT4_Dl`JTyCG_Tus=C~jFK^OAUTLUfgJ2uGz1BglBkFcJ{gch$zkM)|(3Iy1qo@SLn| zxwMa#c*TgrOI%Z#0P>y_bgb#-;2r&P@I${OcU%d4Q} z)8}Lxi^E4#bMvI*pyotPwW>K2+F<2-LW4z5L)J+r>sHjMte-)kvfhY5W&Jk!lh2bckUPj1$(J1cmyu9K{R$iYLH?6` zm3)o-7x_B*1_}RC-x2##NyEORLflIU^HRdQl&~%(oJ&dL3SeA%PT%t3TTar)Fs@)f zJ02hpk{^(?E&n4VR7rQ!x610h#Rc)Cc5Prz`j!vha;om3aRo3g{nqp?AHL;O-9zIF zu4BUM9sT)8sH*OvaRo3g9U*U0fzb-ISXI$gtH zovz`qPS>>902ZrrNr&|xzy&Jn2ifot`7n7Di!EB+=EYOm6DF%E>9IaM)(N|7CR-3Z z#RMBjn3yJng9&{xFcmfoOdH@|O4yeY?xlozDRaClfOlygXIbnD3u9D=1k^G>G z<`uxaG$Xyshj%$;_t3oL*-I1ByL@<;lXVZxEBB$x-;0ELvu@`n?IXKrUIENYM@aAT zKSn~8^(Sojl>Cf5RJRz3FaV3 zNiY{dT7r29G7{vtYyg+lxunVZ3vr>!^9D97A{Uc4k~fib*`ixM^x|pl37^%Jv{`=% zQtBPMnGLs-caV3IOUb)P*qA1Qiz#7YLSGzAg$)PO1{jzU{-taoSCIFUD@nMQUPtrt zVO~zy$8fJ;H9O{bR{-zQgtRXI2_)3p?m9L+Nv%C)ZC$1eYEBve(u!%upb+(o{pbZXwMYHn&D zJ&|f&gNoXFXV}K#2%ff&cz=HBYVSDQpVKuN%K3efb5=x%2ScZ8aaYwf+|}uNA9S5@ zy55gE)%5`cs_TOYQj*{U1ZfFAvVl&n=QzFL=+d(1TZlrJWL4#6Z+y`s%!X{Hq0gGk;gEv0OqCr=v_X% z%jx+kYE=@@1@*hD$)%BxnSVeZxx&m02 zCZu!ua4x6o9$HtB^VH=(i-fA{P5h+It^SHF=cX?ya8ymyViT*7L#X9#0>bdEzYb5P65obhgFi|I9xTq7b*r*B^HmU+1_X0-lw(-jI)`6g~gS~Fo^p8b=xyRlzu?bWSlYijlKmetSIcL(2^{Qf5FnSK5CSbRU( zyEPHrvPFt;2$f;8JOYk1Ge?Y^YE9w55_722F@D_CsfUMEI;&rb!$O^D$FQ1)FIX!B zTWpaVo5(Vz*G(NVqtku$A&g~~jNnWe%K#ZwFOB6FwQI+5ESJj^-BLX>w)7P{Gj{5Q zpO55QZRXTE8OWB0L*BCbbIn*2wI@6^LIr9;-VXw}F%*_$6ZlqZs%bAT6_ncrn z^Xp5jw`n$Z#Lbi`p0{PTmFYHSd;Y8WdKWQ`zZ|=;5T6*qfAZ7PL)X-HK79+79c!YE z4J*;9V^#6shH3peSB+lOFgDt`Dm$v-loL8vopEo&-lEP`wb@ReUz=U-cep@bP`|i9 zPtOiIUl1GA>4Mmxwww!wpNx5*GUcrCn>YSus4@58tLR@G@T;)$mx9o;e)y3do|75P z2(4*)|HT@v;yLK>XV01)Z(95iJN*Wad{Xte`!hm@d(7cd2&&Y?N~)yDN|BKwEk#O- zq!bA$DoahYr1}CI>t0bMJ-St7rAN1ljP&SMk(M6aDpJy;TSZcObgM{6k8Tx}(xV$i zv?TrGXe^$txxhxdS;hvUr*{9Mh^OTTafF61F6rYap3XGqxo_3SpY%;gpOPi)y&|5f zUL1?|V#5EY>ecme)0vb-|ArLUUw^G6`c3&ZefT{Rvgo~B#1qxu`*yUqvueJ0BD?Lo zeU&RXkC-Ye?5;rc&1Zt5L)&t6Hm~Uy;Fl z9fuLGtStIb7bdVE`S}~}p3x>0F4z$^*2B9)%+8x_pAY}QZhRW2XT{{z>v^3eWP!)YU$>-pOCKcCO21ONa4 literal 0 HcmV?d00001 diff --git a/crates/loro-wasm/tests/unparsable_history.test.ts b/crates/loro-wasm/tests/unparsable_history.test.ts new file mode 100644 index 000000000..c83360ab1 --- /dev/null +++ b/crates/loro-wasm/tests/unparsable_history.test.ts @@ -0,0 +1,70 @@ +import { readFileSync } from "node:fs"; +import { expect, it } from "vitest"; +import { LoroDoc, VersionVector, type Frontiers } from "../bundler/index"; + +// Generated by the public Rust regression with LORO_HISTORY_FIXTURE set to this +// absolute path. The first peer-1 block is halved and both checksums are repaired. +const snapshot = readFileSync( + new URL("./fixtures/unparsable-history.snapshot.bin", import.meta.url), +); +const mid: Frontiers = [{ peer: "1", counter: 1 }]; +const remote = new LoroDoc(); +remote.setPeerId(9); +remote.getText("text").insert(0, "concurrent"); +const updates = remote.export({ mode: "update" }); +const json = remote.exportJsonUpdates(); + +const readers: Record unknown> = { + checkout: (doc) => doc.checkout(mid), + diff: (doc) => doc.diff(mid, doc.oplogFrontiers()), + revertTo: (doc) => doc.revertTo(mid), + import: (doc) => doc.import(updates), + importBatch: (doc) => doc.importBatch([updates, updates]), + importJsonUpdates: (doc) => doc.importJsonUpdates(json), + forkAt: (doc) => doc.forkAt(mid), + findIdSpansBetween: (doc) => + doc.findIdSpansBetween(mid, doc.oplogFrontiers()), + frontiersToVV: (doc) => doc.frontiersToVV(mid), + vvToFrontiers: (doc) => { + const vv = new VersionVector(doc.oplogVersion().toJSON()); + vv.setEnd({ peer: "1", counter: 2 }); + return doc.vvToFrontiers(vv); + }, + travelChangeAncestors: (doc) => doc.travelChangeAncestors(mid, () => true), + getChangeAt: (doc) => doc.getChangeAt(mid[0]), + getChangeAtLamport: (doc) => doc.getChangeAtLamport("1", 1), + getOpsInChange: (doc) => doc.getOpsInChange(mid[0]), + getChangedContainersIn: (doc) => doc.getChangedContainersIn(mid[0], 1), + cmpFrontiers: (doc) => doc.cmpFrontiers(mid, doc.oplogFrontiers()), + exportUpdates: (doc) => doc.export({ mode: "update" }), + exportUpdatesInRange: (doc) => + doc.export({ + mode: "updates-in-range", + spans: [{ id: mid[0], len: 1 }], + }), + exportShallow: (doc) => + doc.export({ mode: "shallow-snapshot", frontiers: mid }), +}; + +for (const [name, read] of Object.entries(readers)) { + it(`${name} rejects its first unreadable block without trapping WASM`, () => { + const doc = new LoroDoc(); + doc.import(snapshot); + const value = doc.toJSON(); + const frontiers = doc.frontiers(); + const vv = doc.oplogVersion().toJSON(); + let events = 0; + doc.subscribe(() => events++); + expect(() => read(doc)).toThrow(/cannot parse change block/); + expect(doc.toJSON()).toEqual(value); + expect(doc.frontiers()).toEqual(frontiers); + expect(doc.oplogVersion().toJSON()).toEqual(vv); + expect(doc.isDetached()).toBe(false); + expect(events).toBe(0); + expect(() => doc.checkout(frontiers)).toThrow(/cannot parse change block/); + // A fresh document in the same module still works: no WASM trap occurred. + const healthy = new LoroDoc(); + healthy.getText("text").insert(0, "usable"); + expect(healthy.toJSON()).toEqual({ text: "usable" }); + }); +} diff --git a/crates/loro/Cargo.toml b/crates/loro/Cargo.toml index 9fa1b0579..7389a90cf 100644 --- a/crates/loro/Cargo.toml +++ b/crates/loro/Cargo.toml @@ -26,6 +26,7 @@ tracing = { workspace = true } rustc-hash = { workspace = true } [dev-dependencies] +xxhash-rust = { workspace = true } serde_json = { version = "1.0.87", features = ["float_roundtrip"] } anyhow = "1.0.83" ctor = "0.2" diff --git a/crates/loro/src/lib.rs b/crates/loro/src/lib.rs index 4ff8e9779..ece20fef8 100644 --- a/crates/loro/src/lib.rs +++ b/crates/loro/src/lib.rs @@ -186,6 +186,11 @@ impl LoroDoc { Some(ChangeMeta::from_change(&change)) } + /// Reads change metadata, reporting unreadable history instead of an absent change. + pub fn try_get_change(&self, id: ID) -> LoroResult> { + self.doc.try_get_change(id) + } + /// Decodes the metadata for an imported blob from the provided bytes. /// /// # Example @@ -862,6 +867,21 @@ impl LoroDoc { self.doc.vv_to_frontiers(vv) } + /// Converts frontiers to a version vector, reporting unreadable history. + pub fn try_frontiers_to_vv(&self, frontiers: &Frontiers) -> LoroResult> { + self.doc.try_frontiers_to_vv(frontiers) + } + + /// Converts a version vector to frontiers, reporting unreadable history. + pub fn try_vv_to_frontiers(&self, vv: &VersionVector) -> LoroResult { + self.doc.try_vv_to_frontiers(vv) + } + + /// Removes redundant frontiers, reporting unreadable history or an absent id. + pub fn try_minimize_frontiers(&self, frontiers: &Frontiers) -> LoroResult { + self.doc.try_minimize_frontiers(frontiers) + } + /// Access the `OpLog`. /// /// NOTE: The API in `OpLog` is unstable. Keep the closure short; avoid calling methods @@ -1443,6 +1463,24 @@ impl LoroDoc { self.doc.find_id_spans_between(from, to) } + /// Finds operation spans between versions, reporting unreadable history. + pub fn try_find_id_spans_between( + &self, + from: &Frontiers, + to: &Frontiers, + ) -> LoroResult { + self.doc.try_find_id_spans_between(from, to) + } + + /// Gets modified container IDs, reporting unreadable history. + pub fn try_get_changed_containers_in( + &self, + id: ID, + len: usize, + ) -> LoroResult> { + self.doc.try_get_changed_containers_in(id, len) + } + /// Revert the current document state back to the target version /// /// Internally, it will generate a series of local operations that can revert the diff --git a/crates/loro/tests/perf_history_lazy_load.rs b/crates/loro/tests/perf_history_lazy_load.rs new file mode 100644 index 000000000..ddf12e526 --- /dev/null +++ b/crates/loro/tests/perf_history_lazy_load.rs @@ -0,0 +1,68 @@ +use loro::{ExportMode, Frontiers, LoroDoc, ID}; +use std::{hint::black_box, time::Instant}; + +/// Cold snapshot import, lazy historical checkout, and concurrent update imports. +/// Run in release, with one test thread, on both revisions; compare three repeats. +#[test] +#[ignore] +fn perf_history_lazy_load() { + let source = LoroDoc::new(); + source.set_peer_id(1).unwrap(); + source.set_change_merge_interval(0); + let text = source.get_text("text"); + for _ in 0..600 { + text.insert(0, &"x".repeat(30)).unwrap(); + source.commit(); + } + source.set_peer_id(2).unwrap(); + for _ in 0..300 { + text.insert(0, &"y".repeat(30)).unwrap(); + source.commit(); + } + source.get_map("map").insert("key", 1).unwrap(); + source.commit(); + let snapshot = source.export(ExportMode::Snapshot).unwrap(); + let remote = LoroDoc::new(); + remote.set_peer_id(3).unwrap(); + remote.get_text("text").insert(0, "concurrent").unwrap(); + let updates = remote.export(ExportMode::all_updates()).unwrap(); + let map_remote = LoroDoc::new(); + map_remote.set_peer_id(4).unwrap(); + map_remote.get_map("map").insert("key", 2).unwrap(); + let map_updates = map_remote.export(ExportMode::all_updates()).unwrap(); + for repeat in 1..=3 { + let mut import = std::time::Duration::ZERO; + let mut checkout = import; + let mut concurrent = import; + let mut map_import = import; + for _ in 0..30 { + let doc = LoroDoc::new(); + let start = Instant::now(); + doc.import(black_box(&snapshot)).unwrap(); + import += start.elapsed(); + let start = Instant::now(); + doc.checkout(&Frontiers::from(ID::new(1, 10))).unwrap(); + checkout += start.elapsed(); + black_box(doc.get_deep_value()); + let doc = LoroDoc::new(); + doc.import(&snapshot).unwrap(); + let start = Instant::now(); + doc.import(black_box(&updates)).unwrap(); + concurrent += start.elapsed(); + black_box(doc.get_deep_value()); + let doc = LoroDoc::new(); + doc.import(&snapshot).unwrap(); + let start = Instant::now(); + doc.import(black_box(&map_updates)).unwrap(); + map_import += start.elapsed(); + black_box(doc.get_deep_value()); + } + println!( + "repeat={repeat} snapshot_import_us={} cold_checkout_us={} concurrent_import_us={} map_import_us={}", + import.as_micros(), + checkout.as_micros(), + concurrent.as_micros(), + map_import.as_micros() + ); + } +} diff --git a/crates/loro/tests/unregistered_container_parent.rs b/crates/loro/tests/unregistered_container_parent.rs index a11c96b31..6b33be6f9 100644 --- a/crates/loro/tests/unregistered_container_parent.rs +++ b/crates/loro/tests/unregistered_container_parent.rs @@ -950,3 +950,269 @@ mod random { } } } + +/// Rewrite a change-block value and both checksum layers, as in the independent +/// #1172 verifier. Import must remain lazy: no earlier query records the failure. +fn snapshot_with_truncated_history(snapshot: &[u8], block_index: usize) -> (Vec, ID) { + use loro_kv_store::{mem_store::MemKvConfig, MemKvStore}; + use std::ops::Bound; + let mut at = 22; + let mut sections = Vec::new(); + for _ in 0..3 { + let len = u32::from_le_bytes(snapshot[at..at + 4].try_into().unwrap()) as usize; + at += 4; + sections.push(snapshot[at..at + len].to_vec()); + at += len; + } + let mut kv = MemKvStore::new(MemKvConfig::new()); + kv.import_all(sections[0].clone().into()).unwrap(); + let blocks: Vec<_> = kv + .scan(Bound::Unbounded, Bound::Unbounded) + .filter(|(key, _)| key.len() == 12 && ID::from_bytes(key).peer == 1) + .collect(); + assert!(blocks.len() >= 3); + let index = match block_index { + 0 => 0, + 1 => blocks.len() / 2, + _ => blocks.len() - 1, + }; + let (key, bytes) = &blocks[index]; + let id = ID::from_bytes(key).inc(1); + kv.set(key, bytes.slice(..bytes.len() / 2)); + sections[0] = kv.export_all().to_vec(); + let mut forged = snapshot[..22].to_vec(); + for section in sections { + forged.extend_from_slice(&(section.len() as u32).to_le_bytes()); + forged.extend_from_slice(§ion); + } + let checksum = xxhash_rust::xxh32::xxh32(&forged[20..], u32::from_le_bytes(*b"LORO")); + forged[16..20].copy_from_slice(&checksum.to_le_bytes()); + (forged, id) +} + +#[test] +fn fallible_history_readers_reject_a_truncated_block_on_the_first_read() { + use loro::{Frontiers, VersionVector}; + let source = LoroDoc::new(); + source.set_peer_id(1).unwrap(); + source.set_change_merge_interval(-1); + for i in 0..600 { + source + .get_text("text") + .insert(0, &format!("line-{i:04}-xxxxxxxxxxxxxxxxxxxxxxxx")) + .unwrap(); + source.get_map("map").insert("key", i).unwrap(); + source.commit(); + } + source.set_peer_id(2).unwrap(); + for _ in 0..300 { + source.get_text("text").insert(0, &"y".repeat(30)).unwrap(); + source.commit(); + } + let snapshot = source.export(ExportMode::Snapshot).unwrap(); + let remote = LoroDoc::new(); + remote.set_peer_id(9).unwrap(); + remote.get_text("text").insert(0, "concurrent").unwrap(); + let updates = remote.export(ExportMode::all_updates()).unwrap(); + // Register-only imports normally skip state-apply rollback. Lazy history can + // still fail after these changes are appended, so test that path separately. + let map_remote = LoroDoc::new(); + map_remote.set_peer_id(10).unwrap(); + map_remote.get_map("map").insert("key", -1).unwrap(); + let map_updates = map_remote.export(ExportMode::all_updates()).unwrap(); + let json = remote.export_json_updates_without_peer_compression( + &VersionVector::default(), + &remote.oplog_vv(), + ); + for block in 0..3 { + let (forged, id) = snapshot_with_truncated_history(&snapshot, block); + // Regenerate the WASM regression fixture from the same public-API rewrite: + // LORO_HISTORY_FIXTURE= cargo test -p loro --test + // unregistered_container_parent fallible_history_readers + if block == 0 { + if let Some(path) = std::env::var_os("LORO_HISTORY_FIXTURE") { + std::fs::write(path, &forged).unwrap(); + } + } + let mid = Frontiers::from(id); + let dependent = source.fork_at(&mid).unwrap(); + dependent + .get_text("text") + .insert(0, "depends on the broken block") + .unwrap(); + let dependent_updates = dependent + .export(ExportMode::updates(&source.oplog_vv())) + .unwrap(); + let latest = source.oplog_frontiers(); + // This vector/frontier forces a two-peer walk through the broken block. + let mut vv = VersionVector::default(); + vv.set_end(id.inc(1)); + vv.set_end(latest.as_single().unwrap().inc(1)); + let redundant = Frontiers::from(vec![id, latest.as_single().unwrap()]); + for api in [ + "checkout", + "diff", + "revert_to", + "import", + "import_map", + "import_batch", + "import_batch_multiple", + "import_json", + "detached_import", + "fork_at", + "find_id_spans_between", + "frontiers_to_vv", + "vv_to_frontiers", + "minimize_frontiers", + "travel_change_ancestors", + "get_change_at", + "get_change_at_lamport", + "get_remote_change_at", + "get_changed_containers_in", + "cmp_frontiers", + "export_updates", + "export_updates_in_range", + "export_shallow", + "export_state_only", + "export_snapshot_at", + ] { + let doc = LoroDoc::new(); + doc.import(&forged).unwrap(); + if api == "detached_import" { + doc.detach(); + } + let value = doc.get_deep_value(); + let frontiers = doc.state_frontiers(); + let old_vv = doc.oplog_vv(); + let detached = doc.is_detached(); + let mut visits = 0; + let result: Result<(), String> = match api { + "checkout" => doc.checkout(&mid).map_err(|e| e.to_string()), + "diff" => doc + .diff(&mid, &latest) + .map(|_| ()) + .map_err(|e| e.to_string()), + "revert_to" => doc.revert_to(&mid).map_err(|e| e.to_string()), + "import" => doc.import(&updates).map(|_| ()).map_err(|e| e.to_string()), + "import_map" => doc + .import(&map_updates) + .map(|_| ()) + .map_err(|e| e.to_string()), + "detached_import" => doc + .import(&dependent_updates) + .map(|_| ()) + .map_err(|e| e.to_string()), + "import_batch" => doc + .import_batch(std::slice::from_ref(&updates)) + .map(|_| ()) + .map_err(|e| e.to_string()), + "import_batch_multiple" => doc + .import_batch(&[updates.clone(), map_updates.clone()]) + .map(|_| ()) + .map_err(|e| e.to_string()), + "import_json" => doc + .import_json_updates(json.clone()) + .map(|_| ()) + .map_err(|e| e.to_string()), + "fork_at" => doc.fork_at(&mid).map(|_| ()).map_err(|e| e.to_string()), + "find_id_spans_between" => doc + .try_find_id_spans_between(&mid, &latest) + .map(|_| ()) + .map_err(|e| e.to_string()), + "frontiers_to_vv" => doc + .try_frontiers_to_vv(&mid) + .map(|_| ()) + .map_err(|e| e.to_string()), + "vv_to_frontiers" => doc + .try_vv_to_frontiers(&vv) + .map(|_| ()) + .map_err(|e| e.to_string()), + "minimize_frontiers" => doc + .try_minimize_frontiers(&redundant) + .map(|_| ()) + .map_err(|e| e.to_string()), + "travel_change_ancestors" => doc + .travel_change_ancestors(&[id], &mut |_| { + visits += 1; + std::ops::ControlFlow::Continue(()) + }) + .map_err(|e| e.to_string()), + "get_change_at" => doc + .try_get_change(id) + .map(|_| ()) + .map_err(|e| e.to_string()), + "get_change_at_lamport" => doc + .with_oplog(|oplog| oplog.try_get_change_with_lamport_lte(1, id.counter as u32)) + .map(|_| ()) + .map_err(|e| e.to_string()), + "get_remote_change_at" => doc + .with_oplog(|oplog| oplog.try_get_remote_change_at(id)) + .map(|_| ()) + .map_err(|e| e.to_string()), + "get_changed_containers_in" => doc + .try_get_changed_containers_in(id, 1) + .map(|_| ()) + .map_err(|e| e.to_string()), + "cmp_frontiers" => doc + .cmp_frontiers(&mid, &latest) + .map(|_| ()) + .map_err(|e| e.to_string()), + "export_updates" => doc + .export(ExportMode::all_updates()) + .map(|_| ()) + .map_err(|e| e.to_string()), + "export_updates_in_range" => doc + .export(ExportMode::updates_in_range(vec![loro::IdSpan::new( + 1, + id.counter, + *source.oplog_vv().get(&1).unwrap(), + )])) + .map(|_| ()) + .map_err(|e| e.to_string()), + "export_shallow" => doc + .export(ExportMode::shallow_snapshot(&mid)) + .map(|_| ()) + .map_err(|e| e.to_string()), + "export_state_only" => doc + .export(ExportMode::state_only(Some(&mid))) + .map(|_| ()) + .map_err(|e| e.to_string()), + "export_snapshot_at" => doc + .export(ExportMode::snapshot_at(&mid)) + .map(|_| ()) + .map_err(|e| e.to_string()), + _ => unreachable!(), + }; + let err = result.unwrap_err(); + assert!( + err.contains("cannot parse change block"), + "{api}, block={block}: {err}" + ); + assert_eq!(doc.get_deep_value(), value, "{api}, block={block}"); + assert_eq!(doc.state_frontiers(), frontiers, "{api}, block={block}"); + assert_eq!(doc.oplog_vv(), old_vv, "{api}, block={block}"); + assert_eq!(doc.is_detached(), detached, "{api}, block={block}"); + assert_eq!(visits, 0); + // Locks are usable and every subsequent fallible reader sees the record. + assert!(doc + .checkout(&latest) + .unwrap_err() + .to_string() + .contains("cannot parse change block")); + } + } + // Initial shallow import itself needs the root's DAG node, unlike a full + // snapshot import. Its first unreadable block must also roll back cleanly. + let root = Frontiers::from(ID::new(1, 38)); + let shallow = source.export(ExportMode::shallow_snapshot(&root)).unwrap(); + let (forged, _) = snapshot_with_truncated_history(&shallow, 0); + let doc = LoroDoc::new(); + let value = doc.get_deep_value(); + let err = doc.import(&forged).unwrap_err(); + assert!(err.to_string().contains("cannot parse change block")); + assert_eq!(doc.get_deep_value(), value); + assert!(doc.oplog_vv().is_empty()); + assert!(!doc.is_detached()); + doc.import(&snapshot).unwrap(); + assert_eq!(doc.get_deep_value(), source.get_deep_value()); +} From 2e97be5c6884014ee6cdb1ba78df016206c38351 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Thu, 1 Oct 2026 06:14:10 +0800 Subject: [PATCH 2/2] fix: Roll back imports over unreadable block bodies See context/arena-parent-links.md for body validation and compatibility. Co-Authored-By: GPT-6 --- .changeset/fallible-unparsable-history.md | 4 + context/arena-parent-links.md | 42 +++++- crates/loro-internal/src/dag/iter.rs | 1 + crates/loro-internal/src/loro.rs | 9 +- crates/loro-internal/src/oplog.rs | 14 +- .../loro-internal/src/oplog/change_store.rs | 126 +++++++++++++++--- crates/loro-internal/src/oplog/loro_dag.rs | 12 +- .../fixtures/unparsable-body.snapshot.bin | Bin 0 -> 10002 bytes .../tests/unparsable_history.test.ts | 32 +++++ crates/loro/tests/perf_history_lazy_load.rs | 46 +++++++ .../tests/unregistered_container_parent.rs | 80 ++++++++++- 11 files changed, 327 insertions(+), 39 deletions(-) create mode 100644 crates/loro-wasm/tests/fixtures/unparsable-body.snapshot.bin diff --git a/.changeset/fallible-unparsable-history.md b/.changeset/fallible-unparsable-history.md index 62c460cc4..065ca5f57 100644 --- a/.changeset/fallible-unparsable-history.md +++ b/.changeset/fallible-unparsable-history.md @@ -3,3 +3,7 @@ --- Return decode errors when fallible history operations first read an unreadable change block, preserving document state instead of panicking during lazy DAG loading. + +Keep attached imports atomic even after all DAG headers have been loaded: unparsed change-block bodies still require rollback protection. Share the existing version snapshot between rollback journals to avoid two extra copies of the version vector. Legacy infallible range readers continue skipping only the damaged block while recording the failure. + +Rust compatibility: `ChangeTravelError` gains `HistoryUnreadable(LoroError)` and is now `#[non_exhaustive]`; downstream exhaustive matches must add a wildcard arm. `FrontiersNotIncluded` changes from a unit struct to a struct with a private field plus a same-named constant. Existing construction and constant patterns still compile, but an unreadable-history error is no longer equal to the `FrontiersNotIncluded` constant. diff --git a/context/arena-parent-links.md b/context/arena-parent-links.md index 866815f4a..4593d7af5 100644 --- a/context/arena-parent-links.md +++ b/context/arena-parent-links.md @@ -133,13 +133,25 @@ already recorded failures, and their history reads also propagate failures calculation use fallible reads. `checkout`, `diff`, `revert_to`, `fork_at`, binary/JSON/batch imports, and shallow/state-only/snapshot-at exports propagate the error before applying state. Attached imports enable their existing - rollback journal when old history is still unparsed, including register-only - imports that otherwise need no journal. Initial shallow-snapshot import also + rollback journal when the change store may still hold unparsed **bodies**, + including register-only imports that otherwise need no journal. DAG loading + reads headers only: draining `AppDag::unparsed_vv` does not validate bodies. + `ChangeStoreInner::may_have_unparsed_bodies` is a conservative flag set on + snapshot import/fork, cleared by a successful `visit_all_changes`, and set + again when arena rollback discards parsed bodies. Individual body reads do + not clear it; imports can therefore keep using a journal after all bodies + have been read individually, until a full walk confirms that fact. + The attached import, DAG journal, and change-store journal share one + `Arc` snapshot instead of copying that vector three times. + The DAG's separate `unparsed_vv` checkpoint is still copied; after header + warming it is empty. Initial shallow-snapshot import also returns the error if reading its root fails, then resets state and op log. - `ChangeStore::try_iter_changes` validates the requested range before handing changes to a consumer. Export stops at an unreadable block rather than feeding a gap into a scratch history or returning partial bytes; the public - export's final history check returns the recorded error. + export's final history check returns the recorded error. Legacy `iter_blocks` + and `iter_changes` instead record and skip only the bad block, preserving + the healthy portions of the requested range at their infallible boundaries. - Query signatures that previously returned `Option` or plain values remain available. Use `try_frontiers_to_vv`, `try_vv_to_frontiers`, `try_minimize_frontiers`, `try_find_id_spans_between`, `try_get_change` @@ -147,6 +159,10 @@ already recorded failures, and their history reads also propagate failures decode errors. `cmp_frontiers` keeps its `FrontiersNotIncluded` error type, which can now carry the decode-error message; `travel_change_ancestors` returns `ChangeTravelError::HistoryUnreadable(LoroError::DecodeError(..))`. + `ChangeTravelError` is now non-exhaustive, so downstream matches need a + wildcard arm. `FrontiersNotIncluded` now has a private message field and a + same-named constant: old construction/pattern syntax still works, but an + unreadable-history error is not equal to that missing-frontiers constant. WASM queries that already return `JsResult` use these fallible readers too, including `findIdSpansBetween`, `frontiersToVV`, `vvToFrontiers`, `getChangeAt`, `getChangeAtLamport`, `getOpsInChange`, and @@ -158,6 +174,15 @@ caches. The successful DAG lazy-load path does no additional block reads or parse-failure checks: the error-record lookup is inside the missing-node branch. The release benchmark `crates/loro/tests/perf_history_lazy_load.rs` measures fresh snapshot import, cold historical checkout, and concurrent update import. +Its `perf_map_import_many_peers` case measures a one-op causal map import with +1k/10k peers, both cold and after warming every DAG header without parsing old +bodies. Setup is outside the timer. Against pre-review commit `be17aed3`, +three alternating before/after pairs, each with three repeats of 30 imports, +gave median after/before ratios of 1.001/1.011 (cold) and 0.997/0.996 (headers +warmed), on macOS arm64 in release. Sharing the version snapshot removes two +extra full-vector clones from journal creation; opening the journal no longer +adds those O(peers) copies. These timings show no material increase in this +workload, rather than a bound on every import workload. Limits and compatibility: @@ -186,12 +211,19 @@ rewrites snapshot bytes through the public KV API and repairs both checksum layers. Each fallible entry point gets its own fresh document, with the first, middle, and last peer-1 block truncated; it must return "cannot parse change block" without a panic, preserve the value/version/status, and leave locks -usable. It also tests initial shallow-import rollback. The healthy-doc test +usable. It also tests initial shallow-import rollback. The regression +`register_only_import_rolls_back_when_only_old_block_headers_were_read` flips +the third byte from the end of the first/middle/last peer-1 block and repairs +the checksums. Both peers' DAG version queries succeed before a concurrent +map import first parses the bad body. The import must return the decode error, +with op log and state frontiers still equal to their original frontiers, +unchanged values/version vector, and an attached document. The healthy-doc test `a_recorded_parse_failure_does_not_panic_where_no_error_can_be_returned` keeps manual error records separate from actual unreadable blocks. `crates/loro-wasm/tests/unparsable_history.test.ts` uses the same corrupted snapshot as a fixture and checks JS exceptions, unchanged values/versions, -and a usable WASM instance after each first read. +and a usable WASM instance after each first read. Its body-corruption fixture +also verifies a map import's rollback after both peers' header queries succeed. ## Cost of looking up an ID diff --git a/crates/loro-internal/src/dag/iter.rs b/crates/loro-internal/src/dag/iter.rs index d81ed19b9..8427b1fde 100644 --- a/crates/loro-internal/src/dag/iter.rs +++ b/crates/loro-internal/src/dag/iter.rs @@ -319,6 +319,7 @@ impl> Iterator for DagCausalIter<'_, D> { } // node_id may point into the middle of the node, we need to slice + // try_new has loaded every target node before iteration starts. let node = self.dag.get(node_id).unwrap(); // node start_id may be smaller than node_id let counter = node.id_span().counter; diff --git a/crates/loro-internal/src/loro.rs b/crates/loro-internal/src/loro.rs index 500adc503..b5ff71def 100644 --- a/crates/loro-internal/src/loro.rs +++ b/crates/loro-internal/src/loro.rs @@ -844,7 +844,7 @@ impl LoroDoc { return Ok(result.status); } - let old_vv = oplog.vv().clone(); + let old_vv = Arc::new(oplog.vv().clone()); let old_frontiers = oplog.frontiers().clone(); // Checked before the changes are applied, while the store still holds only old history. let isolated_batch = isolated_scalar_root_batch(&oplog, &changes).filter(|(_, names)| { @@ -855,13 +855,13 @@ impl LoroDoc { }) }); let rollback_enabled = - preflight.needs_state_apply_rollback || oplog.dag.has_unparsed_history(); + preflight.needs_state_apply_rollback || oplog.change_store().may_have_unparsed_bodies(); if rollback_enabled { - oplog.begin_import_rollback_with_arena(arena_checkpoint); + oplog.begin_import_rollback_with_version(arena_checkpoint, old_vv.clone()); } let result = encoding::apply_decoded_changes_to_oplog(&mut oplog, changes); - if &old_vv != oplog.vv() { + if old_vv.as_ref() != oplog.vv() { // The preflight enables rollback whenever the imported or unlocked // pending changes hold movable-list ops, so other imports skip the scan. if rollback_enabled { @@ -3268,6 +3268,7 @@ fn isolated_scalar_root_batch( } #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum ChangeTravelError { #[error(transparent)] HistoryUnreadable(#[from] LoroError), diff --git a/crates/loro-internal/src/oplog.rs b/crates/loro-internal/src/oplog.rs index ba7ecdd75..32e6bf4cc 100644 --- a/crates/loro-internal/src/oplog.rs +++ b/crates/loro-internal/src/oplog.rs @@ -342,11 +342,21 @@ impl OpLog { } pub(crate) fn begin_import_rollback_with_arena(&mut self, arena: SharedArenaRollback) { + let old_vv = Arc::new(self.vv().clone()); + self.begin_import_rollback_with_version(arena, old_vv); + } + + /// Share the attached import's existing version snapshot with both journals. + pub(crate) fn begin_import_rollback_with_version( + &mut self, + arena: SharedArenaRollback, + old_vv: Arc, + ) { debug_assert!(self.import_rollback.is_none()); - self.dag.begin_import_rollback(); + self.dag.begin_import_rollback(old_vv.clone()); self.import_rollback = Some(ImportRollback { arena, - change_store: ChangeStoreRollback::new(self.vv().clone()), + change_store: ChangeStoreRollback::new(old_vv), pending: Default::default(), movable_list_refs: Vec::new(), }); diff --git a/crates/loro-internal/src/oplog/change_store.rs b/crates/loro-internal/src/oplog/change_store.rs index b619f76eb..594ef9d98 100644 --- a/crates/loro-internal/src/oplog/change_store.rs +++ b/crates/loro-internal/src/oplog/change_store.rs @@ -162,11 +162,13 @@ struct ChangeStoreInner { /// Set by [`ChangeStore::retire`]: the op log replaced this store, so loading from it /// must not register anything in the arena any more. retired: bool, + /// Conservative until a full history walk: headers do not validate block bodies. + may_have_unparsed_bodies: bool, } #[derive(Debug)] pub(crate) struct ChangeStoreRollback { - old_vv: VersionVector, + old_vv: Arc, /// Pre-scope blocks that an import appended to, keyed by block id. `None` means the /// block was flushed, so its KV copy is the pre-scope version. blocks_before_mutation: BTreeMap>, @@ -191,7 +193,7 @@ struct BlockShape { } impl ChangeStoreRollback { - pub(crate) fn new(old_vv: VersionVector) -> Self { + pub(crate) fn new(old_vv: Arc) -> Self { Self { old_vv, blocks_before_mutation: BTreeMap::new(), @@ -269,6 +271,7 @@ impl ChangeStore { start_frontiers: Frontiers::default(), mem_parsed_kv: BTreeMap::new(), retired: false, + may_have_unparsed_bodies: false, })), arena: a.clone(), external_vv: Arc::new(Mutex::new(VersionVector::new())), @@ -545,14 +548,17 @@ impl ChangeStore { } fn rollback_arena_in(&self, inner: &mut ChangeStoreInner, arena: SharedArenaRollback) { + let mut dropped_body = false; for block in inner.mem_parsed_kv.values_mut() { if let ChangesBlockContent::Both(_, bytes) = &block.content { if !arena.keeps(block.parsed_extent) { let bytes = bytes.clone(); Arc::make_mut(block).content = ChangesBlockContent::Bytes(bytes); + dropped_body = true; } } } + inner.may_have_unparsed_bodies |= dropped_body; self.arena.rollback(arena); } @@ -577,6 +583,12 @@ impl ChangeStore { Some(block.content.iter_dag_nodes()) } + /// Whether diff calculation may still encounter an unvalidated snapshot body. + /// Loading all DAG headers cannot clear this; only a successful full body walk can. + pub(crate) fn may_have_unparsed_bodies(&self) -> bool { + self.inner.lock().may_have_unparsed_bodies + } + pub fn visit_all_changes(&self, f: &mut dyn FnMut(&Change)) { self.ensure_block_loaded_in_range(Bound::Unbounded, Bound::Unbounded); let mut inner = self.inner.lock(); @@ -590,6 +602,9 @@ impl ChangeStore { f(c); } } + if self.corrupt_block_error().is_ok() { + inner.may_have_unparsed_bodies = false; + } } fn build_root_history_names(&self) -> RootHistoryNamesState { @@ -712,30 +727,43 @@ impl ChangeStore { } pub(crate) fn iter_blocks(&self, id_span: IdSpan) -> Vec<(Arc, usize, usize)> { - self.try_iter_blocks(id_span).unwrap_or_default() + self.iter_blocks_inner(id_span, false).unwrap() } fn try_iter_blocks( &self, id_span: IdSpan, + ) -> LoroResult, usize, usize)>> { + self.iter_blocks_inner(id_span, true) + } + + fn iter_blocks_inner( + &self, + id_span: IdSpan, + fail_on_corrupt: bool, ) -> LoroResult, usize, usize)>> { if id_span.counter.start == id_span.counter.end { return Ok(vec![]); } assert!(id_span.counter.start < id_span.counter.end); - self.try_ensure_block_loaded_in_range( - Bound::Included(id_span.id_start()), - Bound::Excluded(id_span.id_end()), - )?; - let mut inner = self.inner.lock(); - let Some((start_id, _)) = inner.mem_parsed_kv.range(..=id_span.id_start()).next_back() - else { - return Ok(vec![]); - }; - if start_id.peer != id_span.peer { - return Ok(vec![]); + if fail_on_corrupt { + self.try_ensure_block_loaded_in_range( + Bound::Included(id_span.id_start()), + Bound::Excluded(id_span.id_end()), + )?; + } else { + self.ensure_block_loaded_in_range( + Bound::Included(id_span.id_start()), + Bound::Excluded(id_span.id_end()), + ); } - let start_counter = start_id.counter; + let mut inner = self.inner.lock(); + let start_counter = inner + .mem_parsed_kv + .range(..=id_span.id_start()) + .next_back() + .filter(|(id, _)| id.peer == id_span.peer) + .map_or(id_span.counter.start, |(id, _)| id.counter); let mut blocks = Vec::new(); for (block_id, block) in inner.mem_parsed_kv.range_mut( ID::new(id_span.peer, start_counter)..ID::new(id_span.peer, id_span.counter.end), @@ -746,7 +774,10 @@ impl ChangeStore { if let Err(err) = block.ensure_changes(&self.arena) { warn!(?block_id, ?err, "failed to parse change block"); self.parse_failures.record(*block_id, &err); - return Err(self.corrupt_block_error().unwrap_err()); + if fail_on_corrupt { + return Err(self.corrupt_block_error().unwrap_err()); + } + continue; } let changes = block.content.try_changes().unwrap(); let (start, end) = if id_span.counter.start <= block.counter_range.0 @@ -775,7 +806,7 @@ impl ChangeStore { let v = self.iter_blocks(id_span); #[cfg(debug_assertions)] { - if !v.is_empty() { + if !v.is_empty() && self.corrupt_block_error().is_ok() { assert_eq!(v[0].0.peer, id_span.peer); assert_eq!(v.last().unwrap().0.peer, id_span.peer); { @@ -905,6 +936,7 @@ impl ChangeStore { start_frontiers: inner.start_frontiers.clone(), mem_parsed_kv: BTreeMap::new(), retired: false, + may_have_unparsed_bodies: !vv.is_empty(), })), arena, external_vv: Arc::new(Mutex::new(self.external_vv.lock().clone())), @@ -1058,6 +1090,7 @@ mod mut_external_kv { } } + self.inner.lock().may_have_unparsed_bodies = !vv.is_empty(); *self.external_vv.lock() = vv.clone(); let frontiers_bytes = self .external_kv @@ -2449,6 +2482,61 @@ mod test { store.inner.lock().mem_parsed_kv.clear(); } + #[test] + fn legacy_block_iteration_skips_only_the_broken_block() { + for position in 0..3 { + let (store, end, _) = kv_only_store_and_next_change(); + let ids: Vec<_> = store + .external_kv + .lock() + .scan(Bound::Unbounded, Bound::Unbounded) + .filter(|(key, _)| key.len() == 12) + .map(|(key, _)| ID::from_bytes(&key)) + .collect(); + assert!(ids.len() >= 3); + let broken = ids[match position { + 0 => 0, + 1 => ids.len() / 2, + _ => ids.len() - 1, + }]; + truncate_block(&store, broken); + let span = IdSpan::new(1, 0, end); + let blocks = store.iter_blocks(span); + assert_eq!(blocks.len(), ids.len() - 1, "position={position}"); + assert!(blocks + .iter() + .all(|(block, _, _)| block.counter_range.0 != broken.counter)); + assert!(store + .corrupt_block_error() + .unwrap_err() + .to_string() + .contains("cannot parse change block")); + // The legacy iterator also yields the healthy portions, without bounds asserts. + assert!(store.iter_changes(span).count() > 0); + assert!(store + .try_iter_blocks(span) + .unwrap_err() + .to_string() + .contains("cannot parse change block")); + } + } + + #[test] + fn body_validation_stays_lazy_after_header_reads_and_can_be_reset_by_rollback() { + let (store, end, _) = kv_only_store_and_next_change(); + assert!(store.may_have_unparsed_bodies()); + assert!(store.get_dag_nodes_that_contains(ID::new(1, 0)).is_some()); + assert!(store.may_have_unparsed_bodies()); + let arena = store.arena.checkpoint_for_rollback(); + store.visit_all_changes(&mut |_| {}); + assert!(!store.may_have_unparsed_bodies()); + store.rollback_arena(arena); + assert!(store.may_have_unparsed_bodies()); + assert!(store.get_change(ID::new(1, end - 1)).is_some()); + store.visit_all_changes(&mut |_| {}); + assert!(!store.may_have_unparsed_bodies()); + } + #[test] fn the_creator_resolver_reports_an_unparsable_block_instead_of_panicking() { let (store, _, _) = kv_only_store_and_next_change(); @@ -2687,7 +2775,7 @@ mod test { let (store, end, next) = kv_only_store_and_next_change(); let mut old_vv = VersionVector::new(); old_vv.insert(1, end); - let mut rollback = ChangeStoreRollback::new(old_vv); + let mut rollback = ChangeStoreRollback::new(Arc::new(old_vv)); let arena = store.arena.checkpoint_for_rollback(); store.insert_change_with_rollback(next.clone(), true, false, &mut rollback); assert!(store.get_change(ID::new(1, 0)).is_some()); @@ -2737,7 +2825,7 @@ mod test { for c in first { rolled_back.insert_change(c.clone(), true, false); } - let mut rollback = ChangeStoreRollback::new(vv_of(first)); + let mut rollback = ChangeStoreRollback::new(Arc::new(vv_of(first))); let arena_checkpoint = arena.checkpoint_for_rollback(); for c in &rest[..rest.len() / 2] { rolled_back.insert_change_with_rollback(c.clone(), true, false, &mut rollback); diff --git a/crates/loro-internal/src/oplog/loro_dag.rs b/crates/loro-internal/src/oplog/loro_dag.rs index 451728af3..673a0ad85 100644 --- a/crates/loro-internal/src/oplog/loro_dag.rs +++ b/crates/loro-internal/src/oplog/loro_dag.rs @@ -60,7 +60,7 @@ pub struct AppDag { #[derive(Debug)] pub(crate) struct AppDagRollback { frontiers: Frontiers, - vv: VersionVector, + vv: Arc, unparsed_vv: VersionVector, shallow_since_frontiers: Frontiers, shallow_root_frontiers_deps: Frontiers, @@ -153,13 +153,13 @@ impl AppDag { &self.shallow_since_frontiers } - pub(crate) fn begin_import_rollback(&mut self) { + pub(crate) fn begin_import_rollback(&mut self, old_vv: Arc) { let old_vv_is_empty = self.vv.is_empty(); let mut rollback = self.import_rollback.lock(); debug_assert!(rollback.is_none()); *rollback = Some(AppDagRollback { frontiers: self.frontiers.clone(), - vv: self.vv.clone(), + vv: old_vv, unparsed_vv: self.unparsed_vv.lock().clone(), shallow_since_frontiers: self.shallow_since_frontiers.clone(), shallow_root_frontiers_deps: self.shallow_root_frontiers_deps.clone(), @@ -206,7 +206,7 @@ impl AppDag { drop(map); self.frontiers = checkpoint.frontiers; - self.vv = checkpoint.vv.clone(); + self.vv = (*checkpoint.vv).clone(); self.shallow_since_frontiers = checkpoint.shallow_since_frontiers; self.shallow_root_frontiers_deps = checkpoint.shallow_root_frontiers_deps; self.shallow_since_vv = checkpoint.shallow_since_vv; @@ -630,10 +630,6 @@ impl AppDag { } } - pub(crate) fn has_unparsed_history(&self) -> bool { - !self.unparsed_vv.lock().is_empty() - } - pub fn total_parsed_dag_node(&self) -> usize { self.map.lock().len() } diff --git a/crates/loro-wasm/tests/fixtures/unparsable-body.snapshot.bin b/crates/loro-wasm/tests/fixtures/unparsable-body.snapshot.bin new file mode 100644 index 0000000000000000000000000000000000000000..6db12738559596eb86fd55de21d74b52a63ad804 GIT binary patch literal 10002 zcmeI2dvH|c8OFcw>~0bWyQt(lpaEjTMZDl9Dk58C!TcjNhVtT(?JuCIe&S?m{q2w68|Pn zx7HeKs)kuJZkau8jm6$r(SEjx1m?H_)&%7y2+WRyFj5{wZD6o4CK`|YYSOgmlu3#_njA(BCr6MYJu42u471X36bB9^4@ELWwh>{H|xkQD~T#hGRHzH2FPpEV=U7K2`a%ZCUu>NYj4` zxC%T6d%j7HMtIFtv#>U3&n>5!) z)O>#hV|*84T-{O`nA0X+&?UvI%tW8!xp~_@Wgo?O*^r$vuCLb^XCkkek!EbFre>rm z4PVC_Pf76xinJ8}K#`H+pD40Y{0l`+ihrZXOYtU(S}EQ_Q76S_e;nM~Sdo$g`3?`v zyW|$~J#s6#&C|U1Ly0RvOOXz#qtq$xFyq@=|gxIggz0>Hdg@IcfMO z4qQeqATKAcAQzI0$ScXKJl)l3n3smvaNy76wd8eV8+ko>19>C4*wfvFhPBf0FC4g; zyoFpsE+uazZzJ2uWuESKG}KHl=fGdd733Y{o#b8Q-Q+#wy`JtqG}KJr&w&TXm857( z-hpUKDB2Q=w$8Jf<;cTwnKko=wajOh??IN2nVWB^jP5SYZIi!-G^gt8G|jol8Y}M; z8LWU+$EJO(Ayyx2nAOJ`YW1;(TYan{S08KG)yEoo^|6Lu^+>GsbWdSGQ{Bmdr^&ta zYxpd;r(PZO>O)K=+Kgt)XJ zz2)F7pXwfpD}=bTA-(0`EuZQhiYtV;v?0Ca;4Po(;>6|Cyf~0FA9mgGCos+TOPUup z_%vVG<=Wi2^V+=PT5H?S+1D56bbT%3d{ZDtOfIPqtIsveIyU2T4Y~SU!>&Hp(5ufi z{OWTJ!TMaous+vRYzW2bv7})g4C`ZEq+%=X?F^C`6#^uyE$LVX$NFIJon*_xhuL5a z35jV#7)a-)Ob6gECG@3)y_ArbvcO#-+@Rr=IeX}F$SZXh?3 z1@a0ZFYQQoxfjt<fyR;?MX4{5k@isn0Xeb0Tu^CD0;QO*Z7}lYI%2o%P9X z#k!Pibt#Ip6mwBzq?m^yE5&>iIVlP(8^W@BEGb!cIX2XIUcrHdMY@pS9ZP;=cwb%juuzGa%r!CXGAJye(6G-^Yd%i(L6PxYG|c#GUj zzOD3Wen->Xx^>hfrujasXjn4Swp7OOZF`s3=jSdCzJ}{_wm!!=zbkRhO9&xXpKGzJ z<{EbOx!#6c=X|c;$2!gR2PibxAEL-egO5;TrTCb~qQJ5tEUO2Tl68=*k9C1%hc!iK z1j(G{cDI^c;Cn%{+LM-bu&mGZt~Fa$_!UtsrVSu5B^;)Nz=TfxrMZT`bYLDipWK7I zLdZ+^qq`j3<#W9UdF}13t%JFIu6wAi5bDwy(p(Pa^11Gzxm z98Sj0TU!Tn`CRuec!QPW?%8t_S9gIi4ThFxJ`;lq_wUr}qxdn$mI;4a~Ma zTP)t6JfAl)eJqa_(Fj&X&8;zTsF^in=rn8k_OCKKy1h=DFm2k-X;sheli{?m&TPfB zS_dy!DHB^{(Hnm#kU4EKjp&S>ZflBQF0*9@$ID#$$*g*5ENxXG_MQ0uWHEKuD`7zKW|`{ z4fK}u%MJV;>Y(l$B=+mJL1MoZ1se=L7)N^Q)M?XO8%{jXm|O7s^zTUW7b5xlPh@Ey zd@Oe!g_&(At!aJt*?OM06R^YY18X+EVe!%H_Nm+T548{YKMv*iR5@CW2vwUzRjn-Y zvdGCID~pUQ(y~a&qNdu!tLjd%iQ*QuvPV&iyzEibA}4zkwaCgIMJ+P2M^THk>`~Mr zC3_ULsF6L2Sj4NcKZ+-k+4@s#yvSvuKlaq!zg#4<@`2dJz?Yls<0_fWwUzjHwU2+( zx1oJTPGRpYl9{?iiFhwI{C}%nJwN_%rsbr6OBQ&d{z_H++wyMq;rmF*N$=%FGF5lP zq4D1Cs{N9w{43ZIhjgrkL-3<$yCke0egRBYUI3s zGZx93+K$@zH)JyZi_=Kf)Knb17YDE`{lz`)mQf}WE!!A1Ho(0@!p@&#*GG@Ck3NDQ ze~^%yt@RkYq(esg+A;D&8A|^;`m(;y%Z-fOd&NwX{*PzmNUSO`=j)wsbMx3CZB4v4 F`Y*ad_qPB5 literal 0 HcmV?d00001 diff --git a/crates/loro-wasm/tests/unparsable_history.test.ts b/crates/loro-wasm/tests/unparsable_history.test.ts index c83360ab1..661384acd 100644 --- a/crates/loro-wasm/tests/unparsable_history.test.ts +++ b/crates/loro-wasm/tests/unparsable_history.test.ts @@ -68,3 +68,35 @@ for (const [name, read] of Object.entries(readers)) { expect(healthy.toJSON()).toEqual({ text: "usable" }); }); } + +it("rolls back a map import after reading only the old block headers", () => { + // Generated by the Rust register-only regression with LORO_BODY_FIXTURE set + // to this path: XOR byte -3 in the first peer-1 block, repair both checksums. + const bodyCorrupted = readFileSync( + new URL("./fixtures/unparsable-body.snapshot.bin", import.meta.url), + ); + const doc = new LoroDoc(); + doc.import(bodyCorrupted); + expect(doc.frontiersToVV([{ peer: "1", counter: 0 }])).toBeDefined(); + expect(doc.frontiersToVV([{ peer: "2", counter: 0 }])).toBeDefined(); + const value = doc.toJSON(); + const frontiers = doc.frontiers(); + const vv = doc.oplogVersion().toJSON(); + const mapRemote = new LoroDoc(); + mapRemote.setPeerId(10); + mapRemote.getMap("map").set("key", -1); + const mapUpdates = mapRemote.export({ mode: "update" }); + let events = 0; + doc.subscribe(() => events++); + expect(() => doc.import(mapUpdates)).toThrow(/cannot parse change block/); + expect(doc.toJSON()).toEqual(value); + expect(doc.frontiers()).toEqual(frontiers); + expect(doc.oplogFrontiers()).toEqual(frontiers); + expect(doc.oplogVersion().toJSON()).toEqual(vv); + expect(doc.isDetached()).toBe(false); + expect(events).toBe(0); + expect(() => doc.checkout(frontiers)).toThrow(/cannot parse change block/); + const healthy = new LoroDoc(); + healthy.getMap("map").set("key", "usable"); + expect(healthy.toJSON()).toEqual({ map: { key: "usable" } }); +}); diff --git a/crates/loro/tests/perf_history_lazy_load.rs b/crates/loro/tests/perf_history_lazy_load.rs index ddf12e526..8f58c83f9 100644 --- a/crates/loro/tests/perf_history_lazy_load.rs +++ b/crates/loro/tests/perf_history_lazy_load.rs @@ -66,3 +66,49 @@ fn perf_history_lazy_load() { ); } } + +/// A one-op, causal map import with a large version vector. Header warming drains +/// the DAG's unparsed_vv without parsing old block bodies. Setup is not timed. +#[test] +#[ignore] +fn perf_map_import_many_peers() { + for peers in [1_000u64, 10_000] { + let source = LoroDoc::new(); + let map = source.get_map("map"); + for peer in 1..=peers { + source.set_peer_id(peer).unwrap(); + map.insert("key", peer as i64).unwrap(); + source.commit(); + } + let old_vv = source.oplog_vv(); + let snapshot = source.export(ExportMode::Snapshot).unwrap(); + source.set_peer_id(peers + 1).unwrap(); + map.insert("key", -1).unwrap(); + let update = source.export(ExportMode::updates(&old_vv)).unwrap(); + for warm_headers in [false, true] { + for repeat in 1..=3 { + let mut elapsed = std::time::Duration::ZERO; + for _ in 0..30 { + let doc = LoroDoc::new(); + doc.import(&snapshot).unwrap(); + if warm_headers { + doc.with_oplog(|oplog| { + for peer in 1..=peers { + black_box(oplog.dag().get_lamport(&ID::new(peer, 0))); + } + }); + } + let start = Instant::now(); + doc.import(black_box(&update)).unwrap(); + elapsed += start.elapsed(); + assert_eq!(doc.state_frontiers(), doc.oplog_frontiers()); + black_box(doc.get_deep_value()); + } + println!( + "peers={peers} warm_headers={warm_headers} repeat={repeat} map_import_us={}", + elapsed.as_micros() + ); + } + } + } +} diff --git a/crates/loro/tests/unregistered_container_parent.rs b/crates/loro/tests/unregistered_container_parent.rs index 6b33be6f9..d16d9934f 100644 --- a/crates/loro/tests/unregistered_container_parent.rs +++ b/crates/loro/tests/unregistered_container_parent.rs @@ -954,6 +954,14 @@ mod random { /// Rewrite a change-block value and both checksum layers, as in the independent /// #1172 verifier. Import must remain lazy: no earlier query records the failure. fn snapshot_with_truncated_history(snapshot: &[u8], block_index: usize) -> (Vec, ID) { + snapshot_with_broken_history(snapshot, block_index, true) +} + +fn snapshot_with_broken_history( + snapshot: &[u8], + block_index: usize, + truncate: bool, +) -> (Vec, ID) { use loro_kv_store::{mem_store::MemKvConfig, MemKvStore}; use std::ops::Bound; let mut at = 22; @@ -978,7 +986,15 @@ fn snapshot_with_truncated_history(snapshot: &[u8], block_index: usize) -> (Vec< }; let (key, bytes) = &blocks[index]; let id = ID::from_bytes(key).inc(1); - kv.set(key, bytes.slice(..bytes.len() / 2)); + if truncate { + kv.set(key, bytes.slice(..bytes.len() / 2)); + } else { + // Keep the header intact; the compressed body fails only when parsed. + let mut body = bytes.to_vec(); + let at = body.len() - 3; + body[at] ^= 0xff; + kv.set(key, body.into()); + } sections[0] = kv.export_all().to_vec(); let mut forged = snapshot[..22].to_vec(); for section in sections { @@ -990,6 +1006,68 @@ fn snapshot_with_truncated_history(snapshot: &[u8], block_index: usize) -> (Vec< (forged, id) } +#[test] +fn register_only_import_rolls_back_when_only_old_block_headers_were_read() { + use loro::Frontiers; + let source = LoroDoc::new(); + source.set_peer_id(1).unwrap(); + source.set_change_merge_interval(-1); + for i in 0..600 { + source + .get_text("text") + .insert(0, &format!("line-{i:04}-xxxxxxxxxxxxxxxxxxxxxxxx")) + .unwrap(); + source.get_map("map").insert("key", i).unwrap(); + source.commit(); + } + source.set_peer_id(2).unwrap(); + for _ in 0..300 { + source.get_text("text").insert(0, &"y".repeat(30)).unwrap(); + source.commit(); + } + let snapshot = source.export(ExportMode::Snapshot).unwrap(); + let remote = LoroDoc::new(); + remote.set_peer_id(10).unwrap(); + remote.get_map("map").insert("key", -1).unwrap(); + let updates = remote.export(ExportMode::all_updates()).unwrap(); + for block in 0..3 { + let (forged, _) = snapshot_with_broken_history(&snapshot, block, false); + if block == 0 { + if let Some(path) = std::env::var_os("LORO_BODY_FIXTURE") { + std::fs::write(path, &forged).unwrap(); + } + } + let doc = LoroDoc::new(); + doc.import(&forged).unwrap(); + // These load every DAG header, draining unparsed_vv, but not block bodies. + for peer in [1, 2] { + assert!(doc + .try_frontiers_to_vv(&Frontiers::from(ID::new(peer, 0))) + .unwrap() + .is_some()); + } + let value = doc.get_deep_value(); + let frontiers = doc.state_frontiers(); + let vv = doc.oplog_vv(); + assert_eq!(doc.oplog_frontiers(), frontiers); + let error = doc.import(&updates).unwrap_err(); + assert!( + error.to_string().contains("cannot parse change block"), + "block={block}: {error}" + ); + assert_eq!(doc.get_deep_value(), value, "block={block}"); + assert_eq!(doc.state_frontiers(), frontiers, "block={block}"); + assert_eq!(doc.oplog_frontiers(), frontiers, "block={block}"); + assert_eq!(doc.oplog_vv(), vv, "block={block}"); + assert!(!doc.is_detached(), "block={block}"); + assert!(doc + .checkout(&frontiers) + .unwrap_err() + .to_string() + .contains("cannot parse change block")); + } +} + #[test] fn fallible_history_readers_reject_a_truncated_block_on_the_first_read() { use loro::{Frontiers, VersionVector};