max / synckit
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
4 files changed,
+154 insertions,
-22 deletions
| @@ -534,10 +534,18 @@ | |||
| 534 | 534 | // the only way its members can be guaranteed to describe one device's state. | |
| 535 | 535 | let winner = if local_wins { local_obj } else { remote_obj }; | |
| 536 | 536 | for group in dependent { | |
| 537 | - | let triggered = group | |
| 538 | - | .iter() | |
| 539 | - | .any(|c| local_changed.contains_key(c) && remote_changed.contains_key(c)); | |
| 540 | - | if !triggered { | |
| 537 | + | // Both sides having touched the group *anywhere* is the trigger, not both | |
| 538 | + | // having touched the same member. Requiring the same member was the first | |
| 539 | + | // version of this and it is wrong in the case the whole feature is for: | |
| 540 | + | // one device moves `status` and the other moves `completed_at`, so no | |
| 541 | + | // single column is contested, and the field-by-field pass then takes one | |
| 542 | + | // column from each device. That is the split this prevents, arriving by a | |
| 543 | + | // different route. Found by `a_contested_group_never_lands_split`. | |
| 544 | + | let local_touched = group.iter().any(|c| local_changed.contains_key(c)); | |
| 545 | + | let remote_touched = group.iter().any(|c| remote_changed.contains_key(c)); | |
| 546 | + | if !(local_touched && remote_touched) { | |
| 547 | + | // At most one side moved the group, so base-plus-that-side's-changes | |
| 548 | + | // is already that side's version of it. Nothing to reconcile. | |
| 541 | 549 | continue; | |
| 542 | 550 | } | |
| 543 | 551 | tracing::debug!( | |
| @@ -812,6 +820,100 @@ | |||
| 812 | 820 | "a poisoned local won against an honest remote" | |
| 813 | 821 | ); | |
| 814 | 822 | } | |
| 823 | + | ||
| 824 | + | /// **A field merge converges, dependent groups included.** | |
| 825 | + | /// | |
| 826 | + | /// The two devices see mirror images of one conflict: what is local | |
| 827 | + | /// on A is remote on B. They must compute the same merged object, or | |
| 828 | + | /// they hold different bytes forever with nothing to detect it. | |
| 829 | + | /// | |
| 830 | + | /// This is aimed at the group rule specifically. Everything else in | |
| 831 | + | /// the merge decides a field from values both devices have, but the | |
| 832 | + | /// group rule picks a *side*, and "side" is the one concept that is | |
| 833 | + | /// device-relative. It converges because the winner comes from | |
| 834 | + | /// `resolve_tie` over the two entries rather than from which one the | |
| 835 | + | /// caller happened to label local, and this is what would fail if | |
| 836 | + | /// that ever regressed to a "ties go to local" rule. | |
| 837 | + | #[test] | |
| 838 | + | fn field_merge_converges_on_mirrored_inputs( | |
| 839 | + | (a_hlc, b_hlc) in hlc_pair(), | |
| 840 | + | a_state in 0u8..3, | |
| 841 | + | b_state in 0u8..3, | |
| 842 | + | a_at in 0u8..3, | |
| 843 | + | b_at in 0u8..3, | |
| 844 | + | a_note in 0u8..3, | |
| 845 | + | b_note in 0u8..3, | |
| 846 | + | ) { | |
| 847 | + | const GROUPS: &[&[&str]] = &[&["state", "state_at"]]; | |
| 848 | + | let base = json!({"state": "s0", "state_at": "t0", "note": "n0"}); | |
| 849 | + | let a = json!({ | |
| 850 | + | "state": format!("s{a_state}"), | |
| 851 | + | "state_at": format!("t{a_at}"), | |
| 852 | + | "note": format!("n{a_note}"), | |
| 853 | + | }); | |
| 854 | + | let b = json!({ | |
| 855 | + | "state": format!("s{b_state}"), | |
| 856 | + | "state_at": format!("t{b_at}"), | |
| 857 | + | "note": format!("n{b_note}"), | |
| 858 | + | }); | |
| 859 | + | ||
| 860 | + | // On device A the local side is `a`; on device B it is `b`. | |
| 861 | + | let on_a = resolve_field_merge_with(&a, &b, &base, &a_hlc, &b_hlc, GROUPS); | |
| 862 | + | let on_b = resolve_field_merge_with(&b, &a, &base, &b_hlc, &a_hlc, GROUPS); | |
| 863 | + | ||
| 864 | + | prop_assert_eq!( | |
| 865 | + | format!("{on_a:?}"), | |
| 866 | + | format!("{on_b:?}"), | |
| 867 | + | "two devices merged the same conflict differently and will \ | |
| 868 | + | never converge (a={:?}, b={:?})", | |
| 869 | + | a_hlc, | |
| 870 | + | b_hlc | |
| 871 | + | ); | |
| 872 | + | } | |
| 873 | + | ||
| 874 | + | /// **A declared group never lands split across the two sides.** | |
| 875 | + | /// | |
| 876 | + | /// The property the declaration exists to buy. However the merge | |
| 877 | + | /// resolves, every member of a contested group has to come from one | |
| 878 | + | /// side, so the pair describes a state some device actually held. A | |
| 879 | + | /// merge that decided `state` and `state_at` independently fails this | |
| 880 | + | /// on the inputs where the two sides disagree about only one of them, | |
| 881 | + | /// which is exactly the GoingsOn start()-versus-complete() case. | |
| 882 | + | #[test] | |
| 883 | + | fn a_contested_group_never_lands_split( | |
| 884 | + | (a_hlc, b_hlc) in hlc_pair(), | |
| 885 | + | a_state in 0u8..3, | |
| 886 | + | b_state in 0u8..3, | |
| 887 | + | a_at in 0u8..3, | |
| 888 | + | b_at in 0u8..3, | |
| 889 | + | ) { | |
| 890 | + | const GROUPS: &[&[&str]] = &[&["state", "state_at"]]; | |
| 891 | + | let base = json!({"state": "s0", "state_at": "t0"}); | |
| 892 | + | let a = json!({"state": format!("s{a_state}"), "state_at": format!("t{a_at}")}); | |
| 893 | + | let b = json!({"state": format!("s{b_state}"), "state_at": format!("t{b_at}")}); | |
| 894 | + | ||
| 895 | + | let Resolution::Merged(merged) = | |
| 896 | + | resolve_field_merge_with(&a, &b, &base, &a_hlc, &b_hlc, GROUPS) | |
| 897 | + | else { | |
| 898 | + | return Err(TestCaseError::fail("an object base must merge")); | |
| 899 | + | }; | |
| 900 | + | ||
| 901 | + | // The result's group is allowed to be A's, B's, or the base's | |
| 902 | + | // (untouched). What it must never be is one column from one side | |
| 903 | + | // and the other from a different one. | |
| 904 | + | let pair = (&merged["state"], &merged["state_at"]); | |
| 905 | + | let candidates = [ | |
| 906 | + | (&a["state"], &a["state_at"]), | |
| 907 | + | (&b["state"], &b["state_at"]), | |
| 908 | + | (&base["state"], &base["state_at"]), | |
| 909 | + | ]; | |
| 910 | + | prop_assert!( | |
| 911 | + | candidates.contains(&pair), | |
| 912 | + | "the group landed split: got {:?}, which is no device's version \ | |
| 913 | + | of it (a={a}, b={b})", | |
| 914 | + | merged | |
| 915 | + | ); | |
| 916 | + | } | |
| 815 | 917 | } | |
| 816 | 918 | } | |
| 817 | 919 |
| @@ -1933,6 +1933,54 @@ | |||
| 1933 | 1933 | } | |
| 1934 | 1934 | } | |
| 1935 | 1935 | ||
| 1936 | + | /// The same split by the other route, and the one that was briefly wrong. | |
| 1937 | + | /// | |
| 1938 | + | /// Here no single column is contested: one device moves `state`, the other | |
| 1939 | + | /// moves only `state_at`. A field-by-field pass sees two disjoint changes, | |
| 1940 | + | /// merges both, and lands one column from each device, which is the same | |
| 1941 | + | /// broken pair as the contested case. So the trigger is both sides having | |
| 1942 | + | /// touched the group *anywhere*, not both having touched the same column. | |
| 1943 | + | #[test] | |
| 1944 | + | fn a_group_touched_by_both_sides_on_different_columns_is_still_taken_whole() { | |
| 1945 | + | let (mut conn, n) = dep_device(1); | |
| 1946 | + | let peer = node(2); | |
| 1947 | + | let t0 = card_t0(); | |
| 1948 | + | let s = dep_schema(); | |
| 1949 | + | ||
| 1950 | + | pull_apply_with( | |
| 1951 | + | &mut conn, | |
| 1952 | + | &s, | |
| 1953 | + | n, | |
| 1954 | + | vec![job_change( | |
| 1955 | + | peer, | |
| 1956 | + | t0, | |
| 1957 | + | serde_json::json!({"id": "j1", "state": "pending", "state_at": null, "note": "n"}), | |
| 1958 | + | )], | |
| 1959 | + | ); | |
| 1960 | + | ||
| 1961 | + | // Local moves only `state`; remote moves only `state_at`. Disjoint. | |
| 1962 | + | conn.execute("UPDATE job SET state = 'started' WHERE id = 'j1'", []) | |
| 1963 | + | .unwrap(); | |
| 1964 | + | stamp_pending(&conn, n, t0 + 3_000).unwrap(); | |
| 1965 | + | pull_apply_with( | |
| 1966 | + | &mut conn, | |
| 1967 | + | &s, | |
| 1968 | + | n, | |
| 1969 | + | vec![job_change( | |
| 1970 | + | peer, | |
| 1971 | + | t0 + 2_000, | |
| 1972 | + | serde_json::json!({"id": "j1", "state": "pending", "state_at": "T", "note": "n"}), | |
| 1973 | + | )], | |
| 1974 | + | ); | |
| 1975 | + | ||
| 1976 | + | let (state, state_at, _) = job(&conn); | |
| 1977 | + | assert_eq!( | |
| 1978 | + | (state.as_deref(), state_at.as_deref()), | |
| 1979 | + | (Some("started"), None), | |
| 1980 | + | "the group took one column from each device: a state neither one held" | |
| 1981 | + | ); | |
| 1982 | + | } | |
| 1983 | + | ||
| 1936 | 1984 | /// The group only fires when it is contested. A device that moves the group | |
| 1937 | 1985 | /// while the other moves an unrelated column still gets a merge, which is the | |
| 1938 | 1986 | /// whole reason the table opted in. |
| @@ -1,7 +1,0 @@ | |||
| 1 | - | # Seeds for failure cases proptest has generated in the past. It is | |
| 2 | - | # automatically read and these particular cases re-run before any | |
| 3 | - | # novel cases are generated. | |
| 4 | - | # | |
| 5 | - | # It is recommended to check this file in to source control so that | |
| 6 | - | # everyone who runs the test benefits from these saved cases. | |
| 7 | - | cc 9d9f0ab1b1f55d7fe578e6c112c886bca450128961e27b0531affe544f42a200 # shrinks to (a_hlc, b_hlc) = (Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }), a_payload = 0, b_payload = 1 |
| @@ -1,11 +1,0 @@ | |||
| 1 | - | # Seeds for failure cases proptest has generated in the past. It is | |
| 2 | - | # automatically read and these particular cases re-run before any | |
| 3 | - | # novel cases are generated. | |
| 4 | - | # | |
| 5 | - | # It is recommended to check this file in to source control so that | |
| 6 | - | # everyone who runs the test benefits from these saved cases. | |
| 7 | - | cc f316a64a84523c15582b79ba1bc8d0261f26301d5f6a29ac9721b8341d5e96ab # shrinks to pulled = [PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000026, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r2"), "name": String("v0")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }, PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r2"), "name": String("v1")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }] | |
| 8 | - | cc c2d4aa664438a38c412b938ff4a432018fc9dc6e6ab660133123ae2d04ad71b4 # shrinks to pulled = [PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r2"), "name": String("v1")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }, PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r2"), "name": String("v0")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }] | |
| 9 | - | cc c46b37be05f09c1a28abe12636d31cf4f5fb3ef15bda680dcf69e0717307a04e # shrinks to pulled = [PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r1", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r1"), "name": String("v0")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }, PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r1", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r1"), "name": String("v1")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }] | |
| 10 | - | cc d8ac248ba280b6786222875b5e990824e97b6de9ff67160ae191084c2c31ee5e # shrinks to pulled = [PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000002) }, data: Some(Object {"id": String("r2"), "name": String("v0")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000002), seq: 0 }, PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000002) }, data: Some(Object {"id": String("r2"), "name": String("v1")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000002), seq: 0 }] | |
| 11 | - | cc a6d53a57388c6cabc912f655a77cf52e2c8c449a9300a651edb0ce5c3b45a646 # shrinks to pulled = [PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000002, counter: 0, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r2"), "name": String("v0")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }, PulledChange { entry: ChangeEntry { table: "note", op: Update, row_id: "r2", timestamp: 2023-11-14T22:13:20Z, hlc: Hlc { wall_ms: 1700000000000, counter: 1, node: DeviceId(00000000-0000-0000-0000-000000000000) }, data: Some(Object {"id": String("r2"), "name": String("v1")}), extra: {} }, device_id: DeviceId(00000000-0000-0000-0000-000000000000), seq: 0 }] |