fix(protocol): a reply from an epoch the client has left changes nothing (improvise-z2vr)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
committed by
Edward Langley
parent
62810cb3ba
commit
c2f4753861
@@ -110,10 +110,14 @@ body is a gap unless it starts at or below the watermark. Otherwise `held += tar
|
||||
watermark becomes `through`, and the queued notices it covers are dropped. A late reply older than
|
||||
the watermark therefore cannot regress the state, which is what makes a retry safe.
|
||||
|
||||
**A correction from an older generation of the same host is a duplicate, whatever the watermark.** Epochs
|
||||
of different hosts are not ordered, so a late reply from a dead host is told apart only by being
|
||||
covered: a duplicate answer prunes every queued entry from another epoch and moves `seen` onto the live
|
||||
epoch, so a late notice from a dead host costs one extra round and cannot livelock `sync`.
|
||||
**A correction from a generation or host the client has left is a duplicate, whatever the
|
||||
watermark.** A shape records the epochs its watermark has moved out of (`left`, the newest 64), and a
|
||||
correction from one of them, or from an older generation of the host it is on, changes nothing. Epochs of
|
||||
different hosts are not ordered, so a client that has never applied anything from the newer host cannot
|
||||
tell a late reply from a dead one from a legitimate first answer, and applies it; once it has moved on, the
|
||||
dead host's replies are refused. A duplicate answer also prunes every queued entry from another epoch
|
||||
and moves `seen` onto the live epoch, so a late notice from a dead host costs one extra round and cannot
|
||||
livelock `sync`.
|
||||
|
||||
`poll` hands a pulled notice only to the shape it asked about. A notice carries only that shape's keys,
|
||||
and applying it to another tracked shape would advance that shape past keys it never saw.
|
||||
|
||||
@@ -143,8 +143,32 @@ sign, leading zeros and trailing space.
|
||||
|
||||
Rival: a fetch answered with `through: at`. The first version of the random property did not catch it (it
|
||||
delivered each notice before the next commit); the property now splits, delays, duplicates and replays
|
||||
notices and old replies, and fails the rival on every run, which this test also does by name.
|
||||
notices and old replies, and fails the rival in most runs (17 of 20 at 256 cases, measured on the
|
||||
author's machine); this test fails it on every run, by name.
|
||||
|
||||
### equal_values_compare_equal_whatever_the_zero_counts_say
|
||||
|
||||
`ExactSum`'s equality compared the sign-of-zero counters even when the total was nonzero.
|
||||
|
||||
## Review of #348, round two
|
||||
|
||||
### a_late_window_reply_from_a_dead_host_does_not_regress
|
||||
|
||||
A client that has applied a correction from host 2 receives a late `Window` reply from host 1. Epochs of
|
||||
different hosts are unordered, so the first fix (older generation of the same host) did not cover it.
|
||||
Rival: the `left` check removed; this test and the two below fail.
|
||||
|
||||
### a_reply_from_a_generation_the_client_left_does_not_return
|
||||
|
||||
Generations 1, 2, 3 of one host; a late generation-2 reply. Rival: both guards removed.
|
||||
|
||||
### a_reply_held_across_a_host_restart_cannot_pull_the_client_back
|
||||
|
||||
The host-side form: the reply is computed and lost, the host restarts, the client settles against the new
|
||||
host, and the old reply then arrives. Rival: the `left` check removed.
|
||||
|
||||
**The random property, round two.** The property now checks after every step that the watermark's epoch never goes back to one it had left
|
||||
(so a mid-history regression cannot be repaired by the closing `poll`), and replays an earlier reply after
|
||||
every restart and swap. Rival: the `left` check removed fails it. The older-generation guard on its own is
|
||||
not separable from `left` in the histories the property reaches, so it is held by
|
||||
`a_late_window_reply_from_an_older_generation_does_not_regress` alone.
|
||||
|
||||
@@ -1059,6 +1059,33 @@ fn a_fetch_covers_only_the_notices_it_named_when_a_later_commit_is_already_in_th
|
||||
held_equals_drawn(&client, &host);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_reply_held_across_a_host_restart_cannot_pull_the_client_back() {
|
||||
let mut first = host_instance(1);
|
||||
let id = connect(&mut first);
|
||||
let mut client = settled(&mut first);
|
||||
send(&mut first, id, vec![set("North", 99.0)]);
|
||||
let mut lost = Flaky::scripted(&mut first, vec![Some(Fault::DropReply)]);
|
||||
client.connection_lost();
|
||||
client.sync(&mut lost, &shape(), 6);
|
||||
let in_flight = lost.replies.remove(0);
|
||||
|
||||
let mut second = host_instance(2);
|
||||
client.connection_lost();
|
||||
client.sync(&mut Flaky::new(&mut second), &shape(), 6);
|
||||
held_equals_drawn(&client, &second);
|
||||
let at = client.watermark(&shape()).unwrap();
|
||||
|
||||
let Reply::Correction(late) = in_flight else {
|
||||
panic!("the lost reply was a correction")
|
||||
};
|
||||
assert_eq!(client.apply(&late).0, Outcome::Duplicate);
|
||||
assert_eq!(client.watermark(&shape()), Some(at));
|
||||
held_equals_drawn(&client, &second);
|
||||
let (state, _) = client.sync(&mut Flaky::new(&mut second), &shape(), 6);
|
||||
assert_eq!(state, Synced::Settled);
|
||||
}
|
||||
|
||||
// ── the whole thing, at random ─────────────────────────────────────────────
|
||||
|
||||
mod random {
|
||||
@@ -1253,6 +1280,7 @@ mod random {
|
||||
host.take_notices();
|
||||
let mut held_back: Vec<(usize, Notice)> = Vec::new();
|
||||
let mut old: Vec<Reply> = Vec::new();
|
||||
let mut epochs: Vec<(u64, u64)> = Vec::new();
|
||||
|
||||
for (step, (op, weather)) in steps.iter().enumerate() {
|
||||
match (commands(op), op) {
|
||||
@@ -1288,6 +1316,21 @@ mod random {
|
||||
let mut edge = Flaky::scripted(&mut host, script);
|
||||
client.sync(&mut edge, &shape(), 6);
|
||||
old.append(&mut edge.replies);
|
||||
let moved_host = matches!(op, Op::Restart | Op::Swap);
|
||||
if let (true, Some(Reply::Correction(c))) = (moved_host, old.get(step % old.len().max(1))) {
|
||||
client.apply(c);
|
||||
}
|
||||
if let Some(w) = client.watermark(&shape()) {
|
||||
let now = (w.host, w.generation);
|
||||
let seen_at = epochs.iter().position(|e| *e == now);
|
||||
prop_assert!(
|
||||
seen_at.is_none_or(|i| i + 1 == epochs.len()),
|
||||
"the client went back to a superseded epoch {now:?} after {epochs:?}"
|
||||
);
|
||||
if seen_at.is_none() {
|
||||
epochs.push(now);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let rest: Vec<Notice> = held_back.drain(..).map(|(_, n)| n).collect();
|
||||
|
||||
@@ -53,6 +53,9 @@ pub enum Synced {
|
||||
Exhausted,
|
||||
}
|
||||
|
||||
/// How many abandoned epochs a shape remembers.
|
||||
const LEFT_EPOCHS: usize = 64;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
struct Tracked {
|
||||
window: Window,
|
||||
@@ -63,6 +66,8 @@ struct Tracked {
|
||||
unapplied: Vec<(Stamp, Moved)>,
|
||||
/// The last notice that moved an axis: until a window is read past it, positions are unknown.
|
||||
restructured: Option<Stamp>,
|
||||
/// Epochs the watermark has moved out of: nothing from them applies again.
|
||||
left: Vec<(u64, u64)>,
|
||||
stale: bool,
|
||||
}
|
||||
|
||||
@@ -92,6 +97,7 @@ impl EdgeClient {
|
||||
seen: None,
|
||||
unapplied: Vec::new(),
|
||||
restructured: None,
|
||||
left: Vec::new(),
|
||||
stale: true,
|
||||
},
|
||||
);
|
||||
@@ -246,7 +252,10 @@ impl EdgeClient {
|
||||
let older = |s: Option<Stamp>| {
|
||||
s.is_some_and(|s| s.host == c.through.host && c.through.generation < s.generation)
|
||||
};
|
||||
if older(t.watermark) || older(t.seen) {
|
||||
if older(t.watermark)
|
||||
|| older(t.seen)
|
||||
|| t.left.contains(&(c.through.host, c.through.generation))
|
||||
{
|
||||
return (Outcome::Duplicate, Vec::new());
|
||||
}
|
||||
let covered = t
|
||||
@@ -285,6 +294,11 @@ impl EdgeClient {
|
||||
}
|
||||
};
|
||||
t.held += δ
|
||||
if let Some(w) = t.watermark.filter(|w| !w.same_epoch(&c.through)) {
|
||||
t.left.push((w.host, w.generation));
|
||||
let excess = t.left.len().saturating_sub(LEFT_EPOCHS);
|
||||
t.left.drain(..excess);
|
||||
}
|
||||
t.watermark = Some(c.through);
|
||||
let window = matches!(c.body, CorrectionBody::Window);
|
||||
t.stale &= !window;
|
||||
|
||||
@@ -181,3 +181,23 @@ fn an_item_insert_position_crosses_as_a_wide_integer() {
|
||||
assert!(json.contains(r#""at":"1152921504606846976""#), "{json}");
|
||||
assert_eq!(serde_json::from_str::<AxisChange>(&json).unwrap(), change);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_late_window_reply_from_a_dead_host_does_not_regress() {
|
||||
let mut c = synced("s", at(1, 1, 9), &[cell(1, "old")]);
|
||||
c.apply(&window_correction("s", at(2, 1, 3), &[cell(1, "new")]));
|
||||
let late = window_correction("s", at(1, 1, 12), &[cell(1, "dead")]);
|
||||
assert_eq!(c.apply(&late).0, Outcome::Duplicate);
|
||||
assert_eq!(c.watermark(&shape("s")), Some(at(2, 1, 3)));
|
||||
let kept = wire::decode(&target(&[cell(1, "new")]));
|
||||
assert_eq!(c.held(&shape("s")), Some(&kept));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_reply_from_a_generation_the_client_left_does_not_return() {
|
||||
let mut c = synced("s", at(1, 1, 9), &[cell(1, "old")]);
|
||||
c.apply(&window_correction("s", at(1, 2, 1), &[cell(1, "new")]));
|
||||
c.apply(&window_correction("s", at(1, 3, 1), &[cell(1, "newer")]));
|
||||
let late = window_correction("s", at(1, 2, 7), &[cell(1, "gen2")]);
|
||||
assert_eq!(c.apply(&late).0, Outcome::Duplicate);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user