From 5085b681e9338265a31831a8e7db7d0a8dc1c4ef Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 1 Nov 2026 15:36:23 +0100 Subject: [PATCH] Count no decline as an asking; bound declines by time and name the nodes (ADR 0584, issue 1578) A build node off the agreed toolchain declines the ask and, idle, takes the ask put again at once, so three declines used the three askings and failed the build. A decline now uses no asking: the ask is put again after a short pause, and past a bound from the first decline the key fails naming the declining nodes and their reasons. A person reads which node sits out in the key's step text, which the view's builder_builds answers as stored. Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../internal/build/askagain.go | 76 ++++++++++++++++++- .../internal/build/run.go | 3 + .../internal/build/run_test.go | 19 +++++ 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/modules/mesh-delivery-builder/internal/build/askagain.go b/modules/mesh-delivery-builder/internal/build/askagain.go index 23a0974..a7d6cd6 100644 --- a/modules/mesh-delivery-builder/internal/build/askagain.go +++ b/modules/mesh-delivery-builder/internal/build/askagain.go @@ -6,6 +6,7 @@ import ( "fmt" "regexp" "slices" + "sort" "strconv" "strings" "time" @@ -177,6 +178,42 @@ func (b *Builder) drop(ctx context.Context, ask delivery.StepAsk, old string) { } } +// DefaultDeclineBound is how long one ask may go on being declined, from its first decline, before its key fails (the +// checker's DefaultDeclineBound, issue 890); DefaultDeclinePause is how long the builder waits before it puts a declined +// ask again, so a lone node off the agreed toolchain cannot spin on it. +const ( + DefaultDeclineBound = 30 * time.Minute + DefaultDeclinePause = 5 * time.Second +) + +func (b *Builder) declineBound() time.Duration { + if b.DeclineBound > 0 { + return b.DeclineBound + } + return DefaultDeclineBound +} + +func (b *Builder) declinePause() time.Duration { + if b.DeclinePause != 0 { + return max(b.DeclinePause, 0) + } + return DefaultDeclinePause +} + +// declinedWords names the nodes that declined an ask and why, in the order of their names. +func declinedWords(by map[string]string) string { + nodes := make([]string, 0, len(by)) + for n := range by { + nodes = append(nodes, n) + } + sort.Strings(nodes) + var parts []string + for _, n := range nodes { + parts = append(parts, n+": "+by[n]) + } + return strings.Join(parts, "; ") +} + func (b *Builder) askings() int { if b.Askings > 0 { return b.Askings @@ -255,6 +292,8 @@ func (b *Builder) unlisten(key, language string, c chan string) { var ( errCheck = errors.New("the check bound passed") errAgain = errors.New("asked again") + // errDeclined: a build node on another toolchain than the agreed one sat out of the ask (ADR 0584 clause 2). + errDeclined = errors.New("declined") ) type byHand struct{ why string } @@ -268,11 +307,40 @@ func (b *Builder) answer(ctx context.Context, l *lane, ask delivery.StepAsk, ask again := b.listen(key, ask.Language) defer b.unlisten(key, ask.Language, again) asking := askingOf(asked.step, l.module) + var firstDecline time.Time + declines := map[string]string{} // node -> why it declined, the last for { step, why, err := b.waitOnce(ctx, ask, again) switch { case errors.Is(err, errCheck): continue + case errors.Is(err, errDeclined): + // Not an asking: a node that sits out would use them all while it idles and pulls the ask again at once. + // Bounded by time from the first decline, then failed loudly naming the nodes and their reasons (as the + // checker's decline loop is, issue 890). + now := time.Now() + if firstDecline.IsZero() { + firstDecline = now + } + declines[step.Node] = why + if bound := b.declineBound(); now.Sub(firstDecline) > bound { + return step, ask, fmt.Errorf("the ask %s of %s was declined for %s and no build node took it: %s", ask.ID, l.module, bound.Round(time.Second), declinedWords(declines)) + } + select { + case <-ctx.Done(): + return step, ask, ctx.Err() + case <-time.After(b.declinePause()): + } + old := ask.ID + ask.ID, ask.At = buildstep.NewID(), time.Now() + if err := asked.note(ctx, ask.Language, ask.ID, fmt.Sprintf("put %s again after %s declined it", l.module, step.Node)); err != nil { + return step, ask, fmt.Errorf("the ask of %s put again after a decline could not be written to the key: %w", l.module, err) + } + if err := b.Asks.Put(ctx, ask); err != nil { + return step, ask, err + } + b.drop(ctx, ask, old) + b.logf("%s: the ask %s of %s was declined on %s and is put again as %s: %s", key, old, l.module, step.Node, ask.ID, why) case errors.Is(err, errAgain): if asking >= b.askings() { return step, ask, fmt.Errorf("the ask %s could not be answered and was asked %d times, the most it is: %s", ask.ID, asking, why) @@ -317,10 +385,10 @@ func (b *Builder) waitOnce(ctx context.Context, ask delivery.StepAsk, again <-ch defer stop() } step, err := b.Asks.Wait(wait, ask) - if d, declined := delivery.DeclinedOf(step); err == nil && declined { - // A build node on another toolchain than the agreed one sat out (ADR 0584 clause 2): the ask is put again, for a node - // on the agreed one to take. The most askings stays the bound, and the build then fails with these reasons in it. - return step, strings.TrimSpace(step.Node + " declined: " + step.Reason + " (lacks " + strings.Join(d.Lacks, ", ") + ")"), errAgain + if _, declined := delivery.DeclinedOf(step); err == nil && declined { + // A build node off the agreed toolchain sat out (ADR 0584 clause 2). It is no asking: answer() puts the ask again + // after a pause, within DeclineBound. + return step, step.Reason, errDeclined } if err == nil && buildrun.IsLost(step) { // The build seat gave the ask up (issue 734): it can no longer be answered, so it is asked again now, with the diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index f4c2357..c7b812b 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -67,6 +67,9 @@ type Builder struct { // Askings is how many times one build module's part of a build is asked, the first included, before its key is failed // with why; DefaultAskings when 0. Askings int + // DeclineBound and DeclinePause bound and pace the asks build nodes off the agreed toolchain declined (ADR 0584 clause 2): + // they are no askings. DefaultDeclineBound and DefaultDeclinePause when 0; a negative DeclinePause puts it again at once. + DeclineBound, DeclinePause time.Duration // Said is told each ask asked again (the builder's event build.asked-again). Nil tells nobody. Said func(AskedAgain) // Log is told each build's start and end, the entries of the builder's log (novox/hq ADR 0571 ยง1). Nil tells nobody. diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index 2eeb07d..6950dc3 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -738,6 +738,25 @@ func TestABuildAskWaitingInItsStreamIsNotBounded(t *testing.T) { t.Fatalf("the key = %+v after %d asks, want done from the one ask", v, len(r.asks.Asks())) } }) + t.Run("declined by a node off the agreed toolchain: put again, no asking used, the bound ends it loudly", func(t *testing.T) { // ADR 0584 clause 2 + r := newRig(t, goBundle) + r.asks.Answer = nil + r.b.DeclinePause, r.b.DeclineBound = -1, 150*time.Millisecond + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + declined, _ := json.Marshal(delivery.StepDeclined{Declined: true, Lacks: []string{"the agreed toolchain go1.27.1 (recipe 1)"}}) + waitForAsks(t, r, 1) + for ended, deadline := 0, time.Now().Add(5*time.Second); valueOf(t, r.st, r.key).State == delivery.BuildBuilding && time.Now().Before(deadline); time.Sleep(5 * time.Millisecond) { + if asks := r.asks.Asks(); len(asks) > ended { // each ask put again is declined in its turn + r.asks.End(delivery.Step{ID: asks[ended].ID, Node: "ace", State: delivery.StepRefused, Reason: "this node builds with go1.26.0", Result: declined}) + ended++ + } + } + r.b.Wait() + v := valueOf(t, r.st, r.key) + if n := len(r.asks.Asks()); v.State != delivery.BuildFailed || n <= DefaultAskings || !strings.Contains(v.Error, "declined for") || !strings.Contains(v.Error, "ace: this node builds with go1.26.0") { + t.Fatalf("the key = %+v after %d asks, want it failed after more than %d asks, naming ace and its reason", v, n, DefaultAskings) + } + }) for _, c := range []struct { name string askings int