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 4422450..0bc71c9 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 @@ -105,7 +105,7 @@ func TryTools(tr Trying) []stdio.Tool { {Name: check.TryTool, Description: "Try a pushed head before its pull request is opened: the checker judges it as it judges a pull request's head, merged onto its trunk's head, " + "its unit and integration levels through the build modules, and owes the forge nothing. Answers the try's id at once; " + check.TryReadTool + " reads " + - "where it stands. A try of the same head that waits or runs is the answer to a second ask; at most 4 wait or run at once, mesh-wide. A repository " + + "where it stands. A try of the same head that waits or runs is the answer to a second ask; at most 4 run on a build node and 8 wait for one at once, mesh-wide (a try queued behind the seat's checks holds no node). A repository " + "the checker checks no level of (one whose trunk the gate does not name) is refused. A try whose ask no build node took within the step " + "bound ends as error; its ask stays in the build seat's stream and may still run later, judged by nobody (issue 734).", Input: map[string]any{"type": "object", "required": []string{"repository", "head"}, "properties": map[string]any{ diff --git a/modules/mesh-delivery-checker/internal/check/try.go b/modules/mesh-delivery-checker/internal/check/try.go index a44b3b4..c008011 100644 --- a/modules/mesh-delivery-checker/internal/check/try.go +++ b/modules/mesh-delivery-checker/internal/check/try.go @@ -30,8 +30,13 @@ const ( // TriesState is the checker's state of tries: the bucket mesh-delivery-checker_tries, keys try-. TriesState = "tries" TriesBucket = delivery.CheckerModule + "_" + TriesState - // MaxTries is how many tries may wait or run at once, mesh-wide: each holds a build node for minutes. + // MaxTries is how many tries may hold a build node at once, mesh-wide: each holds one for minutes. A try whose asks no + // node has taken yet waits behind the seat's other asks and holds none, so it counts toward MaxTriesWaiting instead + // (issue 1890: at 19:53 on 2026-10-26 four tries sat queued behind 44 checks for up to 47 minutes and no fifth could be + // asked). MaxTries = 4 + // MaxTriesWaiting is how many tries may wait at once, queued or with their asks not yet taken by a node. + MaxTriesWaiting = 8 // TryScan is how many of the newest tries a new ask reads to find the ones that wait or run (issue 1827). TryScan = 64 // TryKeep is how long an ended try's key is kept before the checker removes it. @@ -166,6 +171,18 @@ func (w *tryWriter) end() (Try, uint64) { return w.t, w.rev } +// HoldsANode says whether the try holds a build node now: it runs, and a step of its asks runs on a node, or it names +// no step (a checker that writes its asks only at the end, or one about to put its first). +func (t Try) HoldsANode() bool { + if t.State != TryRunning { + return false + } + if len(t.Steps) == 0 { + return true + } + return slices.ContainsFunc(t.Steps, func(s TryStep) bool { return s.State == delivery.StepRunning }) +} + // Over says whether the try came to its end. func (t Try) Over() bool { return t.State == TryPass || t.State == TryFail || t.State == TryError } @@ -269,7 +286,7 @@ func AskTry(store TryStore, trunks map[string]string, a TryAsk, now time.Time) ( }); err != nil { return "", err } - var open []string + var holding, waiting []string for i, cur := range tries { if !live[i] { continue @@ -277,10 +294,17 @@ func AskTry(store TryStore, trunks map[string]string, a TryAsk, now time.Time) ( if cur.Repository == t.Repository && cur.Head == t.Head && cur.Trunk == t.Trunk && cur.Tests == t.Tests && cur.Level == t.Level { return cur.ID, nil } - open = append(open, fmt.Sprintf("%s (%s@%.12s, %s)", cur.ID, cur.Repository, cur.Head, cur.State)) + said := fmt.Sprintf("%s (%s@%.12s, %s)", cur.ID, cur.Repository, cur.Head, cur.State) + if cur.HoldsANode() { + holding = append(holding, said) + } else { + waiting = append(waiting, said) + } } - if len(open) >= MaxTries { - return "", fmt.Errorf("%d tries wait or run, the most at once: %s; ask again when one ends", len(open), strings.Join(open, ", ")) + // Tries on a build node and tries waiting for one are bounded apart: a try queued behind the seat's checks holds no node. + if len(holding) >= MaxTries || len(waiting) >= MaxTriesWaiting { + return "", fmt.Errorf("%d tries run on a build node (the most at once: %d) and %d wait for one (the most: %d): running %s; waiting %s; ask again when one ends", + len(holding), MaxTries, len(waiting), MaxTriesWaiting, orNone(holding), orNone(waiting)) } t.ID = "try-" + strconv.FormatInt(now.UnixNano(), 10) data, err := encodeTry(t) @@ -553,3 +577,11 @@ func judgeWithin(ctx context.Context, j Judge, cs *Case, bound time.Duration) (O } return out, true } + +// orNone joins the tries said, or says none. +func orNone(said []string) string { + if len(said) == 0 { + return "none" + } + return strings.Join(said, ", ") +} diff --git a/modules/mesh-delivery-checker/internal/check/try_test.go b/modules/mesh-delivery-checker/internal/check/try_test.go index cee1241..e6b2427 100644 --- a/modules/mesh-delivery-checker/internal/check/try_test.go +++ b/modules/mesh-delivery-checker/internal/check/try_test.go @@ -3,6 +3,7 @@ package check import ( "context" "encoding/json" + "fmt" "strings" "testing" "time" @@ -67,14 +68,38 @@ func TestATryIsAskedQueuedOncePerHeadAndAtMostFourAtOnce(t *testing.T) { // ADR if err != nil || again != id { t.Errorf("a second ask of the same head answered %q, %v, want %q", again, err, id) } - heads := []string{head2, trunkA2, trunkS2} - for i, h := range heads { - if _, err := AskTry(store, tryTrunks, TryAsk{Repository: repoA, Head: h, Trunk: trunkA}, at.Add(time.Duration(i+2)*time.Second)); err != nil { - t.Fatalf("try %d: %v", i+2, err) - } + // Issue 1890: tries on a build node (a step of theirs running, or none named yet) and tries waiting for one (queued, or + // their asks not taken) are bounded apart, MaxTries and MaxTriesWaiting; the refusal names both counts. + onNode := func(i int) Try { + return Try{ID: fmt.Sprintf("try-%d", i), Repository: repoA, Head: fmt.Sprintf("%040x", i), Trunk: trunkA, State: TryRunning, + Steps: []TryStep{{Ask: "a", State: delivery.StepRunning}}} } - if _, err := AskTry(store, tryTrunks, TryAsk{Repository: repoSDK, Head: head1}, at.Add(time.Minute)); err == nil || !strings.Contains(err.Error(), "4 tries wait or run") { - t.Errorf("a fifth try answered %v, want refused naming the four", err) + waitingFor := func(i int) Try { + return Try{ID: fmt.Sprintf("try-%d", i), Repository: repoA, Head: fmt.Sprintf("%040x", i), Trunk: trunkA, State: TryRunning, + Steps: []TryStep{{Ask: "a", State: TryAsked}}} + } + for _, c := range []struct { + what string + holding, wait int + refused string + }{ + {"four queued behind the checks, none on a node: a fifth is taken", 0, 4, ""}, + {"four on a node", MaxTries, 0, "4 tries run on a build node (the most at once: 4) and 0 wait"}, + {"three on a node, eight waiting", 3, MaxTriesWaiting, "3 tries run on a build node (the most at once: 4) and 8 wait for one (the most: 8)"}, + } { + kv := newMemKV() + for i := range c.holding + c.wait { + tr := waitingFor(i + 1) + if i < c.holding { + tr = onNode(i + 1) + } + data, _ := encodeTry(tr) + _, _ = kv.Create(context.Background(), tr.ID, data) + } + _, err := AskTry(memTries{kv}, tryTrunks, TryAsk{Repository: repoSDK, Head: head1}, at.Add(time.Minute)) + if (c.refused == "") != (err == nil) || (err != nil && !strings.Contains(err.Error(), c.refused)) { + t.Errorf("%s: answered %v, want refused %q", c.what, err, c.refused) + } } }