From cf4469712a507c35cdeafe218df1f78a6372ac92 Mon Sep 17 00:00:00 2001 From: Paul Masurel Date: Fri, 17 Apr 2026 12:02:40 +0200 Subject: [PATCH 1/2] Adjusts a unit test following a change in behavior. Adjusts the #178 regression test to use a scenario where the sender's gc (120) strictly exceeds the receiver's max (70), which is the condition that actually triggers a reset in practice. --- chitchat/src/state.rs | 71 ++++++++++++++++++++++++++++++++----------- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/chitchat/src/state.rs b/chitchat/src/state.rs index ad78bb6..5bb3ec2 100644 --- a/chitchat/src/state.rs +++ b/chitchat/src/state.rs @@ -158,7 +158,7 @@ impl NodeState { let compatible_without_reset = // Our last GC epoch happened after the delta's peer's node_delta.last_gc_version <= self.last_gc_version || - // We know about all of the operations that happened before this GC. + // We know about all of the operations that happened up to this GC. node_delta.last_gc_version <= self.max_version(); if !compatible_without_reset { @@ -1761,32 +1761,30 @@ mod tests { // Regression test for https://github.com/quickwit-oss/chitchat/issues/178 // - // With the old monotonic property (max(max_version, last_gc_version), max_version), - // a reset delta with last_gc_version=100 and max_version=80 applied to a node - // with max_version=100, last_gc_version=50 would cause: - // before: (100, 100), after: (100, 80) => assertion failure + // A reset delta where last_gc_version > receiver's max_version and + // max_version < receiver's max_version (e.g. due to MTU truncation). // - // The fix: monotonic_property is now (last_gc_version, max_version). Since - // last_gc_version strictly increases on reset, max_version can safely regress - // (e.g. due to MTU truncation) without violating the invariant. + // Receiver: gc=50, max=70. Sender: gc=120, max=80 (truncated by MTU). + // The sender's gc > receiver's max, so this forces a reset. After reset, + // monotonic_property goes from (50, 70) to (120, 80) which is >= because + // the first element (last_gc_version) strictly increased. #[test] fn test_apply_delta_reset_does_not_violate_monotonic_property() { let mut node_state = NodeState::for_test(); node_state.set_with_version("key_a", "val_a", 50); - node_state.set_with_version("key_b", "val_b", 100); + node_state.set_with_version("key_b", "val_b", 70); node_state.last_gc_version = 50; - assert_eq!(node_state.max_version(), 100); + assert_eq!(node_state.max_version(), 70); assert_eq!(node_state.last_gc_version, 50); let monotonic_before = node_state.monotonic_property(); - // A reset delta where last_gc_version > our last_gc_version but - // max_version < our max_version (e.g. due to MTU truncation or - // the sender having a post-reset state with gc > max). + // A reset delta where last_gc_version > our max_version. + // The sender GC'd past our max, so we can't apply without reset. let node_delta = NodeDelta { chitchat_id: node_state.chitchat_id.clone(), from_version_excluded: 0, - last_gc_version: 100, + last_gc_version: 120, max_version: 80, key_values: vec![KeyValueMutation { key: "key_c".to_string(), @@ -1798,15 +1796,13 @@ mod tests { let delta_status = node_state.apply_delta(node_delta, Instant::now()); - // The reset is accepted: last_gc_version advances from 50 to 100, - // so monotonic_property goes from (50, 100) to (100, 80) which is >=. assert_eq!(delta_status, DeltaStatus::ApplyAfterReset); let monotonic_after = node_state.monotonic_property(); - assert_eq!(monotonic_after, (100, 80)); + assert_eq!(monotonic_after, (120, 80)); assert!(monotonic_after >= monotonic_before); assert_eq!(node_state.max_version(), 80); - assert_eq!(node_state.last_gc_version, 100); + assert_eq!(node_state.last_gc_version, 120); // Old keys were wiped by the reset. assert!(node_state.get("key_a").is_none()); assert!(node_state.get("key_b").is_none()); @@ -1814,6 +1810,45 @@ mod tests { assert_eq!(node_state.get("key_c").unwrap(), "val_c"); } + // Similar to test_cluster_state_apply_delta_last_gc_equal_max_does_not_reset, + // but exercises apply_delta at the NodeState level directly. + // + // When A has gc=50, max=70 and B has gc=70, max=100, B's gc equals A's max. + // B decides no reset is needed (digest_max_version < last_gc_version is 70 < 70 = false). + // A must accept this non-reset delta because it has seen every version up to + // the GC boundary. Without the fix (< instead of <=), A would reject the delta + // permanently — neither side would change state, causing a liveness bug. + #[test] + fn test_delta_accepted_when_sender_gc_equals_receiver_max() { + let node_id = ChitchatId::for_local_test(10_001); + + // Node A (receiver): gc=50, max=70 + let mut node_state_a = NodeState::for_test(); + node_state_a.set_with_version("key_a", "val_a", 70); + node_state_a.last_gc_version = 50; + assert_eq!(node_state_a.max_version(), 70); + + // Build the delta that B would send: gc=70, from_version=70, max=100. + // B decided no reset was needed, so from_version_excluded = A's max = 70. + let node_delta = NodeDelta { + chitchat_id: node_id, + from_version_excluded: 70, + last_gc_version: 70, + max_version: 100, + key_values: vec![KeyValueMutation { + key: "key_b".to_string(), + value: "val_b".to_string(), + version: 100, + status: DeletionStatusMutation::Set, + }], + }; + + let delta_status = node_state_a.apply_delta(node_delta, Instant::now()); + assert_eq!(delta_status, DeltaStatus::Apply); + assert_eq!(node_state_a.max_version(), 100); + assert_eq!(node_state_a.get("key_b").unwrap(), "val_b"); + } + #[test] fn test_cluster_state_apply_delta_last_gc_equal_max_does_not_reset() { let mut cluster_state = ClusterState::default(); From e0f64302955b53a038f4d1bd4e19a28167d37ff9 Mon Sep 17 00:00:00 2001 From: Paul Masurel Date: Fri, 17 Apr 2026 13:56:07 +0200 Subject: [PATCH 2/2] Chitchat version bump --- chitchat-test/Cargo.toml | 2 +- chitchat/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/chitchat-test/Cargo.toml b/chitchat-test/Cargo.toml index 14eac9d..9e0c3a1 100644 --- a/chitchat-test/Cargo.toml +++ b/chitchat-test/Cargo.toml @@ -7,7 +7,7 @@ rust-version = "1.86" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -chitchat = { version = "0.10.0", path = "../chitchat" } +chitchat = { version = "0.10.1", path = "../chitchat" } poem = "3.1.11" poem-openapi = {version="5.1.15", features = ["swagger-ui"] } structopt = "0.3" diff --git a/chitchat/Cargo.toml b/chitchat/Cargo.toml index 3a3b7e9..daa9274 100644 --- a/chitchat/Cargo.toml +++ b/chitchat/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "chitchat" -version = "0.10.0" +version = "0.10.1" edition = "2024" license = "MIT" authors = ["Quickwit, Inc. "]