From b32a27c115a2499e339e049e0f120f882c9b3dd9 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:46:05 +0100 Subject: [PATCH] Owe a look when a module's change ends unjudged, so a module with nothing long-running is ok within a look of its apply The facts look inside an apply runs while the module is still applying and judges nothing; the running write then left xorg running and unknown until the five-minute statement, holding every x11-display consumer behind it. The running write now nudges the health loop, which looks and says at once. A declaration's report gets the same motion and composition check, and a removed module leaves nothing marked unjudged (review of fix 15). --- cmd/mesh-host/healthowed_test.go | 90 ++++++++++++++++++++++++++++++++ cmd/mesh-host/main.go | 8 +++ internal/link/have.go | 47 ++++++++++++++--- internal/link/have_test.go | 29 ++++++++++ internal/link/session.go | 20 +++++++ 5 files changed, 187 insertions(+), 7 deletions(-) create mode 100644 cmd/mesh-host/healthowed_test.go diff --git a/cmd/mesh-host/healthowed_test.go b/cmd/mesh-host/healthowed_test.go new file mode 100644 index 0000000..ba4907d --- /dev/null +++ b/cmd/mesh-host/healthowed_test.go @@ -0,0 +1,90 @@ +package main + +import ( + "context" + "errors" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" + "github.com/novox/mesh-host/internal/link" + "github.com/novox/mesh-host/internal/liveness" +) + +// owedBus is a link that takes reports and statements and keeps a have bucket in memory. +type owedBus struct { + mu sync.Mutex + kv map[string][]byte + rv map[string]uint64 +} + +func (b *owedBus) Report(context.Context, string, []byte) error { return nil } +func (b *owedBus) Health(context.Context, string, []byte) error { return nil } +func (b *owedBus) HaveBind(context.Context) (link.HaveBucket, error) { return b, nil } + +func (b *owedBus) Get(_ context.Context, key string) ([]byte, uint64, error) { + b.mu.Lock() + defer b.mu.Unlock() + v, ok := b.kv[key] + if !ok { + return nil, 0, link.ErrHaveAbsent + } + return v, b.rv[key], nil +} + +func (b *owedBus) Set(_ context.Context, key string, value []byte, revision uint64) error { + b.mu.Lock() + defer b.mu.Unlock() + if b.rv[key] != revision { + return errors.New("wrong last sequence") + } + b.kv[key], b.rv[key] = value, revision+1 + return nil +} + +func (b *owedBus) have(key string) (delivery.Have, bool) { + b.mu.Lock() + defer b.mu.Unlock() + h, err := delivery.DecodeHave(b.kv[key]) + return h, err == nil +} + +// A module with nothing long-running (xorg) is ok within one look of its apply, not at the five-minute statement: the +// facts look inside the apply runs while it is applying and judges nothing, and the running write then owes a look, +// which the health loop makes at once (review of fix 15, B1). +func TestAModuleWithNothingLongRunningIsOkWithinALookOfItsApply(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + bus := &owedBus{kv: map[string][]byte{}, rv: map[string]uint64{}} + queue := &link.Session{Membership: link.Membership{Node: "n1"}, Timeout: time.Second} + queue.Attach(ctx, bus) + j, err := liveness.Open(filepath.Join(t.TempDir(), "liveness.json"), nil) + if err != nil { + t.Fatal(err) + } + fact := link.ModuleFact{Module: "xorg", FP: "src2:" + strings.Repeat("a", 64), Composed: "sha256:" + strings.Repeat("b", 64)} + applying := fact + applying.Phase = string(delivery.PhaseApplying) + queue.NoteModule(ctx, applying) + go judgeWhatRuns(ctx, j, queue, func(string) {}, nil) + // The facts look inside the apply, while the module is still applying. + queue.SayFacts(ctx, link.Report{Health: &link.Health{Contract: link.LivenessContract, At: time.Now().UTC()}}) + if h, _ := bus.have("n1.xorg"); h.Health != delivery.HealthUnknown { + t.Fatalf("judged while applying: %+v", h) + } + queue.NoteModule(ctx, fact) + deadline := time.Now().Add(2 * time.Second) // well within one look (liveness.LookEvery, 15 s) + for { + h, _ := bus.have("n1.xorg") + if h.Phase == delivery.PhaseRunning && h.Health == delivery.HealthOK { + return + } + if time.Now().After(deadline) { + t.Fatalf("not ok within a look of its apply: %+v", h) + } + time.Sleep(10 * time.Millisecond) + } +} diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 7b13709..06ebfd9 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -1340,11 +1340,19 @@ func judgeWhatRuns(ctx context.Context, j *liveness.Judge, queue *link.Session, var lastSaid time.Time owed := true spaced := 1.0 + // A module whose change ended unjudged is looked at and said at once (review of fix 15): a module with nothing + // long-running is then ok within a look of its apply, not at the five-minute statement. + var nudged <-chan struct{} + if queue != nil { + nudged = queue.HealthOwed() + } for { select { case <-ctx.Done(): return case <-ticker.C: + case <-nudged: + owed = true } st, changed := j.Look(ctx) owed = owed || changed diff --git a/internal/link/have.go b/internal/link/have.go index 6e9b8f8..d47fdb9 100644 --- a/internal/link/have.go +++ b/internal/link/have.go @@ -77,6 +77,28 @@ func (s *haveState) healthOf(module string, f ModuleFact) delivery.Health { return nothingJudged(f, s.looked) } +// moved marks a module's health not yet judged when its fact f is a change under way, or names another composition than +// the fact before it: the last look judged what ran before. The caller holds the lock. +func (s *haveState) moved(f ModuleFact) { + if prev, known := s.modules[f.Module]; inMotion(f.Phase) || known && (prev.FP != f.FP || prev.Composed != f.Composed) { + delete(s.health, f.Module) + if s.unjudged == nil { + s.unjudged = map[string]bool{} + } + s.unjudged[f.Module] = true + } +} + +// owed says some module left motion and no look judged it since. The caller holds the lock. +func (s *haveState) owed() bool { + for module, f := range s.modules { + if s.unjudged[module] && !inMotion(f.Phase) { + return true + } + } + return false +} + // takeLook keeps a look's health: of a module still in motion nothing, since the look judged what is being replaced; // of every other module what the look found, which is then judged. The caller holds the lock. func (s *haveState) takeLook(h *Health) { @@ -199,10 +221,21 @@ func (q *Session) noteApply(r Report) bool { for module := range next { delete(s.removed, module) // added back: its have is written with its new phase } + for module := range s.modules { + if _, held := next[module]; !held { + delete(s.unjudged, module) + } + } + for _, f := range next { + s.moved(f) + } s.modules = next if r.Health != nil { s.takeLook(r.Health) } + if s.owed() { + q.oweHealth() + } n := nodeFacts(r) s.node = &n return true @@ -418,16 +451,15 @@ func (q *Session) NoteModule(ctx context.Context, f ModuleFact) { // A change under way, or a fact naming another composition than the one looked at, leaves the module's health not // yet judged until a look after it: the last look judged what ran before (2026-10-25, the canary of the controller on // novox held on a have saying applying and failed two seconds before its launch said healthy). - if prev, known := s.modules[f.Module]; inMotion(f.Phase) || known && (prev.FP != f.FP || prev.Composed != f.Composed) { - delete(s.health, f.Module) - if s.unjudged == nil { - s.unjudged = map[string]bool{} - } - s.unjudged[f.Module] = true - } + s.moved(f) s.modules[f.Module] = f delete(s.removed, f.Module) + // Out of motion and not judged: a look is owed now, not at the loop's next statement (review of fix 15, B1). + owe := !inMotion(f.Phase) && s.unjudged[f.Module] s.mu.Unlock() + if owe { + q.oweHealth() + } q.mu.Lock() bus := q.bus q.mu.Unlock() @@ -449,6 +481,7 @@ func (q *Session) NoteRemoved(ctx context.Context, module, build string) { s.removed[module] = build delete(s.modules, module) delete(s.written, module) + delete(s.unjudged, module) s.mu.Unlock() q.mu.Lock() bus := q.bus diff --git a/internal/link/have_test.go b/internal/link/have_test.go index 6706f86..3a87c64 100644 --- a/internal/link/have_test.go +++ b/internal/link/have_test.go @@ -590,3 +590,32 @@ func TestAHealthIsNotCarriedOverToAnotherComposition(t *testing.T) { t.Fatalf("the old composition's look judged the new one: %+v", h) } } + +// A declaration's report naming another composition of a module, with no health of its own, does not carry the old +// look's health over; and a module removed leaves nothing marked unjudged behind. +func TestADeclarationReportMovesAHealthAndARemovalForgetsIt(t *testing.T) { + l := newHaveLink() + q, _ := aHaveQueue(l) + ctx := context.Background() + q.SayFacts(ctx, Report{Applied: []string{"a.x"}, Modules: []ModuleFact{{Module: "a", Build: "b-1"}}, + Health: healthOf(map[string]string{"a": StateUnhealthy})}) + if h := l.have(t, "n1.a"); h.Health != delivery.HealthFailed { + t.Fatalf("the failed look was not said: %+v", h) + } + q.SayFacts(ctx, Report{Applied: []string{"a.x"}, Modules: []ModuleFact{{Module: "a", Build: "b-1", FP: "src2:" + strings.Repeat("c", 64)}}}) + if h := l.have(t, "n1.a"); h.Health != delivery.HealthUnknown { + t.Fatalf("the old composition's look judged the new one: %+v", h) + } + select { + case <-q.HealthOwed(): + default: + t.Fatal("a module out of motion and unjudged owed no look") + } + q.NoteRemoved(ctx, "a", "") + q.haves.mu.Lock() + left := q.haves.unjudged["a"] + q.haves.mu.Unlock() + if left { + t.Fatal("a removed module is still marked unjudged") + } +} diff --git a/internal/link/session.go b/internal/link/session.go index b90f733..0da7943 100644 --- a/internal/link/session.go +++ b/internal/link/session.go @@ -57,6 +57,8 @@ type Session struct { // gone is closed when the link open now closes, and changed whenever a link opens or closes (WhenLinked). gone chan struct{} changed chan struct{} + // owed is HealthOwed's: a module left motion unjudged. + owed chan struct{} // saying is held while a facts event is made and said, so they leave in the order they are made. saying sync.Mutex @@ -76,9 +78,27 @@ func (q *Session) init() { } q.epoch = time.Now().UnixMilli() q.changed = make(chan struct{}) + q.owed = make(chan struct{}, 1) }) } +// HealthOwed is told, at most once until it is read, when a module's change ended and no look has judged it since: the +// health loop looks and says its statement at once rather than at its next tick or five minutes on, so a module with +// nothing long-running is ok within a look of its apply, not five minutes after it (2026-10-25, review of fix 15). +func (q *Session) HealthOwed() <-chan struct{} { + q.init() + return q.owed +} + +// oweHealth nudges the health loop; a nudge already waiting is enough. +func (q *Session) oweHealth() { + q.init() + select { + case q.owed <- struct{}{}: + default: + } +} + // SayFacts says this node's facts on the link open now, in the node-engine's own order, and whether the bus took them. // A facts event made with no link open is not kept: the next link says the facts as they are then. func (q *Session) SayFacts(ctx context.Context, r Report) bool {