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 0f369398..01dc5825 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 @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "os" + "strings" "sync/atomic" "time" @@ -516,6 +517,7 @@ func main() { return err }} trying := Trying{Tries: store, Ender: store, Trunks: trunks} + r.Backlog = backlogOf(stdio.Ask) // the build seats' streams, for checker_ask_again (issue 1373) verdicts := stdio.State("verdicts") written := stateStore{entry: func(key string) (json.RawMessage, uint64, error) { reads.Add(1) @@ -542,3 +544,32 @@ func main() { os.Exit(1) } } + +// buildUsageTool is the delivery view's tool of the build nodes' use, whose backlog is the build seats' streams (issue 1373). +const buildUsageTool = "mesh-delivery-view.build_usage" + +// backlogOf reads the build seats' backlog through the view's tool, asked through ask (stdio.Ask, or a test's). +func backlogOf(ask func(string, any) (json.RawMessage, error)) func() ([]check.BacklogRow, error) { + return func() ([]check.BacklogRow, error) { + reads.Add(1) + raw, err := ask(buildUsageTool, map[string]any{}) + if err != nil { + return nil, fmt.Errorf("%s does not answer: %w", buildUsageTool, err) + } + answer, said, ok := buscalls.Unwrap(raw) + if !ok { + return nil, fmt.Errorf("%s answered: %s", buildUsageTool, said) + } + var a struct { + Backlog []check.BacklogRow `json:"backlog"` + Notes []string `json:"notes"` + } + if err := json.Unmarshal(answer, &a); err != nil { + return nil, fmt.Errorf("%s answered what does not decode: %w", buildUsageTool, err) + } + if len(a.Notes) > 0 { // the backlog was not read: unread is never empty + return nil, fmt.Errorf("%s did not read the backlog: %s", buildUsageTool, strings.Join(a.Notes, "; ")) + } + return a.Backlog, nil + } +} diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go index 2856b698..b42ad7e3 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go @@ -167,6 +167,20 @@ func TestTheToolsServedAreTheFiveReadTools(t *testing.T) { } }) } + // 1373: the build seats' backlog is read through build_usage; one it says it did not read is never taken for empty. + for _, c := range []struct { + name, answer string + rows int + failed bool + }{ + {name: "a backlog read", answer: `{"backlog":[{"language":"go","kind":"check","waiting":2}]}`, rows: 1}, + {name: "a backlog with a note says it was not read", answer: `{"backlog":[],"notes":["the seats' streams cannot be read here"]}`, failed: true}, + } { + rows, err := backlogOf(func(string, any) (json.RawMessage, error) { return json.RawMessage(c.answer), nil })() + if (err != nil) != c.failed || len(rows) != c.rows || (c.failed && !strings.Contains(err.Error(), "cannot be read here")) { + t.Errorf("1373 backlog, %s: %v, %v", c.name, rows, err) + } + } // With nothing to read from a tool says so and changes nothing. for _, tl := range Tools(check.Reader{}) { if tl.Name == check.StatusReadTool { diff --git a/modules/mesh-delivery-checker/internal/check/askagain.go b/modules/mesh-delivery-checker/internal/check/askagain.go index a92e9b2f..9be3f0e6 100644 --- a/modules/mesh-delivery-checker/internal/check/askagain.go +++ b/modules/mesh-delivery-checker/internal/check/askagain.go @@ -150,6 +150,14 @@ func (r Reader) AskAgain(w VerdictEntry, group, why string) (AskedAgain, error) if err == nil && len(kept) > 0 { err = json.Unmarshal(kept, &put) } + if err == nil && len(put.Asks) == 0 && r.Backlog != nil { + // Issue 1373: the checker names no ask, so the build seats' check streams are read: with no check ask waiting or held + // on any of them (and no step of the candidate running, above) none of it can run, and it is asked anew. + if err := r.noCheckAskWaits(); err != nil { + return AskedAgain{}, err + } + return r.askedAgain(w, key, rev, v, why) + } if err != nil || len(put.Asks) == 0 { return AskedAgain{}, fmt.Errorf("the checker names no ask of %s (%v), so whether one waits in a build seat's stream cannot be told; asking again could run it twice", key, err) } @@ -235,3 +243,35 @@ func (ch *Checker) VerdictWritten(ctx context.Context, key string, value []byte) ch.logf("%s: %s; asked again", key, failedOf(v).Words) ch.consider(ctx, group, cand) } + +// BacklogRow is a row of the build seats' backlog as mesh-delivery-view's build_usage answers it (its backlog): the asks +// waiting and held on one language's stream of one kind of ask, or why it could not be read. +type BacklogRow struct { + Language string `json:"language"` + Kind string `json:"kind"` + Waiting uint64 `json:"waiting"` + Held uint64 `json:"held"` + Error string `json:"error,omitempty"` +} + +// noCheckAskWaits is nil when no check or recheck ask waits or is held on any build seat's stream and every row was read; else it +// names the stream that holds asks. It cannot tell whose ask one is, so any check ask refuses: asking again could run it twice. +func (r Reader) noCheckAskWaits() error { + rows, err := r.Backlog() + if err != nil { + return fmt.Errorf("the build seats' streams cannot be read (%v), so whether an ask of the check waits in one cannot be told; asking again could run it twice", err) + } + for _, row := range rows { + if row.Kind != "check" && row.Kind != "recheck" { // both kinds are the checker's asks (judge.go) + continue + } + if row.Error != "" { + return fmt.Errorf("the %s build seat's check stream cannot be read (%s), so whether an ask of the check waits in it cannot be told", row.Language, row.Error) + } + if row.Waiting > 0 || row.Held > 0 { + return fmt.Errorf("the %s build seat's check stream holds asks (%d waiting, %d held), one of them may be the check's: asking again could run it twice; "+ + "the seats' backlog is in mesh-delivery-view.build_usage", row.Language, row.Waiting, row.Held) + } + } + return nil +} diff --git a/modules/mesh-delivery-checker/internal/check/lostask_test.go b/modules/mesh-delivery-checker/internal/check/lostask_test.go index 378a9ad0..d1073b8d 100644 --- a/modules/mesh-delivery-checker/internal/check/lostask_test.go +++ b/modules/mesh-delivery-checker/internal/check/lostask_test.go @@ -328,6 +328,34 @@ func TestACheckWhoseAskJudgedNothingIsAskedAgainOrRead(t *testing.T) { if st, _ := rd.Status(); !strings.Contains(st.Lost["feat/x"], AskAgainTool) { t.Errorf("status lost = %v, want feat/x named with %s", st.Lost, AskAgainTool) } + // Issue 1373: a check that names no ask is asked again only when no check ask waits or is held in any build seat's stream. + { + raw, _, _, _ := r.verdicts.Get(ctx, key) + vcopy := newMemKV() + _, _ = vcopy.Create(ctx, key, raw) + noAsk := rd + noAsk.AskState = kvRead(newMemKV()) + for _, c := range []struct { + name string + rows []BacklogRow + bad string + }{{"a check ask waits", []BacklogRow{{Language: "go", Kind: "check", Waiting: 1}}, "holds asks"}, + {"a check ask is held", []BacklogRow{{Language: "go", Kind: "check", Held: 1}}, "holds asks"}, + {"a recheck ask waits", []BacklogRow{{Language: "go", Kind: "recheck", Waiting: 1}}, "holds asks"}, + {"a recheck ask is held", []BacklogRow{{Language: "go", Kind: "recheck", Held: 1}}, "holds asks"}, + {"a stream cannot be read", []BacklogRow{{Language: "go", Kind: "check", Error: "no worker"}}, "cannot be read"}, + {"only a delivery ask waits", []BacklogRow{{Language: "go", Kind: "delivery", Waiting: 3}}, ""}} { + noAsk.Backlog = func() ([]BacklogRow, error) { return c.rows, nil } + w := VerdictEntry(asVerdicts{r.verdicts}) + if c.bad == "" { + w = asVerdicts{vcopy} + } + got, err := noAsk.AskAgain(w, "feat/x", "no ask") + if (c.bad == "") != (err == nil) || (c.bad != "" && !strings.Contains(err.Error(), c.bad)) || (c.bad == "" && !strings.HasPrefix(got.Said, AskedAgainWords)) { + t.Errorf("a check naming no ask, %s: %+v, %v", c.name, got, err) + } + } + } got, err := rd.AskAgain(asVerdicts{r.verdicts}, "feat/x", "its watch went deaf") if err != nil || got.Key != key || !strings.HasPrefix(got.Said, AskedAgainWords) { t.Fatalf("asked again = %+v, %v", got, err) diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 3840af64..ee8f7f1f 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -63,7 +63,9 @@ type Reader struct { BuildState StateRead // AskState is the checker's state asks: the asks each running check put (AsksState), which checker_ask_again reads. AskState StateRead - Now func() time.Time + // Backlog is the build seats' backlog (mesh-delivery-view.build_usage); checker_ask_again reads it for a check that names no ask. + Backlog func() ([]BacklogRow, error) + Now func() time.Time } // VerdictRead is a verdict as stored with every failure it names, one line each, and the full report of the status of each diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index 732f2af4..1b393318 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -107,6 +107,11 @@ func sorted(s []string) []string { func TestManifestDeclarations(t *testing.T) { // 1678/AC1 m, top := load(t) + // 1373: checker_ask_again asks the delivery view's build_usage for the build seats' backlog; the list below is the rest. + if !slices.Contains(m.Invokes, "mesh-delivery-view.build_usage") { + t.Error("invokes lack mesh-delivery-view.build_usage, which checker_ask_again asks for the build seats' streams") + } + m.Invokes = slices.DeleteFunc(slices.Clone(m.Invokes), func(s string) bool { return s == "mesh-delivery-view.build_usage" }) if m.Module != "mesh-delivery-checker" { t.Errorf("module = %q", m.Module) } diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index b2e3e7e9..9116b3ff 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -96,6 +96,7 @@ "seat:mesh-delivery.source", "seat:mesh-catalog.seat-definitions", "seat:mesh-config.renames", + "mesh-delivery-view.build_usage", "seat:mesh-hand-acts.hand-act", "seat:mesh-module-assigner.assignments", "seat:node-build-go.drop",