From 2ee7589474ad45967c8468cb8cb99a3fc879a9ca Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 22:05:27 +0100 Subject: [PATCH 1/5] A version healthy on another node is not held for one node's failed health check (ADR 0557, issue 1496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On ace, 2026-10-26 21:19, a newly assigned mesh-build-node failed its first health check at 8a98386f (later 31be2f24) while shanks, g14 and novox had run it healthy for an hour. Being first by name, ace was the canary, and the assigner held the version and moved the three healthy nodes back. A failed health check of the newest fingerprint now holds it only when no other node's have names it healthy (ADR 0557 §2 keeps ADR 0429 §6's hold and set-back for that case). Otherwise the failure is that node's (ADR 0557 §1): nothing is held or set back, the node keeps its want, and the assigner raises the node's condition once (failed-alone..), naming the node, the module, the fingerprint and the have's words, and clears it once when it ends; the first pass after a start clears what a previous process may have raised. --- .../cmd/mesh-delivery-assigner/pass.go | 43 +++++++++++++++++++ .../cmd/mesh-delivery-assigner/pass_test.go | 24 +++++++++++ .../internal/assign/asks.go | 23 ++++++++++ .../internal/assign/wants.go | 42 ++++++++++++++++-- .../internal/assign/wants_test.go | 13 ++++++ 5 files changed, 141 insertions(+), 4 deletions(-) diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go index 45c3e77..9480faa 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -39,6 +39,9 @@ type passer struct { // noWant is, by node and module, the reason last said why the node gets no want for the module (issue 1945): a reason is // said in the journal when it appears or changes, and its clearing when the node has a want again; one line per change. noWant map[string]string + // nodeFailed is, by node and module, the words of the condition last raised for a failure that is the node's own (ADR 0557 + // §1, issue 1496): raised once per words, cleared once when it ends. + nodeFailed map[string]string } func (p *passer) clock() time.Time { @@ -156,6 +159,7 @@ func (p *passer) wants(ctx context.Context) { gatherNeither(neither, name, out) leftNeither(left, m, wanted[name], out) p.sayNoWant(name, m.Nodes, wanted[name], out) + p.sayNodeFailed(name, fp, m.Nodes, out, first) if first { startClears(stale, m, out) if fp != "" && !m.Held && out.Held == nil && !out.Lifted { @@ -230,6 +234,45 @@ func (p *passer) sayNoWant(module string, nodes []string, before map[string]deli } } +// sayNodeFailed raises, once per node and words, the condition of a failed health check of a module's newest fingerprint fp +// that is the node's own, since fp runs healthy elsewhere and is not held (ADR 0557 §1, issue 1496), and says it in the journal; +// it clears the condition once when the node no longer fails alone. The first pass after a start clears it for every node that +// does not fail alone, since a previous process may have raised it (a clear of a condition not open is quiet at the router). +// A condition the router did not take is said again at the next pass. +func (p *passer) sayNodeFailed(module, fp string, nodes []string, out assign.Outcome, first bool) { + if p.nodeFailed == nil { + p.nodeFailed = map[string]string{} + } + for _, node := range nodes { + key := node + "." + module + f, failing := out.NodeFailed[node] + last, said := p.nodeFailed[key] + switch { + case failing && f.Words+"|"+strings.Join(f.Others, ",") != last: + p.logf("%s on %s: the health check at %s failed here (%s), while it runs healthy on %s: the failure is %s's, so nothing "+ + "is held or set back", module, node, fp, f.Words, strings.Join(f.Others, ", "), node) + if p.say != nil { + if err := p.say(assign.NodeFailedCondition(node, module, fp, f, true)); err != nil { + p.logf("the condition of %s failing on %s alone could not be said, and is said at the next pass: %v", module, node, err) + continue + } + } + p.nodeFailed[key] = f.Words + "|" + strings.Join(f.Others, ",") + case !failing && (said || first): + if said { + p.logf("%s on %s no longer fails its health check alone", module, node) + } + if p.say != nil { + if err := p.say(assign.NodeFailedCondition(node, module, fp, assign.NodeFailure{}, false)); err != nil && said { + p.logf("the condition of %s failing on %s alone could not be cleared, and is cleared at the next pass: %v", module, node, err) + continue + } + } + delete(p.nodeFailed, key) + } + } +} + // neitherOne is a module assigned to a node that holds no want for it after a pass, and the rule holding its first want back. type neitherOne struct { node, module string diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index 7a89fef..c77705b 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -443,6 +443,30 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { t.Errorf("pass %d said %q, want %q", i, got, c.line) } } + // 1496, ADR 0557 §1: a failure that is the node's own (its fingerprint healthy elsewhere, not held) raises the node's condition + // once, naming the node, the module, F and the have's words, and clears it once when it ends. + var conds []asks.Condition + p.say = func(c asks.Condition) error { conds = append(conds, c); return nil } + failing := map[string]assign.NodeFailure{"shanks": {Others: []string{"ace"}, Words: "health failed, phase running: w"}} + for i, c := range []struct { + failed map[string]assign.NodeFailure + state string // the condition said in this pass, "" for none + }{ + {failing, asks.StateRaised}, + {failing, ""}, + {nil, asks.StateCleared}, + {nil, ""}, + } { + conds = nil + p.sayNodeFailed("rofi", fpTest, []string{"shanks"}, assign.Outcome{NodeFailed: c.failed}, false) + switch { + case c.state == "" && len(conds) != 0, c.state != "" && (len(conds) != 1 || conds[0].State != c.state): + t.Errorf("node failed, pass %d said %+v, want %q", i, conds, c.state) + case c.state == asks.StateRaised && (conds[0].Key != "mesh-delivery-assigner.failed-alone.shanks.rofi" || + !strings.Contains(conds[0].Explanation, "failed on shanks (health failed, phase running: w), while it runs healthy on ace")): + t.Errorf("node failed, raised %+v", conds[0]) + } + } } // A neither or held condition a previous process raised is cleared by the first pass after a start: a fresh pass remembers diff --git a/modules/mesh-delivery-assigner/internal/assign/asks.go b/modules/mesh-delivery-assigner/internal/assign/asks.go index 1c7825a..b8125be 100644 --- a/modules/mesh-delivery-assigner/internal/assign/asks.go +++ b/modules/mesh-delivery-assigner/internal/assign/asks.go @@ -81,6 +81,29 @@ func HeldCondition(module, fp, node, words string, raised, needsYou bool) asks.C return c } +// NodeFailedCondition is the condition the assigner raises for a module whose newest fingerprint fp failed its health check on +// node while other nodes run it healthy (ADR 0557 §1, issue 1496): the failure is the node's, so nothing is held or set back and +// the node keeps its want. It names the node, the module, fp and the have's words, keyed by node and module, and is cleared +// (raised false) once the node no longer fails alone. +func NodeFailedCondition(node, module, fp string, f NodeFailure, raised bool) asks.Condition { + key := delivery.AssignerModule + ".failed-alone." + node + "." + module + subject := asks.Scope{Scope: "module", ID: module, Machine: node} + if !raised { + return asks.Condition{Key: key, Subject: subject, State: asks.StateCleared, + Resolved: fmt.Sprintf("%s on %s no longer fails its health check alone.", module, node)} + } + headline := fmt.Sprintf("%s fails on %s alone", module, node) + if n := len([]rune(headline)); n > asks.HeadlineLength { + headline = string([]rune(headline)[:asks.HeadlineLength]) + } + return asks.Condition{Key: key, Severity: asks.SeverityWarning, Subject: subject, State: asks.StateRaised, + Summary: headline, Headline: headline, + Explanation: fmt.Sprintf("%s The health check of %s at %s failed on %s (%s), while it runs healthy on %s. The failure is %s's, "+ + "so the assigner holds nothing and sets no node back, and %s keeps its want. This clears once %s runs it healthy or "+ + "another version is the newest.", asks.NothingToDo, module, short(fp), node, f.Words, strings.Join(f.Others, ", "), node, + node, node)} +} + // NeitherCondition is the condition of a module assigned to node that holds no want for it (issue 1945): the node's own composition // gives up every module with a fingerprint of the new form, so nothing applies it there and its files are gone. It is raised (urgent) // naming the module, the node and the rule holding its first want back with the rule's words, under the assigner's own name keyed by diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 7bc6885..52142fd 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -169,6 +169,9 @@ type Outcome struct { // Lifted is the hold of m.FP lifted by itself: every node that runs the module runs m.FP healthy (issue 1906). The held // record and its ask are taken away, and the wants move by the rules again in this pass. Lifted bool + // NodeFailed is, by node, a failed health check of m.FP on a node while another node runs m.FP healthy: the failure is + // that node's, so m.FP is not held and nothing is set back (ADR 0557 §1, issue 1496). The pass raises its condition once. + NodeFailed map[string]NodeFailure // Record is the moves a person must allow before the want names them: each is asked once (ADR 0539 §14). Record []RecordNeed // Restore is the nodes of a module holding data whose held fingerprint failed its health check there and that are not set @@ -228,11 +231,24 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st } if !held && !out.Lifted && !m.Released && m.FP != "" { for _, node := range slices.Sorted(slices.Values(m.Nodes)) { - if h, ok := n.Haves[node][m.Name]; ok && h.FP == m.FP && failed(h) { - out.Held = &delivery.Held{Node: node, Words: HeldWords(h, m.Now)} - held = true - break + h, ok := n.Haves[node][m.Name] + if !ok || h.FP != m.FP || !failed(h) { + continue } + if others := healthyElsewhere(m, n, node); len(others) > 0 { + // F runs healthy on another node, so its failure here is this node's, not F's: F is not held, no node is set back, + // and the failing node keeps its want (ADR 0557 §1; issue 1496: on ace a newly assigned mesh-build-node failed its + // first check and the hold moved shanks, g14 and novox, healthy on F for an hour, back). The pass raises the node's + // condition. With no healthy witness the hold and the set-back stand (ADR 0557 §2, ADR 0429 §6). + if out.NodeFailed == nil { + out.NodeFailed = map[string]NodeFailure{} + } + out.NodeFailed[node] = NodeFailure{Others: others, Words: HeldWords(h, time.Time{})} // said once: no clock in it + continue + } + out.Held = &delivery.Held{Node: node, Words: HeldWords(h, m.Now)} + held = true + break } } mm := m @@ -419,6 +435,24 @@ func failed(h delivery.Have) bool { return h.Health == delivery.HealthFailed && (h.Phase == delivery.PhaseRunning || h.Phase == delivery.PhaseFailed) } +// healthyElsewhere is every node of m but node that runs m.FP healthy, by name. +func healthyElsewhere(m Module, n Nodes, node string) []string { + var out []string + for _, other := range slices.Sorted(slices.Values(m.Nodes)) { + if other != node && healthy(n.Haves[other][m.Name], m.FP) { + out = append(out, other) + } + } + return out +} + +// NodeFailure is a failed health check of a module's newest fingerprint on one node while it runs healthy on Others: the have's +// health, phase and words (HeldWords without a time), and the healthy witnesses by name. +type NodeFailure struct { + Others []string + Words string +} + // runs says whether a have is of a module the node runs: running, whatever its health. func runs(h delivery.Have) bool { return h.Phase == delivery.PhaseRunning diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index ea4ca59..68701a4 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -150,6 +150,19 @@ func TestAFailedHealthHoldsAndSetsBack(t *testing.T) { if w, ok := out.Wants["g14"]; ok && w.FP == fNew { t.Errorf("g14 moved to a held fingerprint: %+v", w) } + + // 1496, ADR 0557 §1, §2: the same failure while F runs healthy on another node is that node's: F is not held, nothing is set + // back, the failing node keeps its want, and the outcome names it (on ace a newly assigned module's first check held a version healthy for an hour on three nodes). + n.Haves["g14"]["mail"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + before := map[string]delivery.Want{"ace": {FP: fNew, Composed: dig(fNew)}, "g14": {FP: fNew, Composed: dig(fNew)}} + out = Wants(m, n, before, map[string]string{"ace": fOld, "g14": fOld}) + if out.Held != nil || len(out.Wants) != 0 { + t.Fatalf("held %+v, wants %+v: want no hold and no want changed", out.Held, out.Wants) + } + if f := out.NodeFailed["ace"]; !slices.Equal(f.Others, []string{"g14"}) || f.Words != "health failed, phase running: connection refused" || + len(out.NodeFailed) != 1 { + t.Errorf("node failed = %+v", out.NodeFailed) + } } // 1297/B4: a module holding data is not set back; its want stays and says why. From 16e4e61a0f6b6abd3ca44372839bca5646a45963 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 22:16:38 +0100 Subject: [PATCH 2/5] Only an online healthy witness, and only a first failure, keep a version from being held; failed-alone clears when a node leaves (ADR 0557, issue 1496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of 2ee7589: a node that sleeps after running F healthy kept its stale have as a witness, so F stayed unheld while it failed on the online nodes; and a failed-alone condition was never cleared once its node left the module. A witness must now be online by the watchman's presence. F failing on a second node is no longer one node's failure: the hold and the set-back stand (ADR 0557 version 6, §2). A remembered failed-alone key whose node is no longer one of the module's nodes is cleared once; the first pass after a start clears it for every node the presence bucket knows, for a module with a failed have. --- .../cmd/mesh-delivery-assigner/pass.go | 36 ++++++++++++++++--- .../cmd/mesh-delivery-assigner/pass_test.go | 36 ++++++++++++++++++- .../internal/assign/wants.go | 22 +++++++++--- .../internal/assign/wants_test.go | 27 ++++++++++++++ 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go index 9480faa..2652db4 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "maps" "slices" "strings" "time" @@ -159,7 +160,7 @@ func (p *passer) wants(ctx context.Context) { gatherNeither(neither, name, out) leftNeither(left, m, wanted[name], out) p.sayNoWant(name, m.Nodes, wanted[name], out) - p.sayNodeFailed(name, fp, m.Nodes, out, first) + p.sayNodeFailed(name, fp, m.Nodes, nodes, out, first) if first { startClears(stale, m, out) if fp != "" && !m.Held && out.Held == nil && !out.Lifted { @@ -236,14 +237,29 @@ func (p *passer) sayNoWant(module string, nodes []string, before map[string]deli // sayNodeFailed raises, once per node and words, the condition of a failed health check of a module's newest fingerprint fp // that is the node's own, since fp runs healthy elsewhere and is not held (ADR 0557 §1, issue 1496), and says it in the journal; -// it clears the condition once when the node no longer fails alone. The first pass after a start clears it for every node that -// does not fail alone, since a previous process may have raised it (a clear of a condition not open is quiet at the router). -// A condition the router did not take is said again at the next pass. -func (p *passer) sayNodeFailed(module, fp string, nodes []string, out assign.Outcome, first bool) { +// it clears the condition once when the node no longer fails alone, or is no longer one of the module's nodes. The first pass +// after a start remembers nothing a previous process raised: for a module with a failed have on any node it clears the +// condition of every node the presence bucket knows that does not fail alone (a clear of a condition not open is quiet at the +// router). A condition the router did not take is said again at the next pass. +func (p *passer) sayNodeFailed(module, fp string, nodes []string, known assign.Nodes, out assign.Outcome, first bool) { if p.nodeFailed == nil { p.nodeFailed = map[string]string{} } + look := map[string]bool{} for _, node := range nodes { + look[node] = true + } + for key := range p.nodeFailed { + if node, m, _ := strings.Cut(key, "."); m == module { + look[node] = true + } + } + if first && anyFailed(known, module) { + for node := range known.Online { + look[node] = true + } + } + for _, node := range slices.Sorted(maps.Keys(look)) { key := node + "." + module f, failing := out.NodeFailed[node] last, said := p.nodeFailed[key] @@ -273,6 +289,16 @@ func (p *passer) sayNodeFailed(module, fp string, nodes []string, out assign.Out } } +// anyFailed says whether any node's have of module says its health failed. +func anyFailed(n assign.Nodes, module string) bool { + for _, haves := range n.Haves { + if h, ok := haves[module]; ok && h.Health == delivery.HealthFailed { + return true + } + } + return false +} + // neitherOne is a module assigned to a node that holds no want for it after a pass, and the rule holding its first want back. type neitherOne struct { node, module string diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index c77705b..caebcbd 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -458,7 +458,7 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { {nil, ""}, } { conds = nil - p.sayNodeFailed("rofi", fpTest, []string{"shanks"}, assign.Outcome{NodeFailed: c.failed}, false) + p.sayNodeFailed("rofi", fpTest, []string{"shanks"}, assign.Nodes{}, assign.Outcome{NodeFailed: c.failed}, false) switch { case c.state == "" && len(conds) != 0, c.state != "" && (len(conds) != 1 || conds[0].State != c.state): t.Errorf("node failed, pass %d said %+v, want %q", i, conds, c.state) @@ -467,6 +467,40 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { t.Errorf("node failed, raised %+v", conds[0]) } } + // D2: a node that leaves the module's nodes while it failed alone is cleared once; the first pass after a start clears every + // node the presence bucket knows, for a module with a failed have, and nothing for one without. + conds = nil + p.sayNodeFailed("rofi", fpTest, []string{"shanks"}, assign.Nodes{}, assign.Outcome{NodeFailed: failing}, false) + p.sayNodeFailed("rofi", fpTest, nil, assign.Nodes{}, assign.Outcome{}, false) + p.sayNodeFailed("rofi", fpTest, nil, assign.Nodes{}, assign.Outcome{}, false) + if len(conds) != 2 || conds[1].State != asks.StateCleared || conds[1].Key != "mesh-delivery-assigner.failed-alone.shanks.rofi" { + t.Errorf("a node leaving the module said %+v, want one raise and one clear", conds) + } + known := assign.Nodes{Online: map[string]bool{"ace": true, "g14": false}, + Haves: map[string]map[string]delivery.Have{"g14": {"rofi": {FP: fpTest, Health: delivery.HealthFailed}}}} + for _, c := range []struct { + name string + known assign.Nodes + keys []string + }{ + {"a failed have", known, []string{"mesh-delivery-assigner.failed-alone.ace.rofi", "mesh-delivery-assigner.failed-alone.g14.rofi"}}, + {"no failed have", assign.Nodes{Online: known.Online}, nil}, + } { + conds = nil + fresh := newPasser(buckets{}, time.Date(2026, 10, 26, 22, 0, 0, 0, time.UTC)) + fresh.logf = func(string, ...any) {} + fresh.say = func(c asks.Condition) error { conds = append(conds, c); return nil } + fresh.sayNodeFailed("rofi", fpTest, nil, c.known, assign.Outcome{}, true) + var keys []string + for _, cond := range conds { + if cond.State == asks.StateCleared { + keys = append(keys, cond.Key) + } + } + if !slices.Equal(keys, c.keys) || len(conds) != len(c.keys) { + t.Errorf("first pass, %s: said %+v, want clears of %q", c.name, conds, c.keys) + } + } } // A neither or held condition a previous process raised is cleared by the first pass after a start: a fresh pass remembers diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 52142fd..9956065 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -235,11 +235,12 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st if !ok || h.FP != m.FP || !failed(h) { continue } - if others := healthyElsewhere(m, n, node); len(others) > 0 { + if others := healthyElsewhere(m, n, node); len(others) > 0 && !failedElsewhere(m, n, node) { // F runs healthy on another node, so its failure here is this node's, not F's: F is not held, no node is set back, // and the failing node keeps its want (ADR 0557 §1; issue 1496: on ace a newly assigned mesh-build-node failed its // first check and the hold moved shanks, g14 and novox, healthy on F for an hour, back). The pass raises the node's - // condition. With no healthy witness the hold and the set-back stand (ADR 0557 §2, ADR 0429 §6). + // condition. With no online healthy witness, or with F failed on a second node, the hold and the set-back stand (ADR 0557 + // §2, ADR 0429 §6). if out.NodeFailed == nil { out.NodeFailed = map[string]NodeFailure{} } @@ -435,17 +436,30 @@ func failed(h delivery.Have) bool { return h.Health == delivery.HealthFailed && (h.Phase == delivery.PhaseRunning || h.Phase == delivery.PhaseFailed) } -// healthyElsewhere is every node of m but node that runs m.FP healthy, by name. +// healthyElsewhere is every online node of m but node that runs m.FP healthy, by name. func healthyElsewhere(m Module, n Nodes, node string) []string { var out []string for _, other := range slices.Sorted(slices.Values(m.Nodes)) { - if other != node && healthy(n.Haves[other][m.Name], m.FP) { + // A witness is online: a node that sleeps keeps the have it last wrote, and a stale have is no evidence that F runs + // healthy now. + if other != node && n.Online[other] && healthy(n.Haves[other][m.Name], m.FP) { out = append(out, other) } } return out } +// failedElsewhere says whether another node of m has a have naming m.FP failed: then the failure is not one node's alone, and +// the hold and the set-back stand at the second failure (ADR 0557 §2). +func failedElsewhere(m Module, n Nodes, node string) bool { + for _, other := range m.Nodes { + if h, ok := n.Haves[other][m.Name]; ok && other != node && h.FP == m.FP && failed(h) { + return true + } + } + return false +} + // NodeFailure is a failed health check of a module's newest fingerprint on one node while it runs healthy on Others: the have's // health, phase and words (HeldWords without a time), and the healthy witnesses by name. type NodeFailure struct { diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 68701a4..da9cdbe 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -163,6 +163,33 @@ func TestAFailedHealthHoldsAndSetsBack(t *testing.T) { len(out.NodeFailed) != 1 { t.Errorf("node failed = %+v", out.NodeFailed) } + + // ADR 0557 §2: a witness that is offline keeps the have it last wrote and witnesses nothing (D1); F failing on a second node is + // not one node's failure (D3). Either way F is held and ace is set back. + three := m + three.Nodes = []string{"ace", "g14", "novox"} + for _, c := range []struct { + name string + online map[string]bool + novox delivery.Health + }{ + {"the witness sleeps", map[string]bool{"ace": true, "g14": false, "novox": true}, delivery.HealthOK}, + {"F fails on a second node", map[string]bool{"ace": true, "g14": true, "novox": true}, delivery.HealthFailed}, + } { + n.Online = c.online + n.Haves["novox"] = map[string]delivery.Have{"mail": {FP: fNew, Phase: delivery.PhaseRunning, Health: c.novox}} + if c.novox == delivery.HealthOK { + n.Haves["novox"]["mail"] = delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + } + before["novox"] = delivery.Want{FP: fNew, Composed: dig(fNew)} + out = Wants(three, n, before, map[string]string{"ace": fOld, "g14": fOld, "novox": fOld}) + if out.Held == nil || out.Held.Node != "ace" || len(out.NodeFailed) != 0 { + t.Errorf("%s: held %+v, node failed %+v: want F held at ace", c.name, out.Held, out.NodeFailed) + } + if w := out.Wants["ace"]; w.FP != fOld { + t.Errorf("%s: ace's want = %+v, want set back", c.name, w) + } + } } // 1297/B4: a module holding data is not set back; its want stays and says why. From 92aedb936582d830821f8ef8af1655037bc084ad Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 22:21:00 +0100 Subject: [PATCH 3/5] The first pass after a start clears failed-alone for every module and known node (ADR 0557, issue 1496) Review round 2 of 16e4e61: gating the first-pass clear on a failed have left a condition an earlier process raised standing for good once its node recovered while no assigner watched. The first pass now clears it for every module and every node the presence bucket knows, a quiet clear at the router for one that is not open. --- .../cmd/mesh-delivery-assigner/pass.go | 18 ++++-------------- .../cmd/mesh-delivery-assigner/pass_test.go | 4 ++-- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go index 2652db4..2996684 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -238,9 +238,9 @@ func (p *passer) sayNoWant(module string, nodes []string, before map[string]deli // sayNodeFailed raises, once per node and words, the condition of a failed health check of a module's newest fingerprint fp // that is the node's own, since fp runs healthy elsewhere and is not held (ADR 0557 §1, issue 1496), and says it in the journal; // it clears the condition once when the node no longer fails alone, or is no longer one of the module's nodes. The first pass -// after a start remembers nothing a previous process raised: for a module with a failed have on any node it clears the -// condition of every node the presence bucket knows that does not fail alone (a clear of a condition not open is quiet at the -// router). A condition the router did not take is said again at the next pass. +// after a start remembers nothing a previous process raised: it clears, for every module, the condition of every node the +// presence bucket knows that does not fail alone, since a node that recovered while no process watched leaves no failed have to +// find it by (a clear of a condition not open is quiet at the router). A condition the router did not take is said again at the next pass. func (p *passer) sayNodeFailed(module, fp string, nodes []string, known assign.Nodes, out assign.Outcome, first bool) { if p.nodeFailed == nil { p.nodeFailed = map[string]string{} @@ -254,7 +254,7 @@ func (p *passer) sayNodeFailed(module, fp string, nodes []string, known assign.N look[node] = true } } - if first && anyFailed(known, module) { + if first { for node := range known.Online { look[node] = true } @@ -289,16 +289,6 @@ func (p *passer) sayNodeFailed(module, fp string, nodes []string, known assign.N } } -// anyFailed says whether any node's have of module says its health failed. -func anyFailed(n assign.Nodes, module string) bool { - for _, haves := range n.Haves { - if h, ok := haves[module]; ok && h.Health == delivery.HealthFailed { - return true - } - } - return false -} - // neitherOne is a module assigned to a node that holds no want for it after a pass, and the rule holding its first want back. type neitherOne struct { node, module string diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index caebcbd..e344555 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -468,7 +468,7 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { } } // D2: a node that leaves the module's nodes while it failed alone is cleared once; the first pass after a start clears every - // node the presence bucket knows, for a module with a failed have, and nothing for one without. + // node the presence bucket knows, for every module, failed have or not (a node that recovered unwatched leaves none). conds = nil p.sayNodeFailed("rofi", fpTest, []string{"shanks"}, assign.Nodes{}, assign.Outcome{NodeFailed: failing}, false) p.sayNodeFailed("rofi", fpTest, nil, assign.Nodes{}, assign.Outcome{}, false) @@ -484,7 +484,7 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { keys []string }{ {"a failed have", known, []string{"mesh-delivery-assigner.failed-alone.ace.rofi", "mesh-delivery-assigner.failed-alone.g14.rofi"}}, - {"no failed have", assign.Nodes{Online: known.Online}, nil}, + {"no failed have", assign.Nodes{Online: known.Online}, []string{"mesh-delivery-assigner.failed-alone.ace.rofi", "mesh-delivery-assigner.failed-alone.g14.rofi"}}, } { conds = nil fresh := newPasser(buckets{}, time.Date(2026, 10, 26, 22, 0, 0, 0, time.UTC)) From 50c4674dfa234a49b3917601398a1f4598d37f57 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 22:30:00 +0100 Subject: [PATCH 4/5] =?UTF-8?q?While=20a=20node=20fails=20a=20version=20al?= =?UTF-8?q?one,=20no=20further=20node=20gets=20a=20want=20for=20it=20(ADR?= =?UTF-8?q?=200557=20=C2=A72,=20issue=201496)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0557 version 9: a failure that is one node's holds nothing and sets nothing back, but the version reaches no node that does not want it yet until it is over. A node whose want does not name F gets no new want naming it (rule 2, in words naming the failing nodes); wants already written stay. --- .../internal/assign/wants.go | 12 ++++++++++ .../internal/assign/wants_test.go | 23 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 9956065..2903a82 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -2,6 +2,7 @@ package assign import ( "fmt" + "maps" "slices" "sort" "strings" @@ -315,6 +316,11 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st ok, target = true, have.FP } } + if ok && target == m.FP && len(out.NodeFailed) > 0 && !(had && !before.Removed && before.FP == m.FP) { + // While a node fails F alone, F reaches no further node: a node whose want does not name F yet gets no new want + // naming it, and wants already written stay (ADR 0557 §2). + ok, rule, words = false, delivery.RuleNotHeld, FailedAloneWords(m.Name, m.FP, slices.Sorted(maps.Keys(out.NodeFailed))) + } switch { case ok: d, _ := m.composition(node, target) @@ -460,6 +466,12 @@ func failedElsewhere(m Module, n Nodes, node string) bool { return false } +// FailedAloneWords say why a node gets no new want naming fp: fp fails its health check on the nodes named, alone. +func FailedAloneWords(module, fp string, failing []string) string { + return fmt.Sprintf("%s at %s fails its health check on %s, so no further node moves to it until that is over", module, short(fp), + strings.Join(failing, ", ")) +} + // NodeFailure is a failed health check of a module's newest fingerprint on one node while it runs healthy on Others: the have's // health, phase and words (HeldWords without a time), and the healthy witnesses by name. type NodeFailure struct { diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index da9cdbe..3e6985b 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -164,10 +164,29 @@ func TestAFailedHealthHoldsAndSetsBack(t *testing.T) { t.Errorf("node failed = %+v", out.NodeFailed) } - // ADR 0557 §2: a witness that is offline keeps the have it last wrote and witnesses nothing (D1); F failing on a second node is - // not one node's failure (D3). Either way F is held and ace is set back. + // ADR 0557 §2 (version 9): while one node fails F alone, novox, whose want names fOld, gets no new want naming F; the wants + // at F stay. The canary ace runs F healthy here and g14 fails it, so the canary rule (3) would let novox move: only §2 holds it. three := m three.Nodes = []string{"ace", "g14", "novox"} + failedHave := n.Haves["ace"]["mail"] + n.Haves["ace"]["mail"], n.Haves["g14"]["mail"] = n.Haves["g14"]["mail"], failedHave + n.Online["novox"] = true + n.Haves["novox"] = map[string]delivery.Have{"mail": {FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}} + before["novox"] = delivery.Want{FP: fOld, Composed: dig(fOld)} + out = Wants(three, n, before, map[string]string{"ace": fOld, "g14": fOld, "novox": fOld}) + if _, alone := out.NodeFailed["g14"]; !alone { + t.Errorf("g14 does not fail alone: %+v", out.NodeFailed) + } + if w, written := out.Wants["novox"]; out.Held != nil || len(out.NodeFailed) != 1 || (written && w.FP == fNew) || out.Rules["novox"].Rule != delivery.RuleNotHeld { + t.Errorf("held %+v, node failed %+v, novox's want %+v (%+v): want novox kept off F", out.Held, out.NodeFailed, out.Wants["novox"], out.Rules["novox"]) + } + if _, written := out.Wants["g14"]; written { + t.Errorf("g14's want at F was rewritten: %+v", out.Wants["g14"]) + } + n.Haves["ace"]["mail"], n.Haves["g14"]["mail"] = failedHave, n.Haves["ace"]["mail"] + + // ADR 0557 §2: a witness that is offline keeps the have it last wrote and witnesses nothing (D1); F failing on a second node is + // not one node's failure (D3). Either way F is held and ace is set back. for _, c := range []struct { name string online map[string]bool From f007ca42279a77058d59ca3ef106a160a2cb1cc8 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 22:34:25 +0100 Subject: [PATCH 5/5] =?UTF-8?q?A=20node=20that=20runs=20a=20version=20alon?= =?UTF-8?q?e-failed=20elsewhere=20still=20gets=20its=20want=20at=20it=20(A?= =?UTF-8?q?DR=200557=20=C2=A72,=20issue=201496)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of 50c4674: the bar on new wants naming F while a node fails it alone also caught a node that runs F with no live want, the issue-1945 recovery path: it went on neither list and its node-engine gave the module up. The bar now holds only a node that neither wants nor runs F. --- modules/mesh-delivery-assigner/internal/assign/wants.go | 8 +++++--- .../mesh-delivery-assigner/internal/assign/wants_test.go | 7 +++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 2903a82..bd736c6 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -316,9 +316,11 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st ok, target = true, have.FP } } - if ok && target == m.FP && len(out.NodeFailed) > 0 && !(had && !before.Removed && before.FP == m.FP) { - // While a node fails F alone, F reaches no further node: a node whose want does not name F yet gets no new want - // naming it, and wants already written stay (ADR 0557 §2). + if ok && target == m.FP && len(out.NodeFailed) > 0 && !(had && !before.Removed && before.FP == m.FP) && + n.Haves[node][m.Name].FP != m.FP { + // While a node fails F alone, F reaches no further node: a node that neither wants nor runs F gets no new want + // naming it, and wants already written stay (ADR 0557 §2). A node that runs F with no live want gets its want at F, or + // it would be on neither list and its node-engine would give the module up (issue 1945). ok, rule, words = false, delivery.RuleNotHeld, FailedAloneWords(m.Name, m.FP, slices.Sorted(maps.Keys(out.NodeFailed))) } switch { diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 3e6985b..ff629e2 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -183,6 +183,13 @@ func TestAFailedHealthHoldsAndSetsBack(t *testing.T) { if _, written := out.Wants["g14"]; written { t.Errorf("g14's want at F was rewritten: %+v", out.Wants["g14"]) } + // A node that runs F healthy with no want, while g14 fails it alone, gets its want at F and is on no neither list (issue 1945). + n.Haves["novox"]["mail"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + delete(before, "novox") + out = Wants(three, n, before, map[string]string{"ace": fOld, "g14": fOld, "novox": fOld}) + if w := out.Wants["novox"]; w.FP != fNew || len(out.Neither) != 0 { + t.Errorf("novox running F with no want: want %+v, neither %+v; want its want at F and no neither", w, out.Neither) + } n.Haves["ace"]["mail"], n.Haves["g14"]["mail"] = failedHave, n.Haves["ace"]["mail"] // ADR 0557 §2: a witness that is offline keeps the have it last wrote and witnesses nothing (D1); F failing on a second node is