Merge pull request 'checker_ask_again recovers a lost check that names no ask, when no check ask waits in any build stream' (#210) from fix/1373-ask-again-no-ask into main

modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go

@@ -7,6 +7,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 	"sync/atomic"
 	"time"
 
@@ -471,6 +472,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)
@@ -496,3 +498,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
+	}
+}

modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go

@@ -165,6 +165,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 {

modules/mesh-delivery-checker/internal/check/askagain.go

@@ -151,6 +151,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)
 	}
@@ -236,3 +244,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
+}

modules/mesh-delivery-checker/internal/check/lostask_test.go

@@ -327,6 +327,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)

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

modules/mesh-delivery-checker/manifest_test.go

@@ -105,6 +105,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)
 	}

modules/mesh-delivery-checker/module.json

@@ -93,6 +93,7 @@
     "mesh-delivery-indexer.indexer_read_seat_definitions",
     "seat:mesh-catalog.seat-definitions",
     "seat:mesh-config.renames",
+    "mesh-delivery-view.build_usage",
     "seat:mesh-hand-acts.hand-act",
     "seat:node-build-go.drop",
     "seat:node-build-node.drop",