Assigner: a hold lifts over haves not running, the first pass after a start clears stale conditions, and a first want after a removed want is asked (issue 1945, review round 2 of 3d7d65ed)
mesh/delivery merged: source.main moved to novox/mesh-delivery@653da54c0da3
mesh/check pass 975f0ed65460 on 653da54c0da3 | unit 154 run, 419 kept | integration 164 run, 485 kept | composition passed on ace

Phase guard, same kind as the failed-health hold. allRunHealthy counted
every have whatever its phase, so a node with a removed, held or still
moving have kept a hold up for ever: it counts only phase running (healthy
or not) and failed (not healthy). The failed-health hold already ignores
removed and held haves (phase running or failed only).

Stale conditions (B1). The neither and held conditions this process said
lived only in its memory, so one raised by a previous process stayed urgent
for ever once the module had its want or was no longer held. The first pass
after a start clears, once, the neither condition of every assigned module
with a want and not on neither list, and the held condition of every
module's newest fingerprint that is not held; a clear of a condition that is
not open is quiet at the router. A hold raised again forgets that it was
cleared, so a later release clears it again.

Record ask (review note 3). mustAsk wrote no ask when the want before was
removed, so the first-want path could write a recreates or moves-data
composition of a policy-record module with no ask. A removed want is now
no want, and the first want after it goes through the record ask.

