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.