diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go index 371003d2..fd39fe86 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go @@ -246,7 +246,7 @@ func Tools(r check.Reader) []stdio.Tool { "hand-now tool steers one. dead_letters says whether the checker hears the asks the build seats give up on (issue 734), and why not: " + "deaf, a try or check whose ask is given up waits until its bound. builds_not_ended names, of each running check, the builder's builds of " + "its candidate's changed modules that are neither done nor failed: the check waits on them before it composes, within its build-wait-bound " + - "(novox/hq ADR 0603 §10); a failed one fails the check: the builder's builder_build_again builds it again, then mesh-delivery.recheck asks the check again. Read-only.", + "(novox/hq ADR 0603 §10); a failed one fails the check: rebuild it with the builder's builder_build_again, and the check asks itself again when the build is done. Read-only.", Input: map[string]any{"type": "object", "properties": map[string]any{}}, Run: func(map[string]any) (any, error) { return r.Status() }}, } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index d2dc3250..823deb6c 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -618,12 +618,23 @@ func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300 if v := b.verdict(t, key, func(delivery.Verdict) bool { return true }); v.State != delivery.VerdictRunning || len(asked()) != 0 { t.Fatalf("verdict %s and %d asks while m builds: the check composes only once its build ended", v.State, len(asked())) } + // m's build fails: the check fails naming it. Built again (builder_build_again writes the key done), the check asks + // itself again for the same heads, with no recheck, and passes. + failedBuild, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildFailed, Error: "go build: undefined: x"}) + if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, failedBuild); err != nil { + t.Fatal(err) + } + if v := b.verdict(t, key, ended); v.State != delivery.VerdictFail || v.Failed.Test != check.BuildFailedTest+"m" { + t.Fatalf("verdict %+v, want it failed on m's build", v) + } built, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildDone, Manifest: digest}) if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, built); err != nil { t.Fatal(err) } - v := b.verdict(t, key, ended) - if v.State != delivery.VerdictPass || v.Levels[delivery.LevelUnit].Run != 2 || v.Levels[delivery.LevelIntegration].Run != 2 || v.Trunk[repo] != trunk { + b.said(t, "the builds of m it failed on are done again, so it is checked again") + v := b.verdict(t, key, func(v delivery.Verdict) bool { return v.State == delivery.VerdictPass }) + // The check asked again takes the packages the failed check ran from their kept verdicts. + if v.State != delivery.VerdictPass || v.Levels[delivery.LevelUnit].Kept != 2 || v.Levels[delivery.LevelIntegration].Kept != 2 || v.Trunk[repo] != trunk { t.Fatalf("verdict = %+v", v) } if c := v.Composition; c == nil || c.State != delivery.CompositionPass || !slices.Equal(c.Machines, []string{"n1"}) || @@ -643,10 +654,11 @@ func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300 t.Errorf("the check ask named the nodes %v, want [n1] from the have's node record", names) } o := b.owed(t, 7, head1, "success") - if o.Context != delivery.StatusContextCheck || !strings.HasPrefix(o.Text, "pass 111111111111 on aaaaaaaaaaaa") || !strings.Contains(o.Text, "unit 2 run") { + if o.Context != delivery.StatusContextCheck || !strings.HasPrefix(o.Text, "pass 111111111111 on aaaaaaaaaaaa") || !strings.Contains(o.Text, "unit 0 run, 2 kept") { t.Errorf("owed = %+v", o) } - for tool, want := range map[string]int{"mesh-build-go.build_go_fetch": 1, "mesh-build-go.build_go_digest": 2, "mesh-build-go.build_go_test": 2} { + // Two checks of the same heads: the one failed on m's build and the one it asked itself again; the second runs no test. + for tool, want := range map[string]int{"mesh-build-go.build_go_fetch": 2, "mesh-build-go.build_go_digest": 4, "mesh-build-go.build_go_test": 2} { if got := b.fake.count(tool); got != want { t.Errorf("%s called %d times, want %d", tool, got, want) } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go index 949f4376..047e9ab0 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -305,6 +305,18 @@ func (r buildReader) Build(ctx context.Context, module, fp2 string) (delivery.Bu type buildEnds struct{ kv jetstream.KeyValue } func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) { + return e.until(ctx, builds, func(b delivery.BuilderBuild) bool { + return b.State == delivery.BuildDone || b.State == delivery.BuildFailed + }) +} + +func (e buildEnds) Done(ctx context.Context, builds map[string]string) bool { + done, err := e.until(ctx, builds, func(b delivery.BuilderBuild) bool { return b.State == delivery.BuildDone }) + return err == nil && len(done) == len(builds) +} + +// until hears the builds' keys until each holds a build that ended holds, or ctx ends, and answers those that do, by module. +func (e buildEnds) until(ctx context.Context, builds map[string]string, ended func(delivery.BuilderBuild) bool) (map[string]delivery.BuilderBuild, error) { byKey := map[string]string{} keys := make([]string, 0, len(builds)) for module, fp2 := range builds { @@ -315,22 +327,22 @@ func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[str byKey[key] = module keys = append(keys, key) } - ended := map[string]delivery.BuilderBuild{} + out := map[string]delivery.BuilderBuild{} if len(keys) == 0 { - return ended, nil + return out, nil } w, err := e.kv.WatchFiltered(ctx, keys) if err != nil { return nil, err } defer func() { _ = w.Stop() }() - for len(ended) < len(keys) { + for len(out) < len(keys) { select { case <-ctx.Done(): - return ended, nil // the bound: the builds not ended are left out + return out, nil // the bound: the builds not ended are left out case entry, open := <-w.Updates(): if !open { - return ended, nil + return out, nil } if entry == nil || entry.Operation() != jetstream.KeyValuePut { continue // the end of the values now, or a key deleted: it has not ended @@ -340,17 +352,14 @@ func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[str continue } b, err := delivery.DecodeBuilderBuild(entry.Value()) - if err != nil { - continue // not a build the check can read: it has not ended for the check - } - if b.State == delivery.BuildDone || b.State == delivery.BuildFailed { - ended[module] = b + if err == nil && ended(b) { + out[module] = b } else { - delete(ended, module) // asked again (builder_build_again): it runs once more + delete(out, module) // asked again (builder_build_again), or not a build the check can read: not ended } } } - return ended, nil + return out, nil } // ManifestDigests is the manifest digest of every module the indexer holds, by module: the catalogue the indexer's judge diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index 8ca5d68c..1cc2b977 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -170,6 +170,9 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro if kv := process.OptionalBucket(ctx, js, delivery.BuilderBuildsBucket, "the line says nothing of the builds, and a check composes without waiting on them, until the checker starts again", logger.Printf); kv != nil { builds, ends = buildReader{kvStore{kv}}, buildEnds{kv} } + if ends != nil { + ch.Rebuilds = ends // a check failed on a build is asked again once that build is done again (ADR 0603 §10) + } // The check's composition is the checker's, from what each placed node's engine answers (ADR 0593 §7, §9). set, err := readSettings(getenv("MESH_CHECKER_SETTINGS")) if err != nil { diff --git a/modules/mesh-delivery-checker/internal/check/builds.go b/modules/mesh-delivery-checker/internal/check/builds.go index 68e51c7c..2afa2d42 100644 --- a/modules/mesh-delivery-checker/internal/check/builds.go +++ b/modules/mesh-delivery-checker/internal/check/builds.go @@ -80,6 +80,37 @@ type BuildEnds interface { // Ended waits until the build of every module at its fingerprint (module to fingerprint) is done or failed, or ctx ends, // and answers the builds that ended, by module. An error is only builds that cannot be heard at all. Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) + // Done waits until the build of every module at its fingerprint is done (a failed one built again, builder_build_again), + // and says whether they all are; false when ctx ends first or they cannot be heard. + Done(ctx context.Context, builds map[string]string) bool +} + +// BuildFailedTest is what a failure of the check names a failed build of the candidate by: the test `build: `. +// A verdict failed on builds is asked again by the checker itself once each is built again (Checker.Rebuilt). +const BuildFailedTest = "build: " + +// failedBuilds are the candidate's builds a failed verdict names as failed (BuildFailedTest), by module, at the fingerprint +// the case carries for it; none for a verdict that names no failed build. +func failedBuilds(v delivery.Verdict, cs *Case) map[string]string { + if v.State != delivery.VerdictFail { + return nil + } + out := map[string]string{} + for _, f := range append([]delivery.FailedFirst{v.Failed}, v.Failures...) { + module, named := strings.CutPrefix(f.Test, BuildFailedTest) + if !named { + continue + } + for _, p := range cs.Pulls { + if cm, ok := p.Fps[module]; ok && cm.FP2 != "" { + out[module] = cm.FP2 + } + } + } + if len(out) == 0 { + return nil + } + return out } func (j *Examiner) buildWait() time.Duration { @@ -99,6 +130,10 @@ func (st *state) awaitBuilds(ctx context.Context, v *delivery.Verdict) []string return words } modules := slices.Sorted(maps.Keys(wait)) + if v.State == delivery.VerdictFail { + // The check has failed already: its red status is never held back by a build (the lead, 2026-11-03). + return append(words, fmt.Sprintf("the check failed before its builds were waited on, so it composes at once, without waiting on the builds of %s (ADR 0603 §10)", strings.Join(modules, ", "))) + } if st.j.BuildEnds == nil { return append(words, fmt.Sprintf("the builds of %s cannot be heard, so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "))) } @@ -120,7 +155,7 @@ func (st *state) awaitBuilds(ctx context.Context, v *delivery.Verdict) []string case !done: late = append(late, module) case b.State == delivery.BuildFailed: - f := delivery.FailedFirst{Test: "build: " + module, Words: fmt.Sprintf("the candidate's build at %s failed (builder_build_again builds it again, then mesh-delivery.recheck asks the check again): %s", wait[module], b.Error)} + f := delivery.FailedFirst{Test: BuildFailedTest + module, Words: fmt.Sprintf("the candidate's build at %s failed (rebuild with builder_build_again; the check asks itself again when the build is done): %s", wait[module], b.Error)} if v.State != delivery.VerdictFail { v.State, v.Failed = delivery.VerdictFail, f continue diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 3fbffddb..4e8e91bc 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "maps" + "slices" "sort" "strings" "sync" @@ -79,6 +81,9 @@ type Checker struct { } // Bound is how long one check may run; none when 0 (DefaultBound). Bound time.Duration + // Rebuilds hears the builder's build keys a failed verdict names (failedBuilds): once each is written done, the check of + // those heads is asked again by the checker itself (ADR 0603 §10), as a failure of the nodes is; nil asks none again. + Rebuilds BuildEnds // Asks keeps the asks each running check put, by verdict key (AsksState), for checker_ask_again; nil keeps none. Asks AsksKV // Link is the base of the address a status points at; the verdict key follows. @@ -104,6 +109,8 @@ type Checker struct { held map[string]string // by verdict key: the turn that holds its check off, as last said live map[string]liveCheck // by verdict key: the check running in this process, ended when its heads go settling map[string]*window // by candidates key: the window its heads settle in + rebuilt map[string]bool // by verdict key: every failed build its verdict names is done again + awaiting map[string]bool // by verdict key: its failed builds are heard (awaitRebuilt) // outsideSaid are the candidates outside the mesh said in the log, by group and heads. outsideSaid map[string]bool } @@ -162,6 +169,7 @@ func (ch *Checker) init() { ch.known, ch.inflight, ch.again, ch.tried = map[string]delivery.Candidate{}, map[string]bool{}, map[string]bool{}, map[string]bool{} ch.nodesFail, ch.held = map[string]uint64{}, map[string]string{} ch.live, ch.settling = map[string]liveCheck{}, map[string]*window{} + ch.rebuilt, ch.awaiting = map[string]bool{}, map[string]bool{} ch.outsideSaid = map[string]bool{} }) } @@ -380,8 +388,9 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat // Every check is asked at once: the build seats' streams hold the backlog, a new head's check and a check after a // failure of the nodes on check, a re-judge after a trunk move on recheck, pulled after them (ADR 0525 §4, §9; // ADR 0536 §4). The checker holds no place and no capacity of its own. - ch.prepare(ctx, cs) + ch.prepare(ctx, key, cs) ch.try(ctx, run, key, cs) + ch.hearFailedBuilds(ctx, key, cs) // a check it failed on a build is asked again once that build is done again ch.mu.Lock() more := ch.again[vkey] delete(ch.again, vkey) @@ -518,7 +527,7 @@ const ( // or sibling commits that moved since is carried, its packages run only where the move changed their digest, and a pass's // commits are named as the carry's CarriedFrom. A verdict that stands is ranked with the carries: its claim finds nothing // to do. -func (ch *Checker) prepare(ctx context.Context, cs *Case) { +func (ch *Checker) prepare(ctx context.Context, group string, cs *Case) { cs.Kind, cs.CarriedFrom, cs.Composition = KindNew, nil, nil value, _, found, err := ch.Verdicts.Get(ctx, cs.Key) if err != nil || !found { @@ -531,6 +540,10 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { case IsNodesFailure(v): cs.Kind = KindNodes return + case failedBuilds(v, cs) != nil: + cs.Kind = KindNodes // asked again on check once its builds are done again (awaitRebuilt), never by a recheck + ch.awaitRebuilt(ctx, group, cs.Key, failedBuilds(v, cs)) + return case IsConflictFailure(v): return // a head git refused: its next check is whole (ADR 0548 §7) case IsEnded(v), IsUntrunked(v): @@ -687,6 +700,8 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 return nil, nil case IsEnded(cur), ch.stale(cur, cs): retake = true // heads held again after their check was ended unjudged (issue 1890), or a trunk that moved + case cur.State == delivery.VerdictFail && ch.takeRebuilt(cs.Key): + retake = true // every failed build it names is done again (ADR 0603 §10) case cur.State == delivery.VerdictFail && IsNodesFailure(cur): ch.mu.Lock() at, ok := ch.nodesFail[cs.Key] @@ -1070,3 +1085,61 @@ func (ch *Checker) movedSince(cs *Case) bool { } return false } + +// awaitRebuilt hears, once per verdict key, the builder's keys of the builds a failed verdict names: when each is written +// done again (builder_build_again), the check of those heads is asked again (claim retakes it), as a failure of the nodes +// is after the nodes changed, so no recheck by hand is needed (ADR 0603 §10). Heard on the builder's own writes, never +// polled; it ends with the checker. +func (ch *Checker) awaitRebuilt(ctx context.Context, group, vkey string, builds map[string]string) { + if ch.Rebuilds == nil { + return + } + ch.mu.Lock() + if ch.awaiting[vkey] { + ch.mu.Unlock() + return + } + ch.awaiting[vkey] = true + ch.mu.Unlock() + ch.wg.Add(1) + go func() { + defer ch.wg.Done() + done := ch.Rebuilds.Done(ctx, builds) + ch.mu.Lock() + delete(ch.awaiting, vkey) + if done { + ch.rebuilt[vkey] = true + } + c, known := ch.known[group] + ch.mu.Unlock() + if done && known && ctx.Err() == nil { + ch.logf("%s: the builds of %s it failed on are done again, so it is checked again (ADR 0603 §10)", vkey, strings.Join(slices.Sorted(maps.Keys(builds)), ", ")) + ch.consider(ctx, group, c) + } + }() +} + +// hearFailedBuilds starts awaitRebuilt for the case's verdict when it names failed builds (failedBuilds). +func (ch *Checker) hearFailedBuilds(ctx context.Context, group string, cs *Case) { + if ch.Rebuilds == nil || ctx.Err() != nil { + return + } + value, _, found, err := ch.Verdicts.Get(ctx, cs.Key) + if err != nil || !found { + return + } + if v, err := delivery.DecodeVerdict(value); err == nil { + if builds := failedBuilds(v, cs); builds != nil { + ch.awaitRebuilt(ctx, group, cs.Key, builds) + } + } +} + +// takeRebuilt says whether every failed build the verdict of vkey names is done again, and forgets it: one retake each. +func (ch *Checker) takeRebuilt(vkey string) bool { + ch.mu.Lock() + defer ch.mu.Unlock() + was := ch.rebuilt[vkey] + delete(ch.rebuilt, vkey) + return was +} diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index 53f42d07..72ec4ecb 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -125,6 +125,8 @@ func (f *fakeEnds) Ended(_ context.Context, builds map[string]string) (map[strin return out, nil } +func (f *fakeEnds) Done(context.Context, map[string]string) bool { return false } + // knownRuns are the modules the trunk holds a record of (the indexer's modules): any commit. type knownRuns []string @@ -153,7 +155,8 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { runs knownRuns // nil: the trunk holds every module want []string waited string // the modules waited on, sorted - failed string // the failure's test, when the check fails + failed string // the failure's test, when the check fails on a build + tests bool // the tests fail: the check fails before it would wait }{ {name: "the Go build module changes", manifest: map[string]catalogue.Manifest{"mesh-build-go": {Module: "mesh-build-go", Build: goBuild}, "web": {Module: "web", Build: goBuild}, @@ -175,6 +178,9 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { {name: "a build fails", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "mail": {Module: "mail", Build: goBuild}}, builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildFailed, Error: "go build: undefined: x"}, "mail": done}, waited: "mail, web", failed: "build: web"}, + {name: "the tests fail", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildBuilding}}, tests: true, + want: []string{"composes at once, without waiting on the builds of web"}}, {name: "a build does not end within the bound", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildBuilding}}, want: []string{"the builds of web did not end within the build-wait-bound"}, waited: "web"}, @@ -185,6 +191,13 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { want: []string{"the builds of web cannot be heard"}}, } { b := candidateBench(t, true) + if row.tests { + b.test = func(c stepCall) (any, error) { + r := captured[Tested](t, "go-test-failing") + r.Level, _ = c.args["level"].(string) + return r, nil + } + } comps := &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionPass, SDK: "x", Machines: []string{"g14"}}} b.ex.Compositions = comps var ends *fakeEnds @@ -216,9 +229,12 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { } } switch { - case row.failed != "" && (v.State != delivery.VerdictFail || v.Failed.Test != row.failed || !strings.Contains(v.Failed.Words, "undefined: x")): - t.Errorf("%s: the verdict is %s failing %+v, want it failed naming %s and the builder's words", row.name, v.State, v.Failed, row.failed) - case row.failed == "" && v.State != delivery.VerdictPass: + case row.failed != "" && (v.State != delivery.VerdictFail || v.Failed.Test != row.failed || !strings.Contains(v.Failed.Words, "undefined: x") || + !strings.Contains(v.Failed.Words, "the check asks itself again when the build is done")): + t.Errorf("%s: the verdict is %s failing %+v, want it failed naming %s, the builder's words and how it is asked again", row.name, v.State, v.Failed, row.failed) + case row.tests && v.State != delivery.VerdictFail: + t.Errorf("%s: %+v, want the tests' failure", row.name, v) + case row.failed == "" && !row.tests && v.State != delivery.VerdictPass: t.Errorf("%s: %+v", row.name, v) } words := compositionWords(v.Composition, v.Took.Composition) diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 2086eced..1fc0efbe 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -350,7 +350,7 @@ type Status struct { Lost map[string]string `json:"lost"` // BuildsNotEnded are, of each candidate whose check runs, the builder's builds of its changed modules that are neither // done nor failed, by group: ` `, sorted (no key yet is `not begun`). A check waits on them before it - // composes (ADR 0603 §10), but for the modules its line names as composed without them; builder_build_again builds a failed one again. + // composes (ADR 0603 §10), but for the modules its line names as composed without them; builder_build_again builds a failed one again, and the check asks itself again when it is done. BuildsNotEnded map[string]string `json:"builds_not_ended,omitempty"` // DeadLetters says whether the checker hears the asks the build seats give up on (issue 734): deaf, a try or check // whose ask is given up waits until its bound.