diff --git a/context/code/delivery.md b/context/code/delivery.md index 8f8e10c8..a9330ba4 100644 --- a/context/code/delivery.md +++ b/context/code/delivery.md @@ -136,7 +136,10 @@ unordered, so a notice from an epoch other than `seen`'s cannot be placed. Readi cleared it. Now such a notice sets `stale` and nothing else, the next request is answered by the live host, and that answer (`apply_answer`) is what moves `seen`. Two cheaper guards stay: a notice from an older generation of the host the shape is on, or from an epoch it has left, is ignored without the extra -round; and a duplicate answer clears a `restructured` mark it covers. A notice the client asked for +round; and a duplicate answer clears a `restructured` mark it covers. The last is defensive: no test +reaches it now that `seen` cannot be rewound, and it is kept because a covered answer that cannot clear a mark +it covers is the shape of both livelocks. Merging notices cut for different shapes, or a cut one with an +uncut one, is refused. A notice the client asked for (`poll`) skips the left-epoch test, since the host that answers is live. `poll` hands a pulled notice only to the shape it asked about, and the notice says so: `for_shape` sets diff --git a/context/test-evidence/delivery.md b/context/test-evidence/delivery.md index c8e7cf3d..13733493 100644 --- a/context/test-evidence/delivery.md +++ b/context/test-evidence/delivery.md @@ -202,8 +202,11 @@ swap's notice delivered three steps late, an old reply replayed). The client had the generation-1 notice arrived: `seen` was rewound into generation 1, a notice from the same history set `restructured`, and the window correction that answered was covered by the watermark, so it never cleared the mark and `poll` retried until `Exhausted`. The test replays the history step for step, with the -property's own weather and a Flaky edge. Rival: the notice-ignore test in `accept` disabled; this test -and the random property fail. The seed file is not committed (`proptest-regressions/` is ignored). +property's own weather and a Flaky edge. Rival at the time: the notice-ignore test in `accept` disabled; +this test and the random property failed. **Since the round-three change (a foreign notice only marks the +shape stale) nothing fails with that shortcut disabled**: it is an optimisation that saves one round, not a +correctness rule. The test still replays the seed. The seed file is not committed +(`proptest-regressions/` is ignored). Run after the fix, the property passed 4 x 4000 cases and 2 x 20000 cases, with no third failure. @@ -221,9 +224,15 @@ The same rewind, then an unsolicited covered correction: the duplicate branch se below a queued entry, and each fetch for that `through` came back a duplicate. Failed on the head before the change. -### a_notice_from_another_epoch_marks_the_shape_stale_and_moves_nothing_else +### a_foreign_notice_marks_the_shape_stale_and_the_home_hosts_answer_settles_it -The new rule itself: a foreign notice sets `stale`; the home host's next notice is still contiguous. +A foreign notice sets `stale`, and the home host's answer then settles the shape. It does not see `seen` +stay put (an answer heals it either way); route A and route B are what pin that. + +### notices_cut_for_different_shapes_or_not_cut_do_not_merge + +A merge of notices cut for different shapes, or a cut one with an uncut one, would keep one shape's +mark and make the other shape see a gap. `merge` refuses it. Rival: the `cut_for` comparison removed. ### a_pulled_notice_pushed_through_on_notice_still_reaches_only_its_shape diff --git a/crates/improvise-protocol/src/delivery/notice.rs b/crates/improvise-protocol/src/delivery/notice.rs index 2530f960..d56e824e 100644 --- a/crates/improvise-protocol/src/delivery/notice.rs +++ b/crates/improvise-protocol/src/delivery/notice.rs @@ -182,12 +182,11 @@ impl Notice { /// `self` followed by `next`, if `next` starts where `self` ends. Anything else /// has a gap or another epoch in it, and merging would hide a loss. pub fn merge(mut self, next: Notice) -> Result> { - let adjacent = self.range.end() == next.range.start(); + let adjacent = self.range.end() == next.range.start() && self.cut_for == next.cut_for; match adjacent { false => Err(Box::new((self, next))), true => { self.range.to = next.range.to; - self.cut_for = self.cut_for.take().or(next.cut_for); self.axis.extend(next.axis); self.relabels.extend(next.relabels); self.flags = self.flags.union(next.flags); @@ -275,6 +274,16 @@ mod tests { assert!(merged.moved.contains_key(&ShapeKey("b".into()))); } + #[test] + fn notices_cut_for_different_shapes_or_not_cut_do_not_merge() { + let (a, b) = (ShapeKey("a".into()), ShapeKey("b".into())); + let cut = |p: u64, s: &ShapeKey| notice(p, &[]).for_shape(s); + assert!(cut(1, &a).merge(cut(2, &a)).is_ok()); + assert!(cut(1, &a).merge(cut(2, &b)).is_err()); + assert!(cut(1, &a).merge(notice(2, &[])).is_err()); + assert!(notice(1, &[]).merge(cut(2, &a)).is_err()); + } + #[test] fn a_gap_does_not_merge() { let (a, b) = *notice(3, &[]).merge(notice(5, &[])).unwrap_err(); diff --git a/crates/improvise-protocol/src/delivery/review_tests.rs b/crates/improvise-protocol/src/delivery/review_tests.rs index 1d32c299..e53dbe6a 100644 --- a/crates/improvise-protocol/src/delivery/review_tests.rs +++ b/crates/improvise-protocol/src/delivery/review_tests.rs @@ -275,7 +275,7 @@ fn route_b_a_covered_reply_cannot_set_seen_below_a_queued_entry() { } #[test] -fn a_notice_from_another_epoch_marks_the_shape_stale_and_moves_nothing_else() { +fn a_foreign_notice_marks_the_shape_stale_and_the_home_hosts_answer_settles_it() { let mut c = synced("s", at(1, 0, 4), &[cell(1, "a")]); c.on_notice(&Notice::empty(PubRange::one(at(2, 0, 9)))); assert!(matches!(