Tests first, each seen failing: a hold lifted over removed, held and moving
haves; the record ask after a removed want; a fresh pass clearing what a
previous process raised, once.
This commit is contained in:
2026-10-25 15:47:57 +01:00
parent 706169aa8a
commit 975f0ed654
4 changed files with 152 additions and 16 deletions
@@ -30,7 +30,8 @@ type passer struct {
asks *composeAsks asks *composeAsks
// now is the clock; nil is time.Now. // now is the clock; nil is time.Now.
now func() time.Time now func() time.Time
// neither is, by node and module, each module on neither list whose condition this process raised (issue 1945). // neither is, by node and module, each module on neither list whose condition this process raised (issue 1945). Nil until the
// first pass after a start, which clears what a previous process may have raised (startClears).
neither map[string]neitherOne neither map[string]neitherOne
} }
@@ -97,7 +98,13 @@ func (p *passer) wants(ctx context.Context) {
names[name] = true names[name] = true
} }
neither := map[string]neitherOne{} neither := map[string]neitherOne{}
defer p.sayNeither(neither) // The first pass after a start remembers nothing a previous process said: a neither or held condition it raised would stay
// raised for ever once the module has its want or is no longer held. That pass clears, once, the neither condition of every
// assigned module with a want and the held condition of every module not held; a clear of a condition that is not open is
// quiet at the router.
first := p.neither == nil
stale := map[string]neitherOne{}
defer func() { p.sayNeither(neither, stale) }()
for name := range names { for name := range names {
fp := trunk[name].FP2 fp := trunk[name].FP2
name := name name := name
@@ -132,6 +139,12 @@ func (p *passer) wants(ctx context.Context) {
} }
out := assign.Wants(m, nodes, wanted[name], last) out := assign.Wants(m, nodes, wanted[name], last)
gatherNeither(neither, name, out) gatherNeither(neither, name, out)
if first {
startClears(stale, m, wanted[name], out)
if fp != "" && !m.Held && out.Held == nil && !out.Lifted {
p.clearHeld(name, fp)
}
}
if out.Held != nil { if out.Held != nil {
key, err := delivery.HeldKey(name, fp) key, err := delivery.HeldKey(name, fp)
if err == nil { if err == nil {
@@ -177,11 +190,32 @@ func gatherNeither(into map[string]neitherOne, module string, out assign.Outcome
} }
} }
// startClears adds, for the first pass after a start, every node of m that holds a want for it after the pass and is not on
// neither list: a neither condition a previous process raised for it is cleared.
func startClears(into map[string]neitherOne, m assign.Module, before map[string]delivery.Want, out assign.Outcome) {
for _, node := range m.Nodes {
w, ok := out.Wants[node]
if !ok {
w, ok = before[node]
}
if _, on := out.Neither[node]; ok && !w.Removed && !on {
into[node+"."+m.Name] = neitherOne{node: node, module: m.Name}
}
}
}
// sayNeither raises, once, the condition of every module on neither list in this pass and writes it to the journal, and clears, // sayNeither raises, once, the condition of every module on neither list in this pass and writes it to the journal, and clears,
// once, the condition of each one said before that has a want again (issue 1945). A say that fails is made again at the next pass. // once, the condition of each one said before that has a want again (issue 1945). A say that fails is made again at the next pass.
func (p *passer) sayNeither(now map[string]neitherOne) { // stale is, on the first pass after a start, the modules whose condition a previous process may have raised and that have a want:
// each is cleared once.
func (p *passer) sayNeither(now, stale map[string]neitherOne) {
if p.neither == nil { if p.neither == nil {
p.neither = map[string]neitherOne{} p.neither = map[string]neitherOne{}
for key, one := range stale {
if _, on := now[key]; !on {
p.neither[key] = one
}
}
} }
for key, one := range now { for key, one := range now {
if _, said := p.neither[key]; said { if _, said := p.neither[key]; said {
@@ -363,7 +397,8 @@ func (p *passer) heldCondition(ctx context.Context, module, fp string, fresh *de
return return
} }
p.said[c.Key+".raised"] = true p.said[c.Key+".raised"] = true
delete(p.said, c.Key+".lifted") // a hold raised again and lifted again is said lifted again delete(p.said, c.Key+".lifted") // a hold raised again and lifted again is said lifted again
delete(p.said, c.Key+".cleared") // and released again is cleared again
} }
// restoreAsk writes, once, the ask about a module holding data whose held fingerprint failed its health check on node. // restoreAsk writes, once, the ask about a module holding data whose held fingerprint failed its health check on node.
@@ -12,6 +12,7 @@ import (
"git.novox.be/novox/mesh-sdk/go/asks" "git.novox.be/novox/mesh-sdk/go/asks"
"git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/delivery"
"git.novox.be/novox/mesh-sdk/go/provider"
"mesh-delivery-assigner/internal/assign" "mesh-delivery-assigner/internal/assign"
) )
@@ -121,17 +122,17 @@ func TestAModuleOnNeitherListIsRaisedOnceAndClearedOnce(t *testing.T) {
p.logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) } p.logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) }
one := map[string]neitherOne{"g14.screen-lock": {node: "g14", module: "screen-lock", one := map[string]neitherOne{"g14.screen-lock": {node: "g14", module: "screen-lock",
holding: assign.Holding{Rule: delivery.RuleProviders, Words: "no provider of x11-display runs healthy on g14 or at its canary"}}} holding: assign.Holding{Rule: delivery.RuleProviders, Words: "no provider of x11-display runs healthy on g14 or at its canary"}}}
p.sayNeither(one) // the say fails p.sayNeither(one, nil) // the say fails
p.sayNeither(one) p.sayNeither(one, nil)
p.sayNeither(one) p.sayNeither(one, nil)
if len(said) != 1 || said[0].State != asks.StateRaised || said[0].Subject.Machine != "g14" || said[0].Subject.ID != "screen-lock" { if len(said) != 1 || said[0].State != asks.StateRaised || said[0].Subject.Machine != "g14" || said[0].Subject.ID != "screen-lock" {
t.Fatalf("said = %+v, want one raised condition of screen-lock on g14", said) t.Fatalf("said = %+v, want one raised condition of screen-lock on g14", said)
} }
if !strings.Contains(strings.Join(logged, "\n"), "screen-lock") { if !strings.Contains(strings.Join(logged, "\n"), "screen-lock") {
t.Errorf("the journal does not name the module on neither list: %q", logged) t.Errorf("the journal does not name the module on neither list: %q", logged)
} }
p.sayNeither(map[string]neitherOne{}) p.sayNeither(map[string]neitherOne{}, nil)
p.sayNeither(map[string]neitherOne{}) p.sayNeither(map[string]neitherOne{}, nil)
if len(said) != 2 || said[1].State != asks.StateCleared || said[1].Key != said[0].Key { if len(said) != 2 || said[1].State != asks.StateCleared || said[1].Key != said[0].Key {
t.Errorf("said = %+v, want the condition cleared once under its key", said) t.Errorf("said = %+v, want the condition cleared once under its key", said)
} }
@@ -146,3 +147,41 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) {
t.Errorf("gathered = %+v", got) t.Errorf("gathered = %+v", got)
} }
} }
// A neither or held condition a previous process raised is cleared by the first pass after a start: a fresh pass remembers
// nothing it said, so it clears, once, the neither condition of every assigned module that has a want and the held condition of
// every module that is not held. Clearing a condition that is not open is quiet at the router.
func TestTheFirstPassAfterAStartClearsWhatAPreviousProcessRaised(t *testing.T) {
wantsKV := &memKV{values: map[string][]byte{}}
p := newPasser(buckets{delivery.AssignerWantsBucket: wantsKV, delivery.AssignerAsksBucket: &memKV{values: map[string][]byte{}}}, time.Now())
var said []asks.Condition
p.say = func(c asks.Condition) error { said = append(said, c); return nil }
mod, _ := delivery.EncodeModule(delivery.Module{FP2: fpTest, Repository: "novox/web", Commit: strings.Repeat("1", 40), Path: delivery.RootPath})
p.w.put(delivery.ModulesBucket, "web", mod)
p.w.put(provider.AssignmentsBucket, "g14.web", []byte(`{"node":"g14","module":"web","build":"b-1"}`))
on, _ := delivery.EncodePresence(delivery.Presence{Online: true})
p.w.put(delivery.PresenceBucket, "g14", on)
have, _ := delivery.EncodeHave(delivery.Have{FP: fpTest, Phase: delivery.PhaseRunning, Health: delivery.HealthOK})
p.w.put(delivery.HaveBucket, "g14.web", have)
key, _ := delivery.WantKey("g14", "web")
want, _ := delivery.EncodeWant(delivery.Want{FP: fpTest, Composed: "sha256:" + strings.Repeat("c", 64)})
p.w.put(delivery.AssignerWantsBucket, key, want)
p.wants(context.Background())
cleared := map[string]bool{}
for _, c := range said {
if c.State != asks.StateCleared {
t.Errorf("the first pass raised %+v", c)
}
cleared[c.Key] = true
}
neither := assign.NeitherCondition("g14", "web", assign.Holding{}, false).Key
held := assign.HeldCondition("web", fpTest, "", "", false, false).Key
if !cleared[neither] || !cleared[held] {
t.Errorf("cleared = %v, want %s and %s", cleared, neither, held)
}
n := len(said)
p.wants(context.Background())
if len(said) != n {
t.Errorf("a second pass said again: %+v", said[n:])
}
}
@@ -262,7 +262,12 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st
next.Backup, next.Words = before.Backup, before.Words // the answer to a failed backup stays while its target stays (ADR 0539 §16) next.Backup, next.Words = before.Backup, before.Words // the answer to a failed backup stays while its target stays (ADR 0539 §16)
} }
running, ran := n.Haves[node][m.Name] running, ran := n.Haves[node][m.Name]
if need, wait := at.mustAsk(node, before, had, ran, running.FP, running.Composed == "", d); wait { // A removed want is no want: a first want after it goes through the record ask as a first want does.
asked, hadLive := before, had
if before.Removed {
asked, hadLive = delivery.Want{}, false
}
if need, wait := at.mustAsk(node, asked, hadLive, ran, running.FP, running.Composed == "", d); wait {
if need != nil { if need != nil {
out.Record = append(out.Record, *need) out.Record = append(out.Record, *need)
} }
@@ -315,7 +320,8 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st
return out return out
} }
// failed says whether a have's health check failed once the node is done moving: phase running or failed. A failed health while it // failed says whether a have's health check failed once the node is done moving: phase running or failed (never removed, held, or
// moving). A failed health while it
// backs up, applies or checks is a step of the move, not its outcome (on novox at 15:27:36 on 2026-10-25, "health failed, phase // backs up, applies or checks is a step of the move, not its outcome (on novox at 15:27:36 on 2026-10-25, "health failed, phase
// applying" held the controller's canary two seconds before the new build was healthy). Every healthy read (healthy, providedHealthy, // applying" held the controller's canary two seconds before the new build was healthy). Every healthy read (healthy, providedHealthy,
// the world's lastHealthy) already asks for phase running. // the world's lastHealthy) already asks for phase running.
@@ -328,15 +334,16 @@ func runs(h delivery.Have) bool {
return h.Phase == delivery.PhaseRunning return h.Phase == delivery.PhaseRunning
} }
// allRunHealthy says whether every node of m that runs the module (has a have of it) runs m.FP healthy, and at least one does: // allRunHealthy says whether every node of m that runs the module (its have's phase is running, or failed) runs m.FP healthy, and
// a hold whose fingerprint runs healthy wherever the module runs holds nothing back any more (issue 1906: seven modules stayed // at least one does: a hold whose fingerprint runs healthy wherever the module runs holds nothing back any more (issue 1906: seven
// held on every machine after one transient failed check). A node set back to another fingerprint does not run m.FP, so its // modules stayed held on every machine after one transient failed check). A node set back to another fingerprint does not run
// hold stays. // m.FP, and a failed one does not run it healthy, so the hold stays; a have that is removed, held for its backup or still moving
// (backing up, applying, checking) is not counted, or it would keep the hold up for ever.
func allRunHealthy(m Module, n Nodes) bool { func allRunHealthy(m Module, n Nodes) bool {
ran := 0 ran := 0
for _, node := range m.Nodes { for _, node := range m.Nodes {
h, ok := n.Haves[node][m.Name] h, ok := n.Haves[node][m.Name]
if !ok { if !ok || (h.Phase != delivery.PhaseRunning && h.Phase != delivery.PhaseFailed) {
continue continue
} }
if !healthy(h, m.FP) { if !healthy(h, m.FP) {
@@ -782,3 +782,58 @@ func TestAFailedHealthWhileTheNodeMovesHoldsNothing(t *testing.T) {
} }
} }
} }
// A hold is lifted by itself once every node that runs the module runs F healthy: a have of a node that is not running the module
// (removed, held for its backup, or still moving) is not counted, so it does not keep the hold up for ever; a node running another
// fingerprint, or failed, still does.
func TestAHoldIsLiftedOverHavesThatAreNotRunning(t *testing.T) {
setup := func(phase delivery.Phase, fp string, health delivery.Health) (Module, Nodes) {
m := Module{Name: "web", FP: fNew, Built: true, Granted: true, Held: true, Composed: composedEverywhere, Nodes: []string{"ace", "g14"}}
n := Nodes{Online: map[string]bool{"ace": true, "g14": true}, Haves: map[string]map[string]delivery.Have{
"ace": {"web": {FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}},
"g14": {"web": {FP: fp, Phase: phase, Health: health}}}}
return m, n
}
for _, phase := range []delivery.Phase{delivery.PhaseRemoved, delivery.PhaseHeld, delivery.PhaseApplying, delivery.PhaseBackingUp, delivery.PhaseChecking} {
m, n := setup(phase, fOld, delivery.HealthUnknown)
if out := Wants(m, n, nil, nil); !out.Lifted {
t.Errorf("g14's have in phase %s kept the hold up", phase)
}
}
for what, h := range map[string]delivery.Have{
"running another fingerprint": {FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK},
"failed": {FP: fNew, Phase: delivery.PhaseFailed, Health: delivery.HealthFailed},
} {
m, n := setup(h.Phase, h.FP, h.Health)
if out := Wants(m, n, nil, nil); out.Lifted {
t.Errorf("g14 %s: the hold was lifted", what)
}
}
// a failed health on a removed have holds nothing
m, n := setup(delivery.PhaseRemoved, fNew, delivery.HealthFailed)
m.Held = false
if out := Wants(m, n, nil, nil); out.Held != nil {
t.Errorf("a removed have with a failed health held F: %+v", out.Held)
}
}
// A module of policy record whose want was removed but which the node still runs gets its first want through the record ask: a
// removed want is no want, so a move the controller judges to recreate or move data waits for the operator's yes.
func TestAFirstWantAfterARemovedWantGoesThroughTheRecordAsk(t *testing.T) {
m, n := x11()
recording(&m)
m.Composed = func(_, fp string) (delivery.ComposedDigest, bool) {
return delivery.ComposedDigest{Digest: dig(fp), Judgement: delivery.JudgedRecreates, Policy: delivery.PolicyRecord}, true
}
out := Wants(m, n, map[string]delivery.Want{"shanks": {Removed: true}}, nil)
if w, ok := out.Wants["shanks"]; ok {
t.Errorf("shanks's first want after a removed want was written with no record ask: %+v", w)
}
asked := false
for _, r := range out.Record {
asked = asked || (r.Node == "shanks" && r.Digest == dig(fOld) && r.Judgement == delivery.JudgedRecreates)
}
if !asked {
t.Errorf("the record asks = %+v, want one for shanks at the composition it runs", out.Record)
}
}