From 3e7f85247289bf95f4244f54bdc7e4cbc4f2b2b7 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 21:03:16 +0100 Subject: [PATCH] Count a build node's toolchain report only once written since its assignment, and keep a computed fingerprint while builders disagree (issue 1940) On 2026-10-25 20:56 assigning mesh-build-node to ace again brought ace's report of 2026-10-23 (v26.12.0) back into the agreement before the module ran there; the indexer blanked every module's fingerprint to "builders disagree" and the controller's take-in refused modules already computed and built. - The toolchain gate (indexer, builder, checker; still one file in three) leaves a report out, said once, until its node writes it since its current assignment: the assignment's own time, or the revision that opened it. - The indexer keeps a module's held fingerprint while a language has no agreed toolchain, when its kept inputs with the toolchains it was computed with give it again; only a new fingerprint (a merge, a base, an agreed toolchain moved) needs the agreement. --- .../cmd/mesh-delivery-builder/standing.go | 89 +++++++++++------ .../mesh-delivery-builder/standing_test.go | 97 ++++++++++++++----- .../cmd/mesh-delivery-checker/standing.go | 89 +++++++++++------ .../mesh-delivery-checker/standing_test.go | 97 ++++++++++++++----- .../cmd/mesh-delivery-indexer/standing.go | 89 +++++++++++------ .../mesh-delivery-indexer/standing_test.go | 97 ++++++++++++++----- .../internal/index/toolchains.go | 58 ++++++++--- .../internal/index/toolchains_test.go | 32 +++++- 8 files changed, 485 insertions(+), 163 deletions(-) diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing.go index 5bb7d77..3d6decf 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing.go @@ -28,8 +28,11 @@ import ( // // toolchainGate stands between the toolchains buckets and what takes their entries in (the indexer's PutToolchain, a // buildrun.ToolchainSet): an entry is passed on only while the controller's assignments (mesh-controller_assignments, -// key .) hold an open assignment of its build module on its node; it is passed on as deleted otherwise, -// and passed on again when the module is assigned there again. Until the assignments are settled (or where they cannot +// key .) hold an open assignment of its build module on its node, and only once it was written since that +// assignment; it is passed on as deleted otherwise, said once, and passed on when its node writes it again. A report +// written before the assignment is the one an earlier run of the module left (issue 1940, 2026-10-25 20:56: mesh-build-node +// assigned to ace again brought ace.typescript v26.12.0 of 2026-10-23 back into the agreement, before the module ran +// there, and the controller's take-in refused every module "builders disagree"). Until the assignments are settled (or where they cannot // be read) every entry is passed on, as before. Builders that run and report different versions still disagree: that // refusal is the guard of novox/hq ADR 0465, and it stays. type toolchainGate struct { @@ -38,14 +41,14 @@ type toolchainGate struct { forward func(module, key string, value []byte, deleted bool) bool // answers whether it changed what it holds logf func(format string, a ...any) known bool // the assignments are settled - standing map[string]bool // . with an open assignment + standing map[string]time.Time // . with an open assignment -> since when it stands raw map[string]map[string][]byte // module -> toolchains key -> value, as the bucket holds it - left map[string]bool // / said as left out, so it is said once + left map[string]string // / -> why it was said left out, so each reason is said once } func newToolchainGate(modules []string, forward func(module, key string, value []byte, deleted bool) bool, logf func(string, ...any)) *toolchainGate { - g := &toolchainGate{modules: map[string]bool{}, forward: forward, logf: logf, standing: map[string]bool{}, - raw: map[string]map[string][]byte{}, left: map[string]bool{}} + g := &toolchainGate{modules: map[string]bool{}, forward: forward, logf: logf, standing: map[string]time.Time{}, + raw: map[string]map[string][]byte{}, left: map[string]string{}} for _, m := range modules { g.modules[m] = true } @@ -67,20 +70,31 @@ func (g *toolchainGate) Toolchain(module, key string, value []byte, deleted bool return g.pass(module, key) } -// Assignment takes one change of the controller's assignments in and answers whether what it passed on changed. -func (g *toolchainGate) Assignment(key string, value []byte, deleted bool) bool { +// Assignment takes one change of the controller's assignments in (at is when the bus wrote that revision) and answers +// whether what it passed on changed. +func (g *toolchainGate) Assignment(key string, value []byte, deleted bool, at time.Time) bool { node, module, ok := strings.Cut(key, ".") if !ok || !g.modules[module] { return false } - open := !deleted && openAssignment(value) + assigned, open := openAssignment(value) + open = open && !deleted g.mu.Lock() defer g.mu.Unlock() - if g.standing[key] == open { + was, stood := g.standing[key] + since := assigned + if since.IsZero() { + // A value with no time of its own (an assignment a module pulled in) stands since the revision that opened it. + since = at + if stood { + since = was + } + } + if open == stood && (!open || since.Equal(was)) { return false } if open { - g.standing[key] = true + g.standing[key] = since } else { delete(g.standing, key) } @@ -111,37 +125,58 @@ func (g *toolchainGate) Settled() bool { return changed } -// pass hands one key on as it stands: its value when its node is assigned (or the assignments are not known), deleted -// otherwise. The caller holds mu. +// pass hands one key on as it stands: its value when its node is assigned and wrote it since (or the assignments are not +// known), deleted otherwise. The caller holds mu. func (g *toolchainGate) pass(module, key string) bool { value, held := g.raw[module][key] if !held { return g.forward(module, key, nil, true) } node, _, err := delivery.ParseToolchainKey(key) - if err != nil || !g.known || g.standing[node+"."+module] { + why := "" + if err == nil && g.known { + why = g.leftOut(module, node, value) + } + if why == "" { delete(g.left, module+"/"+key) return g.forward(module, key, value, false) } - if !g.left[module+"/"+key] { - g.left[module+"/"+key] = true - g.logf("%s.%s is left out of the agreed toolchain: %s has no assignment on %s, so it is no builder (issue 1940)", - delivery.ToolchainsBucket(module), key, module, node) + if g.left[module+"/"+key] != why { + g.left[module+"/"+key] = why + g.logf("%s.%s is left out of the agreed toolchain: %s (issue 1940)", delivery.ToolchainsBucket(module), key, why) } return g.forward(module, key, nil, true) } -// openAssignment is whether a value of the controller's assignments holds an assignment that stands: one that has not -// ended and has no state (a pending one is not on the machine yet; mesh-controller internal/chain Assignment). -func openAssignment(value []byte) bool { +// leftOut is why a node's report is no builder's, or empty when it counts: its node has no open assignment of the module, +// or the report was written before that assignment. A value that is no toolchain is passed on for its reader to refuse. +// The caller holds mu. +func (g *toolchainGate) leftOut(module, node string, value []byte) string { + since, open := g.standing[node+"."+module] + if !open { + return fmt.Sprintf("%s has no assignment on %s, so it is no builder", module, node) + } + tc, err := delivery.DecodeToolchain(value) + if err == nil && tc.At.Before(since) { + return fmt.Sprintf("it was written at %s, before %s's assignment on %s at %s: it counts once %s writes it again", + tc.At.UTC().Format(time.RFC3339), module, node, since.UTC().Format(time.RFC3339), node) + } + return "" +} + +// openAssignment is whether a value of the controller's assignments holds an assignment that stands (one that has not +// ended and has no state: a pending one is not on the machine yet; mesh-controller internal/chain Assignment), and when +// it was assigned, zero where the value does not say. +func openAssignment(value []byte) (time.Time, bool) { var a struct { - Ended time.Time `json:"ended"` - State string `json:"state"` + Assigned time.Time `json:"assigned"` + Ended time.Time `json:"ended"` + State string `json:"state"` } if json.Unmarshal(value, &a) != nil { - return false + return time.Time{}, false } - return a.Ended.IsZero() && a.State == "" + return a.Assigned, a.Ended.IsZero() && a.State == "" } // watchAssignments settles the controller's assignments into the gate, says them settled, and follows them; changed is @@ -155,7 +190,7 @@ func watchAssignments(ctx context.Context, js bucketOpener, gate *toolchainGate, logger.Printf("the bucket %s cannot be read, so every node's toolchain report counts, an unassigned build node's too (issue 1940): %v", provider.AssignmentsBucket, err) return errs, nil } - each := func(e jetstream.KeyValueEntry) { gate.Assignment(e.Key(), e.Value(), !opOf(e)) } + each := func(e jetstream.KeyValueEntry) { gate.Assignment(e.Key(), e.Value(), !opOf(e), e.Created()) } w, err := settle(ctx, kv, each) if err != nil { return nil, fmt.Errorf("the bucket %s: %w", provider.AssignmentsBucket, err) @@ -165,7 +200,7 @@ func watchAssignments(ctx context.Context, js bucketOpener, gate *toolchainGate, } go func() { errs <- follow(ctx, w, func(e jetstream.KeyValueEntry) { - if gate.Assignment(e.Key(), e.Value(), !opOf(e)) { + if gate.Assignment(e.Key(), e.Value(), !opOf(e), e.Created()) { changed(ctx) } }) diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing_test.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing_test.go index 16ee063..28fccc4 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing_test.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/standing_test.go @@ -3,6 +3,7 @@ package main // The same in mesh-delivery-indexer, mesh-delivery-builder and mesh-delivery-checker, as standing.go is (issue 1940). import ( + "fmt" "strings" "testing" "time" @@ -44,10 +45,10 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { t.Fatalf("before the assignments are read every report counts, so they disagree; got %q", none) } for _, node := range []string{"g14", "novox", "shanks"} { - gate.Assignment(node+"."+buildrun.NodeBuildModule, []byte(`{"node":"`+node+`","module":"mesh-build-node"}`), false) + gate.Assignment(node+"."+buildrun.NodeBuildModule, []byte(`{"node":"`+node+`","module":"mesh-build-node"}`), false, time.Time{}) } - gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","ended":"2026-10-23T08:00:00Z"}`), false) - gate.Assignment("ace.mesh-build-go", []byte(`{"node":"ace","module":"mesh-build-go"}`), false) // another module's: no matter + gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","ended":"2026-10-23T08:00:00Z"}`), false, time.Time{}) + gate.Assignment("ace.mesh-build-go", []byte(`{"node":"ace","module":"mesh-build-go"}`), false, time.Time{}) // another module's: no matter if !gate.Settled() { t.Fatal("settling the assignments left ace's report out, so what was passed on changed") } @@ -63,20 +64,20 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { t.Fatal("an unassigned node's report changed what was passed on") } // Assigned to ace again: its report counts, and the builders really disagree: the guard stays. - if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node"}`), false) { + if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node"}`), false, time.Time{}) { t.Fatal("assigning ace again passed nothing on") } if _, none := agreedOf(set); !strings.HasPrefix(none, "builders disagree: v26.12.0 recipe f59571cf4be75adf on ace") { t.Fatalf("builders that run different versions still disagree; got %q", none) } // The assignment's key deleted, or a pending one written: no assignment stands, and ace's report goes again. - if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","state":"pending"}`), false) { + if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","state":"pending"}`), false, time.Time{}) { t.Fatal("a pending assignment is no assignment on the machine") } if v, none := agreedOf(set); v != "v26.12.1" || none != "" { t.Fatalf("got %q, %q", v, none) } - gate.Assignment("ace.mesh-build-node", nil, true) + gate.Assignment("ace.mesh-build-node", nil, true, time.Time{}) if v, _ := agreedOf(set); v != "v26.12.1" { t.Fatalf("got %q", v) } @@ -90,23 +91,75 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { } } -// A newly assigned build node that has not reported yet changes nothing; its report counts as it comes. +// A build node's report counts from the first one it writes since its assignment: a newly assigned node that has not +// reported changes nothing, and the report an earlier run left on the bus (issue 1940, 2026-10-25 20:56: ace.typescript +// v26.12.0 of 2026-10-23 06:29, mesh-build-node assigned to ace again at 20:56:45) is left out, said once, until the node +// writes it again. An assignment whose value names no time stands since the revision that opened it. func TestANewlyAssignedNodeCountsFromItsReport(t *testing.T) { - set := &buildrun.ToolchainSet{} - gate := newToolchainGate([]string{buildrun.NodeBuildModule}, set.Put, func(string, ...any) {}) - gate.Settled() - if gate.Assignment("g14.mesh-build-node", []byte(`{}`), false) { - t.Fatal("an assignment with no report passed something on") + left := time.Date(2026, 10, 23, 6, 29, 18, 0, time.UTC) // ace's report from its earlier run + assigned := time.Date(2026, 10, 25, 19, 56, 45, 0, time.UTC) // mesh-build-node assigned to ace again + report := func(t *testing.T, node, version string, at time.Time) (string, []byte) { + key, _ := gateReport(t, node, version) + value, err := delivery.EncodeToolchain(delivery.Toolchain{Language: "typescript", Version: version, Recipe: "f59571cf4be75adf", At: at}) + if err != nil { + t.Fatal(err) + } + return key, value } - key, value := gateReport(t, "g14", "v26.12.1") - if !gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { - t.Fatal("an assigned node's report was not passed on") - } - key, value = gateReport(t, "ace", "v26.12.0") - if gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { - t.Fatal("a report of a node with no assignment key was passed on after the assignments settled") - } - if v, none := agreedOf(set); v != "v26.12.1" || none != "" { - t.Fatalf("got %q, %q", v, none) + for _, c := range []struct { + name string + assignment string // ace's assignment value + revision time.Time // when the bus wrote it + at time.Time // when ace wrote its report + counts bool + }{ + {"no report yet, then one", `{}`, time.Time{}, time.Now(), true}, + {"a report left before the assignment", `{"node":"ace","module":"mesh-build-node","assigned":"2026-10-25T19:56:45Z"}`, assigned, left, false}, + {"a report left before a value with no time", `{"node":"ace","module":"mesh-build-node"}`, assigned, left, false}, + {"a report written since the assignment", `{"node":"ace","module":"mesh-build-node","assigned":"2026-10-25T19:56:45Z"}`, assigned, assigned.Add(30 * time.Second), true}, + } { + t.Run(c.name, func(t *testing.T) { + set := &buildrun.ToolchainSet{} + var said []string + gate := newToolchainGate([]string{buildrun.NodeBuildModule}, set.Put, func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) }) + for _, node := range []string{"g14", "novox", "shanks"} { + gate.Assignment(node+".mesh-build-node", []byte(`{}`), false, time.Time{}) + key, value := gateReport(t, node, "v26.12.1") + gate.Toolchain(buildrun.NodeBuildModule, key, value, false) + } + gate.Settled() + if gate.Assignment("ace.mesh-build-node", []byte(c.assignment), false, c.revision) { + t.Fatal("an assignment with no report passed something on") + } + key, value := report(t, "ace", "v26.12.0", c.at) + if got := gate.Toolchain(buildrun.NodeBuildModule, key, value, false); got != c.counts { + t.Fatalf("ace's report passed on: %v, want %v", got, c.counts) + } + // A rewrite of the assignment that does not move it changes nothing, and nothing is said again. + if gate.Assignment("ace.mesh-build-node", []byte(c.assignment), false, c.revision.Add(time.Minute)) { + t.Fatal("the same assignment written again changed what was passed on") + } + v, none := agreedOf(set) + if c.counts { + if !strings.HasPrefix(none, "builders disagree: v26.12.0 recipe f59571cf4be75adf on ace") { + t.Fatalf("ace's own report counts and disagrees; got %q, %q", v, none) + } + return + } + if v != "v26.12.1" || none != "" { + t.Fatalf("ace's left report is no builder's, so g14, novox and shanks agree; got %q, %q", v, none) + } + if len(said) != 1 || !strings.Contains(said[0], "before mesh-build-node's assignment on ace at 2026-10-25T19:56:45Z") { + t.Fatalf("said %q, want once why ace's report is left out", said) + } + // ace writes its report again since it was assigned: it counts. + key, value = report(t, "ace", "v26.12.1", assigned.Add(time.Minute)) + if !gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { + t.Fatal("ace's new report was not passed on") + } + if n := len(set.Entries("typescript")); n != 4 { + t.Fatalf("four builders report; got %d", n) + } + }) } } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing.go index 5bb7d77..3d6decf 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing.go @@ -28,8 +28,11 @@ import ( // // toolchainGate stands between the toolchains buckets and what takes their entries in (the indexer's PutToolchain, a // buildrun.ToolchainSet): an entry is passed on only while the controller's assignments (mesh-controller_assignments, -// key .) hold an open assignment of its build module on its node; it is passed on as deleted otherwise, -// and passed on again when the module is assigned there again. Until the assignments are settled (or where they cannot +// key .) hold an open assignment of its build module on its node, and only once it was written since that +// assignment; it is passed on as deleted otherwise, said once, and passed on when its node writes it again. A report +// written before the assignment is the one an earlier run of the module left (issue 1940, 2026-10-25 20:56: mesh-build-node +// assigned to ace again brought ace.typescript v26.12.0 of 2026-10-23 back into the agreement, before the module ran +// there, and the controller's take-in refused every module "builders disagree"). Until the assignments are settled (or where they cannot // be read) every entry is passed on, as before. Builders that run and report different versions still disagree: that // refusal is the guard of novox/hq ADR 0465, and it stays. type toolchainGate struct { @@ -38,14 +41,14 @@ type toolchainGate struct { forward func(module, key string, value []byte, deleted bool) bool // answers whether it changed what it holds logf func(format string, a ...any) known bool // the assignments are settled - standing map[string]bool // . with an open assignment + standing map[string]time.Time // . with an open assignment -> since when it stands raw map[string]map[string][]byte // module -> toolchains key -> value, as the bucket holds it - left map[string]bool // / said as left out, so it is said once + left map[string]string // / -> why it was said left out, so each reason is said once } func newToolchainGate(modules []string, forward func(module, key string, value []byte, deleted bool) bool, logf func(string, ...any)) *toolchainGate { - g := &toolchainGate{modules: map[string]bool{}, forward: forward, logf: logf, standing: map[string]bool{}, - raw: map[string]map[string][]byte{}, left: map[string]bool{}} + g := &toolchainGate{modules: map[string]bool{}, forward: forward, logf: logf, standing: map[string]time.Time{}, + raw: map[string]map[string][]byte{}, left: map[string]string{}} for _, m := range modules { g.modules[m] = true } @@ -67,20 +70,31 @@ func (g *toolchainGate) Toolchain(module, key string, value []byte, deleted bool return g.pass(module, key) } -// Assignment takes one change of the controller's assignments in and answers whether what it passed on changed. -func (g *toolchainGate) Assignment(key string, value []byte, deleted bool) bool { +// Assignment takes one change of the controller's assignments in (at is when the bus wrote that revision) and answers +// whether what it passed on changed. +func (g *toolchainGate) Assignment(key string, value []byte, deleted bool, at time.Time) bool { node, module, ok := strings.Cut(key, ".") if !ok || !g.modules[module] { return false } - open := !deleted && openAssignment(value) + assigned, open := openAssignment(value) + open = open && !deleted g.mu.Lock() defer g.mu.Unlock() - if g.standing[key] == open { + was, stood := g.standing[key] + since := assigned + if since.IsZero() { + // A value with no time of its own (an assignment a module pulled in) stands since the revision that opened it. + since = at + if stood { + since = was + } + } + if open == stood && (!open || since.Equal(was)) { return false } if open { - g.standing[key] = true + g.standing[key] = since } else { delete(g.standing, key) } @@ -111,37 +125,58 @@ func (g *toolchainGate) Settled() bool { return changed } -// pass hands one key on as it stands: its value when its node is assigned (or the assignments are not known), deleted -// otherwise. The caller holds mu. +// pass hands one key on as it stands: its value when its node is assigned and wrote it since (or the assignments are not +// known), deleted otherwise. The caller holds mu. func (g *toolchainGate) pass(module, key string) bool { value, held := g.raw[module][key] if !held { return g.forward(module, key, nil, true) } node, _, err := delivery.ParseToolchainKey(key) - if err != nil || !g.known || g.standing[node+"."+module] { + why := "" + if err == nil && g.known { + why = g.leftOut(module, node, value) + } + if why == "" { delete(g.left, module+"/"+key) return g.forward(module, key, value, false) } - if !g.left[module+"/"+key] { - g.left[module+"/"+key] = true - g.logf("%s.%s is left out of the agreed toolchain: %s has no assignment on %s, so it is no builder (issue 1940)", - delivery.ToolchainsBucket(module), key, module, node) + if g.left[module+"/"+key] != why { + g.left[module+"/"+key] = why + g.logf("%s.%s is left out of the agreed toolchain: %s (issue 1940)", delivery.ToolchainsBucket(module), key, why) } return g.forward(module, key, nil, true) } -// openAssignment is whether a value of the controller's assignments holds an assignment that stands: one that has not -// ended and has no state (a pending one is not on the machine yet; mesh-controller internal/chain Assignment). -func openAssignment(value []byte) bool { +// leftOut is why a node's report is no builder's, or empty when it counts: its node has no open assignment of the module, +// or the report was written before that assignment. A value that is no toolchain is passed on for its reader to refuse. +// The caller holds mu. +func (g *toolchainGate) leftOut(module, node string, value []byte) string { + since, open := g.standing[node+"."+module] + if !open { + return fmt.Sprintf("%s has no assignment on %s, so it is no builder", module, node) + } + tc, err := delivery.DecodeToolchain(value) + if err == nil && tc.At.Before(since) { + return fmt.Sprintf("it was written at %s, before %s's assignment on %s at %s: it counts once %s writes it again", + tc.At.UTC().Format(time.RFC3339), module, node, since.UTC().Format(time.RFC3339), node) + } + return "" +} + +// openAssignment is whether a value of the controller's assignments holds an assignment that stands (one that has not +// ended and has no state: a pending one is not on the machine yet; mesh-controller internal/chain Assignment), and when +// it was assigned, zero where the value does not say. +func openAssignment(value []byte) (time.Time, bool) { var a struct { - Ended time.Time `json:"ended"` - State string `json:"state"` + Assigned time.Time `json:"assigned"` + Ended time.Time `json:"ended"` + State string `json:"state"` } if json.Unmarshal(value, &a) != nil { - return false + return time.Time{}, false } - return a.Ended.IsZero() && a.State == "" + return a.Assigned, a.Ended.IsZero() && a.State == "" } // watchAssignments settles the controller's assignments into the gate, says them settled, and follows them; changed is @@ -155,7 +190,7 @@ func watchAssignments(ctx context.Context, js bucketOpener, gate *toolchainGate, logger.Printf("the bucket %s cannot be read, so every node's toolchain report counts, an unassigned build node's too (issue 1940): %v", provider.AssignmentsBucket, err) return errs, nil } - each := func(e jetstream.KeyValueEntry) { gate.Assignment(e.Key(), e.Value(), !opOf(e)) } + each := func(e jetstream.KeyValueEntry) { gate.Assignment(e.Key(), e.Value(), !opOf(e), e.Created()) } w, err := settle(ctx, kv, each) if err != nil { return nil, fmt.Errorf("the bucket %s: %w", provider.AssignmentsBucket, err) @@ -165,7 +200,7 @@ func watchAssignments(ctx context.Context, js bucketOpener, gate *toolchainGate, } go func() { errs <- follow(ctx, w, func(e jetstream.KeyValueEntry) { - if gate.Assignment(e.Key(), e.Value(), !opOf(e)) { + if gate.Assignment(e.Key(), e.Value(), !opOf(e), e.Created()) { changed(ctx) } }) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing_test.go index 16ee063..28fccc4 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/standing_test.go @@ -3,6 +3,7 @@ package main // The same in mesh-delivery-indexer, mesh-delivery-builder and mesh-delivery-checker, as standing.go is (issue 1940). import ( + "fmt" "strings" "testing" "time" @@ -44,10 +45,10 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { t.Fatalf("before the assignments are read every report counts, so they disagree; got %q", none) } for _, node := range []string{"g14", "novox", "shanks"} { - gate.Assignment(node+"."+buildrun.NodeBuildModule, []byte(`{"node":"`+node+`","module":"mesh-build-node"}`), false) + gate.Assignment(node+"."+buildrun.NodeBuildModule, []byte(`{"node":"`+node+`","module":"mesh-build-node"}`), false, time.Time{}) } - gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","ended":"2026-10-23T08:00:00Z"}`), false) - gate.Assignment("ace.mesh-build-go", []byte(`{"node":"ace","module":"mesh-build-go"}`), false) // another module's: no matter + gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","ended":"2026-10-23T08:00:00Z"}`), false, time.Time{}) + gate.Assignment("ace.mesh-build-go", []byte(`{"node":"ace","module":"mesh-build-go"}`), false, time.Time{}) // another module's: no matter if !gate.Settled() { t.Fatal("settling the assignments left ace's report out, so what was passed on changed") } @@ -63,20 +64,20 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { t.Fatal("an unassigned node's report changed what was passed on") } // Assigned to ace again: its report counts, and the builders really disagree: the guard stays. - if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node"}`), false) { + if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node"}`), false, time.Time{}) { t.Fatal("assigning ace again passed nothing on") } if _, none := agreedOf(set); !strings.HasPrefix(none, "builders disagree: v26.12.0 recipe f59571cf4be75adf on ace") { t.Fatalf("builders that run different versions still disagree; got %q", none) } // The assignment's key deleted, or a pending one written: no assignment stands, and ace's report goes again. - if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","state":"pending"}`), false) { + if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","state":"pending"}`), false, time.Time{}) { t.Fatal("a pending assignment is no assignment on the machine") } if v, none := agreedOf(set); v != "v26.12.1" || none != "" { t.Fatalf("got %q, %q", v, none) } - gate.Assignment("ace.mesh-build-node", nil, true) + gate.Assignment("ace.mesh-build-node", nil, true, time.Time{}) if v, _ := agreedOf(set); v != "v26.12.1" { t.Fatalf("got %q", v) } @@ -90,23 +91,75 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { } } -// A newly assigned build node that has not reported yet changes nothing; its report counts as it comes. +// A build node's report counts from the first one it writes since its assignment: a newly assigned node that has not +// reported changes nothing, and the report an earlier run left on the bus (issue 1940, 2026-10-25 20:56: ace.typescript +// v26.12.0 of 2026-10-23 06:29, mesh-build-node assigned to ace again at 20:56:45) is left out, said once, until the node +// writes it again. An assignment whose value names no time stands since the revision that opened it. func TestANewlyAssignedNodeCountsFromItsReport(t *testing.T) { - set := &buildrun.ToolchainSet{} - gate := newToolchainGate([]string{buildrun.NodeBuildModule}, set.Put, func(string, ...any) {}) - gate.Settled() - if gate.Assignment("g14.mesh-build-node", []byte(`{}`), false) { - t.Fatal("an assignment with no report passed something on") + left := time.Date(2026, 10, 23, 6, 29, 18, 0, time.UTC) // ace's report from its earlier run + assigned := time.Date(2026, 10, 25, 19, 56, 45, 0, time.UTC) // mesh-build-node assigned to ace again + report := func(t *testing.T, node, version string, at time.Time) (string, []byte) { + key, _ := gateReport(t, node, version) + value, err := delivery.EncodeToolchain(delivery.Toolchain{Language: "typescript", Version: version, Recipe: "f59571cf4be75adf", At: at}) + if err != nil { + t.Fatal(err) + } + return key, value } - key, value := gateReport(t, "g14", "v26.12.1") - if !gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { - t.Fatal("an assigned node's report was not passed on") - } - key, value = gateReport(t, "ace", "v26.12.0") - if gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { - t.Fatal("a report of a node with no assignment key was passed on after the assignments settled") - } - if v, none := agreedOf(set); v != "v26.12.1" || none != "" { - t.Fatalf("got %q, %q", v, none) + for _, c := range []struct { + name string + assignment string // ace's assignment value + revision time.Time // when the bus wrote it + at time.Time // when ace wrote its report + counts bool + }{ + {"no report yet, then one", `{}`, time.Time{}, time.Now(), true}, + {"a report left before the assignment", `{"node":"ace","module":"mesh-build-node","assigned":"2026-10-25T19:56:45Z"}`, assigned, left, false}, + {"a report left before a value with no time", `{"node":"ace","module":"mesh-build-node"}`, assigned, left, false}, + {"a report written since the assignment", `{"node":"ace","module":"mesh-build-node","assigned":"2026-10-25T19:56:45Z"}`, assigned, assigned.Add(30 * time.Second), true}, + } { + t.Run(c.name, func(t *testing.T) { + set := &buildrun.ToolchainSet{} + var said []string + gate := newToolchainGate([]string{buildrun.NodeBuildModule}, set.Put, func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) }) + for _, node := range []string{"g14", "novox", "shanks"} { + gate.Assignment(node+".mesh-build-node", []byte(`{}`), false, time.Time{}) + key, value := gateReport(t, node, "v26.12.1") + gate.Toolchain(buildrun.NodeBuildModule, key, value, false) + } + gate.Settled() + if gate.Assignment("ace.mesh-build-node", []byte(c.assignment), false, c.revision) { + t.Fatal("an assignment with no report passed something on") + } + key, value := report(t, "ace", "v26.12.0", c.at) + if got := gate.Toolchain(buildrun.NodeBuildModule, key, value, false); got != c.counts { + t.Fatalf("ace's report passed on: %v, want %v", got, c.counts) + } + // A rewrite of the assignment that does not move it changes nothing, and nothing is said again. + if gate.Assignment("ace.mesh-build-node", []byte(c.assignment), false, c.revision.Add(time.Minute)) { + t.Fatal("the same assignment written again changed what was passed on") + } + v, none := agreedOf(set) + if c.counts { + if !strings.HasPrefix(none, "builders disagree: v26.12.0 recipe f59571cf4be75adf on ace") { + t.Fatalf("ace's own report counts and disagrees; got %q, %q", v, none) + } + return + } + if v != "v26.12.1" || none != "" { + t.Fatalf("ace's left report is no builder's, so g14, novox and shanks agree; got %q, %q", v, none) + } + if len(said) != 1 || !strings.Contains(said[0], "before mesh-build-node's assignment on ace at 2026-10-25T19:56:45Z") { + t.Fatalf("said %q, want once why ace's report is left out", said) + } + // ace writes its report again since it was assigned: it counts. + key, value = report(t, "ace", "v26.12.1", assigned.Add(time.Minute)) + if !gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { + t.Fatal("ace's new report was not passed on") + } + if n := len(set.Entries("typescript")); n != 4 { + t.Fatalf("four builders report; got %d", n) + } + }) } } diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing.go index 5bb7d77..3d6decf 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing.go @@ -28,8 +28,11 @@ import ( // // toolchainGate stands between the toolchains buckets and what takes their entries in (the indexer's PutToolchain, a // buildrun.ToolchainSet): an entry is passed on only while the controller's assignments (mesh-controller_assignments, -// key .) hold an open assignment of its build module on its node; it is passed on as deleted otherwise, -// and passed on again when the module is assigned there again. Until the assignments are settled (or where they cannot +// key .) hold an open assignment of its build module on its node, and only once it was written since that +// assignment; it is passed on as deleted otherwise, said once, and passed on when its node writes it again. A report +// written before the assignment is the one an earlier run of the module left (issue 1940, 2026-10-25 20:56: mesh-build-node +// assigned to ace again brought ace.typescript v26.12.0 of 2026-10-23 back into the agreement, before the module ran +// there, and the controller's take-in refused every module "builders disagree"). Until the assignments are settled (or where they cannot // be read) every entry is passed on, as before. Builders that run and report different versions still disagree: that // refusal is the guard of novox/hq ADR 0465, and it stays. type toolchainGate struct { @@ -38,14 +41,14 @@ type toolchainGate struct { forward func(module, key string, value []byte, deleted bool) bool // answers whether it changed what it holds logf func(format string, a ...any) known bool // the assignments are settled - standing map[string]bool // . with an open assignment + standing map[string]time.Time // . with an open assignment -> since when it stands raw map[string]map[string][]byte // module -> toolchains key -> value, as the bucket holds it - left map[string]bool // / said as left out, so it is said once + left map[string]string // / -> why it was said left out, so each reason is said once } func newToolchainGate(modules []string, forward func(module, key string, value []byte, deleted bool) bool, logf func(string, ...any)) *toolchainGate { - g := &toolchainGate{modules: map[string]bool{}, forward: forward, logf: logf, standing: map[string]bool{}, - raw: map[string]map[string][]byte{}, left: map[string]bool{}} + g := &toolchainGate{modules: map[string]bool{}, forward: forward, logf: logf, standing: map[string]time.Time{}, + raw: map[string]map[string][]byte{}, left: map[string]string{}} for _, m := range modules { g.modules[m] = true } @@ -67,20 +70,31 @@ func (g *toolchainGate) Toolchain(module, key string, value []byte, deleted bool return g.pass(module, key) } -// Assignment takes one change of the controller's assignments in and answers whether what it passed on changed. -func (g *toolchainGate) Assignment(key string, value []byte, deleted bool) bool { +// Assignment takes one change of the controller's assignments in (at is when the bus wrote that revision) and answers +// whether what it passed on changed. +func (g *toolchainGate) Assignment(key string, value []byte, deleted bool, at time.Time) bool { node, module, ok := strings.Cut(key, ".") if !ok || !g.modules[module] { return false } - open := !deleted && openAssignment(value) + assigned, open := openAssignment(value) + open = open && !deleted g.mu.Lock() defer g.mu.Unlock() - if g.standing[key] == open { + was, stood := g.standing[key] + since := assigned + if since.IsZero() { + // A value with no time of its own (an assignment a module pulled in) stands since the revision that opened it. + since = at + if stood { + since = was + } + } + if open == stood && (!open || since.Equal(was)) { return false } if open { - g.standing[key] = true + g.standing[key] = since } else { delete(g.standing, key) } @@ -111,37 +125,58 @@ func (g *toolchainGate) Settled() bool { return changed } -// pass hands one key on as it stands: its value when its node is assigned (or the assignments are not known), deleted -// otherwise. The caller holds mu. +// pass hands one key on as it stands: its value when its node is assigned and wrote it since (or the assignments are not +// known), deleted otherwise. The caller holds mu. func (g *toolchainGate) pass(module, key string) bool { value, held := g.raw[module][key] if !held { return g.forward(module, key, nil, true) } node, _, err := delivery.ParseToolchainKey(key) - if err != nil || !g.known || g.standing[node+"."+module] { + why := "" + if err == nil && g.known { + why = g.leftOut(module, node, value) + } + if why == "" { delete(g.left, module+"/"+key) return g.forward(module, key, value, false) } - if !g.left[module+"/"+key] { - g.left[module+"/"+key] = true - g.logf("%s.%s is left out of the agreed toolchain: %s has no assignment on %s, so it is no builder (issue 1940)", - delivery.ToolchainsBucket(module), key, module, node) + if g.left[module+"/"+key] != why { + g.left[module+"/"+key] = why + g.logf("%s.%s is left out of the agreed toolchain: %s (issue 1940)", delivery.ToolchainsBucket(module), key, why) } return g.forward(module, key, nil, true) } -// openAssignment is whether a value of the controller's assignments holds an assignment that stands: one that has not -// ended and has no state (a pending one is not on the machine yet; mesh-controller internal/chain Assignment). -func openAssignment(value []byte) bool { +// leftOut is why a node's report is no builder's, or empty when it counts: its node has no open assignment of the module, +// or the report was written before that assignment. A value that is no toolchain is passed on for its reader to refuse. +// The caller holds mu. +func (g *toolchainGate) leftOut(module, node string, value []byte) string { + since, open := g.standing[node+"."+module] + if !open { + return fmt.Sprintf("%s has no assignment on %s, so it is no builder", module, node) + } + tc, err := delivery.DecodeToolchain(value) + if err == nil && tc.At.Before(since) { + return fmt.Sprintf("it was written at %s, before %s's assignment on %s at %s: it counts once %s writes it again", + tc.At.UTC().Format(time.RFC3339), module, node, since.UTC().Format(time.RFC3339), node) + } + return "" +} + +// openAssignment is whether a value of the controller's assignments holds an assignment that stands (one that has not +// ended and has no state: a pending one is not on the machine yet; mesh-controller internal/chain Assignment), and when +// it was assigned, zero where the value does not say. +func openAssignment(value []byte) (time.Time, bool) { var a struct { - Ended time.Time `json:"ended"` - State string `json:"state"` + Assigned time.Time `json:"assigned"` + Ended time.Time `json:"ended"` + State string `json:"state"` } if json.Unmarshal(value, &a) != nil { - return false + return time.Time{}, false } - return a.Ended.IsZero() && a.State == "" + return a.Assigned, a.Ended.IsZero() && a.State == "" } // watchAssignments settles the controller's assignments into the gate, says them settled, and follows them; changed is @@ -155,7 +190,7 @@ func watchAssignments(ctx context.Context, js bucketOpener, gate *toolchainGate, logger.Printf("the bucket %s cannot be read, so every node's toolchain report counts, an unassigned build node's too (issue 1940): %v", provider.AssignmentsBucket, err) return errs, nil } - each := func(e jetstream.KeyValueEntry) { gate.Assignment(e.Key(), e.Value(), !opOf(e)) } + each := func(e jetstream.KeyValueEntry) { gate.Assignment(e.Key(), e.Value(), !opOf(e), e.Created()) } w, err := settle(ctx, kv, each) if err != nil { return nil, fmt.Errorf("the bucket %s: %w", provider.AssignmentsBucket, err) @@ -165,7 +200,7 @@ func watchAssignments(ctx context.Context, js bucketOpener, gate *toolchainGate, } go func() { errs <- follow(ctx, w, func(e jetstream.KeyValueEntry) { - if gate.Assignment(e.Key(), e.Value(), !opOf(e)) { + if gate.Assignment(e.Key(), e.Value(), !opOf(e), e.Created()) { changed(ctx) } }) diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing_test.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing_test.go index 16ee063..28fccc4 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing_test.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/standing_test.go @@ -3,6 +3,7 @@ package main // The same in mesh-delivery-indexer, mesh-delivery-builder and mesh-delivery-checker, as standing.go is (issue 1940). import ( + "fmt" "strings" "testing" "time" @@ -44,10 +45,10 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { t.Fatalf("before the assignments are read every report counts, so they disagree; got %q", none) } for _, node := range []string{"g14", "novox", "shanks"} { - gate.Assignment(node+"."+buildrun.NodeBuildModule, []byte(`{"node":"`+node+`","module":"mesh-build-node"}`), false) + gate.Assignment(node+"."+buildrun.NodeBuildModule, []byte(`{"node":"`+node+`","module":"mesh-build-node"}`), false, time.Time{}) } - gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","ended":"2026-10-23T08:00:00Z"}`), false) - gate.Assignment("ace.mesh-build-go", []byte(`{"node":"ace","module":"mesh-build-go"}`), false) // another module's: no matter + gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","ended":"2026-10-23T08:00:00Z"}`), false, time.Time{}) + gate.Assignment("ace.mesh-build-go", []byte(`{"node":"ace","module":"mesh-build-go"}`), false, time.Time{}) // another module's: no matter if !gate.Settled() { t.Fatal("settling the assignments left ace's report out, so what was passed on changed") } @@ -63,20 +64,20 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { t.Fatal("an unassigned node's report changed what was passed on") } // Assigned to ace again: its report counts, and the builders really disagree: the guard stays. - if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node"}`), false) { + if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node"}`), false, time.Time{}) { t.Fatal("assigning ace again passed nothing on") } if _, none := agreedOf(set); !strings.HasPrefix(none, "builders disagree: v26.12.0 recipe f59571cf4be75adf on ace") { t.Fatalf("builders that run different versions still disagree; got %q", none) } // The assignment's key deleted, or a pending one written: no assignment stands, and ace's report goes again. - if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","state":"pending"}`), false) { + if !gate.Assignment("ace.mesh-build-node", []byte(`{"node":"ace","module":"mesh-build-node","state":"pending"}`), false, time.Time{}) { t.Fatal("a pending assignment is no assignment on the machine") } if v, none := agreedOf(set); v != "v26.12.1" || none != "" { t.Fatalf("got %q, %q", v, none) } - gate.Assignment("ace.mesh-build-node", nil, true) + gate.Assignment("ace.mesh-build-node", nil, true, time.Time{}) if v, _ := agreedOf(set); v != "v26.12.1" { t.Fatalf("got %q", v) } @@ -90,23 +91,75 @@ func TestAnUnassignedBuildNodesReportIsNoBuilder(t *testing.T) { } } -// A newly assigned build node that has not reported yet changes nothing; its report counts as it comes. +// A build node's report counts from the first one it writes since its assignment: a newly assigned node that has not +// reported changes nothing, and the report an earlier run left on the bus (issue 1940, 2026-10-25 20:56: ace.typescript +// v26.12.0 of 2026-10-23 06:29, mesh-build-node assigned to ace again at 20:56:45) is left out, said once, until the node +// writes it again. An assignment whose value names no time stands since the revision that opened it. func TestANewlyAssignedNodeCountsFromItsReport(t *testing.T) { - set := &buildrun.ToolchainSet{} - gate := newToolchainGate([]string{buildrun.NodeBuildModule}, set.Put, func(string, ...any) {}) - gate.Settled() - if gate.Assignment("g14.mesh-build-node", []byte(`{}`), false) { - t.Fatal("an assignment with no report passed something on") + left := time.Date(2026, 10, 23, 6, 29, 18, 0, time.UTC) // ace's report from its earlier run + assigned := time.Date(2026, 10, 25, 19, 56, 45, 0, time.UTC) // mesh-build-node assigned to ace again + report := func(t *testing.T, node, version string, at time.Time) (string, []byte) { + key, _ := gateReport(t, node, version) + value, err := delivery.EncodeToolchain(delivery.Toolchain{Language: "typescript", Version: version, Recipe: "f59571cf4be75adf", At: at}) + if err != nil { + t.Fatal(err) + } + return key, value } - key, value := gateReport(t, "g14", "v26.12.1") - if !gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { - t.Fatal("an assigned node's report was not passed on") - } - key, value = gateReport(t, "ace", "v26.12.0") - if gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { - t.Fatal("a report of a node with no assignment key was passed on after the assignments settled") - } - if v, none := agreedOf(set); v != "v26.12.1" || none != "" { - t.Fatalf("got %q, %q", v, none) + for _, c := range []struct { + name string + assignment string // ace's assignment value + revision time.Time // when the bus wrote it + at time.Time // when ace wrote its report + counts bool + }{ + {"no report yet, then one", `{}`, time.Time{}, time.Now(), true}, + {"a report left before the assignment", `{"node":"ace","module":"mesh-build-node","assigned":"2026-10-25T19:56:45Z"}`, assigned, left, false}, + {"a report left before a value with no time", `{"node":"ace","module":"mesh-build-node"}`, assigned, left, false}, + {"a report written since the assignment", `{"node":"ace","module":"mesh-build-node","assigned":"2026-10-25T19:56:45Z"}`, assigned, assigned.Add(30 * time.Second), true}, + } { + t.Run(c.name, func(t *testing.T) { + set := &buildrun.ToolchainSet{} + var said []string + gate := newToolchainGate([]string{buildrun.NodeBuildModule}, set.Put, func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) }) + for _, node := range []string{"g14", "novox", "shanks"} { + gate.Assignment(node+".mesh-build-node", []byte(`{}`), false, time.Time{}) + key, value := gateReport(t, node, "v26.12.1") + gate.Toolchain(buildrun.NodeBuildModule, key, value, false) + } + gate.Settled() + if gate.Assignment("ace.mesh-build-node", []byte(c.assignment), false, c.revision) { + t.Fatal("an assignment with no report passed something on") + } + key, value := report(t, "ace", "v26.12.0", c.at) + if got := gate.Toolchain(buildrun.NodeBuildModule, key, value, false); got != c.counts { + t.Fatalf("ace's report passed on: %v, want %v", got, c.counts) + } + // A rewrite of the assignment that does not move it changes nothing, and nothing is said again. + if gate.Assignment("ace.mesh-build-node", []byte(c.assignment), false, c.revision.Add(time.Minute)) { + t.Fatal("the same assignment written again changed what was passed on") + } + v, none := agreedOf(set) + if c.counts { + if !strings.HasPrefix(none, "builders disagree: v26.12.0 recipe f59571cf4be75adf on ace") { + t.Fatalf("ace's own report counts and disagrees; got %q, %q", v, none) + } + return + } + if v != "v26.12.1" || none != "" { + t.Fatalf("ace's left report is no builder's, so g14, novox and shanks agree; got %q, %q", v, none) + } + if len(said) != 1 || !strings.Contains(said[0], "before mesh-build-node's assignment on ace at 2026-10-25T19:56:45Z") { + t.Fatalf("said %q, want once why ace's report is left out", said) + } + // ace writes its report again since it was assigned: it counts. + key, value = report(t, "ace", "v26.12.1", assigned.Add(time.Minute)) + if !gate.Toolchain(buildrun.NodeBuildModule, key, value, false) { + t.Fatal("ace's new report was not passed on") + } + if n := len(set.Entries("typescript")); n != 4 { + t.Fatalf("four builders report; got %d", n) + } + }) } } diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index 43db138..cbf1794 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -112,6 +112,9 @@ type known struct { languages map[string]string bases []string contexts bool + // held is the fingerprint the bucket holds for the module and the toolchains it was computed with (for a merge, the + // value before it): what stillComputed keeps while the builders agree on no toolchain. + held delivery.Module } // Recompute writes again every module whose new-form fingerprint, or reason for none, is no longer what it holds: from @@ -159,20 +162,24 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli stale := map[string]bool{} // values an SDK that could not read their manifest wrote, judged again (issue 1923) all := map[string]*known{} for _, key := range keys { - cur, ok := overlay[key] - if !ok { - value, _, found, err := ix.Modules.Get(ctx, key) - if err != nil { - return fmt.Errorf("modules.%s: %w", key, err) - } - if !found { - continue - } - if cur, err = delivery.DecodeModule(value); err != nil { + cur, merged := overlay[key] + var before delivery.Module + value, _, found, err := ix.Modules.Get(ctx, key) + if err != nil { + return fmt.Errorf("modules.%s: %w", key, err) + } + if found { + if before, err = delivery.DecodeModule(value); err != nil && !merged { ix.logf("modules.%s holds what is not a module, left: %v", key, err) continue } } + if !merged { + if !found { + continue + } + cur = before + } held[key] = cur if cur.Manifest == "" || cur.Inputs2 == nil { continue @@ -186,7 +193,7 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli cur, stale[key] = again, true held[key] = cur } - all[key] = &known{key: key, cur: cur, languages: languagesOf(m), bases: basesOf(m), contexts: hasContext(m)} + all[key] = &known{key: key, cur: cur, languages: languagesOf(m), bases: basesOf(m), contexts: hasContext(m), held: before} } tools := ix.agreed() type result struct { @@ -251,6 +258,9 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli r.none = strings.Join(missing, "; ") case noToolchain != "": r.none = noToolchain + if fp, ok := stillComputed(module, kept, k, toolchains); ok { + r = result{fp: fp, tcs: k.held.Toolchains} + } default: r.fp, r.none = delivery.SourceFingerprint2(kept.With(module, toolchains)) if r.fp != "" { @@ -381,6 +391,32 @@ func hasContext(m catalogue.Manifest) bool { return false } +// stillComputed is the fingerprint a module already holds, kept while a language it is built with has no agreed toolchain +// (issue 1940, 2026-10-25 20:56: a build node assigned again brought a stale report into the agreement, every module's +// fingerprint was blanked to "builders disagree", and the controller's take-in refused modules whose fingerprints were +// computed and built). Only computing a new fingerprint needs the agreement: the held one is kept when its kept inputs, +// with the toolchains it was computed with in place of the languages agreed on none, give it again. Any other input that +// moved (a merge, a base, an agreed toolchain) gives another fingerprint, and the module has none until they agree. +func stillComputed(module string, kept delivery.KeptInputs2, k *known, agreed map[string]delivery.Toolchain2) (string, bool) { + if k.held.FP2 == "" { + return "", false + } + toolchains := map[string]delivery.Toolchain2{} + for artifact, language := range k.languages { + if tc, ok := agreed[artifact]; ok { + toolchains[artifact] = tc + continue + } + used, ok := k.held.Toolchains[language] + if !ok { + return "", false + } + toolchains[artifact] = delivery.Toolchain2{Language: language, Version: used.Version, Recipe: used.Recipe} + } + fp, none := delivery.SourceFingerprint2(kept.With(module, toolchains)) + return fp, none == "" && fp == k.held.FP2 +} + // toolchainsUsed are the toolchains a fingerprint was computed with, by language (an artifact's toolchain is its language's). func toolchainsUsed(by map[string]delivery.Toolchain2) map[string]delivery.ToolchainUsed { if len(by) == 0 { diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go index d97fdc4..df6128a 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go @@ -123,9 +123,16 @@ func TestAgreedToolchainChangesAGoModulesReasonWithNoMerge(t *testing.T) { // 14 } func TestNodesReportingDifferentVersionsLeaveTheReasonNamingBoth(t *testing.T) { // 1444/AC6 + // Both reports are read before the module's first fingerprint is computed (a fingerprint computed before they + // disagree is kept: TestAModuleKeepsTheToolchainsItsFingerprintWasComputedWith). ix, mods := newFormIndexer(t, mod{"avahi", catalogueManifest(t, "avahi")}) - reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") - reportToolchain(t, ix, "mesh-build-go", "ace", "go", "go1.26.0") + for node, version := range map[string]string{"novox": "go1.27.1", "ace": "go1.26.0"} { + key, _ := delivery.ToolchainKey(node, "go") + ix.PutToolchain("mesh-build-go", key, toolchainValue(t, "go", version, "1"), false) + } + if err := ix.Recompute(context.Background()); err != nil { + t.Fatal(err) + } m, _ := moduleOf(t, mods, "avahi") if !strings.Contains(m.None2, "builders disagree") || !strings.Contains(m.None2, "go1.27.1") || !strings.Contains(m.None2, "go1.26.0") { t.Errorf("none2 = %q, want 'builders disagree' naming both versions", m.None2) @@ -288,9 +295,24 @@ func TestAModuleKeepsTheToolchainsItsFingerprintWasComputedWith(t *testing.T) { if n.FP2 == m.FP2 || n.Toolchains["go"].Version != "go1.27.2" { t.Errorf("after the upgrade: fp2 %q (was %q) toolchains %+v, want the new version with the new fingerprint", n.FP2, m.FP2, n.Toolchains) } - // Nodes that disagree leave no fingerprint and no toolchains. + // Nodes that disagree keep the fingerprint already computed, with its toolchains (issue 1940, 2026-10-25 20:56: a + // stale report blanked every module and the controller's take-in refused modules already built); only a new + // fingerprint needs the agreement: a merge that moves the module's tree gives it none, naming the disagreement. reportToolchain(t, ix, "mesh-build-go", "ace", "go", "go1.26.0") - if d, _ := moduleOf(t, mods, "avahi"); d.FP2 != "" || d.Toolchains != nil { - t.Errorf("disagreeing nodes: fp2 %q toolchains %+v, want neither", d.FP2, d.Toolchains) + if d, _ := moduleOf(t, mods, "avahi"); d.FP2 != n.FP2 || !reflect.DeepEqual(d.Toolchains, n.Toolchains) || d.None2 != "" { + t.Errorf("disagreeing nodes: fp2 %q toolchains %+v none2 %q, want the fingerprint kept (%q) with %+v", d.FP2, d.Toolchains, d.None2, n.FP2, n.Toolchains) + } + _, _, f := newFormIndexerUnmerged(t, builderModules(t)...) + f.Trees["modules/avahi"] = "tree-avahi-moved" + if err := ix.Apply(context.Background(), f); err != nil { + t.Fatal(err) + } + if d, _ := moduleOf(t, mods, "avahi"); d.FP2 != "" || d.Toolchains != nil || !strings.HasPrefix(d.None2, "builders disagree") { + t.Errorf("a new tree while nodes disagree: fp2 %q toolchains %+v none2 %q, want no fingerprint and the disagreement", d.FP2, d.Toolchains, d.None2) + } + // They agree again: the new tree gets its fingerprint. + reportToolchain(t, ix, "mesh-build-go", "ace", "go", "go1.27.2") + if a, _ := moduleOf(t, mods, "avahi"); a.FP2 == "" || a.FP2 == n.FP2 { + t.Errorf("agreed again: fp2 %q (was %q), want a new fingerprint", a.FP2, n.FP2) } } -- 2.54.0