F1, F7: the asker drops an ask only on a withdrawal cause (superseded, taken out, asked again by hand), not on the check bound; the checker's drop on Forget is asserted; a step's own run count and a failed drop are said (ADR 0664 §4, issue 1748 review 1)

Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ
This commit is contained in:
2026-11-09 23:37:51 +01:00
parent 82f0f6a134
commit 68fedded8c
8 changed files with 168 additions and 9 deletions
+26 -6
View File
@@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"log"
"strings"
"sync"
"time"
@@ -42,8 +43,18 @@ type Asker struct {
// again that the bus did not give within its bound, or a wait ended with a cause its caller names (a superseded
// fingerprint). Never a wait ended by a shutdown (a plain cancel): the ask is then taken over by the next run.
Dropper AskDropper
// Logf says a drop that failed (the log's Printf when unset): the ask then stays on its stream, and it is said.
Logf func(format string, args ...any)
}
// Withdrawn is the cause a caller gives the context of an ask's wait to withdraw the ask (ADR 0664 §4): a build superseded by a
// newer fingerprint, a candidate taken out, an ask asked again by hand. Asker.Wait drops the ask on this cause alone: a
// plain cancel (a shutdown) and any other cause (a bound passing, a check of whether the ask is gone) leave it on its stream
// for the next run. A cause that wraps a Withdrawn (errors.As) withdraws too.
type Withdrawn struct{ Why string }
func (w Withdrawn) Error() string { return w.Why }
// AskDropper ends an ask: the seat's verb drop, served by any holder of the build seat (no node is named). It writes the
// ask in the build module's shared state of pauses, so every node refuses it when the stream gives it, and the node that
// runs it ends the step (ADR 0664 §5).
@@ -71,14 +82,20 @@ func (d SeatDrops) DropAsk(_ context.Context, a buildstep.StepAsk, why string) e
return nil
}
// dropped ends a without its end. A failed drop is not the wait's error: the wait says why it ended.
// dropped ends a without its end. A failed drop is not the wait's error, but it is said (Logf).
func (k Asker) dropped(a buildstep.StepAsk, why string) {
if k.Dropper == nil {
return
}
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
_ = k.Dropper.DropAsk(ctx, a, why)
if err := k.Dropper.DropAsk(ctx, a, why); err != nil {
say := k.Logf
if say == nil {
say = log.Printf
}
say("the ask %s was not dropped (%s), it stays on its stream: %v", a.ID, why, err)
}
}
// Put puts a on its stream: the seat's accept subject of its language and kind.
@@ -133,8 +150,9 @@ func (k Asker) Wait(ctx context.Context, a buildstep.StepAsk) (buildstep.Step, e
for {
select {
case <-ctx.Done():
if cause := context.Cause(ctx); cause != nil && !errors.Is(cause, context.Canceled) {
k.dropped(a, cause.Error())
var w Withdrawn
if errors.As(context.Cause(ctx), &w) {
k.dropped(a, context.Cause(ctx).Error())
}
return buildstep.Step{}, ctx.Err()
case <-bound:
@@ -167,8 +185,10 @@ func (k Asker) Wait(ctx context.Context, a buildstep.StepAsk) (buildstep.Step, e
continue
}
if buildstep.Ended(step) {
if step.Passing && step.State == buildstep.StepFailed && len(agains) > 0 {
step.Reason = fmt.Sprintf("a transport fault on %d runs (%s): %s", len(agains)+1, strings.Join(append(agains, step.Node), ", "), step.Reason)
if step.Passing && step.State == buildstep.StepFailed && (len(agains) > 0 || step.Run > 1) {
// The step's own count of runs (ADR 0664 §2), not only the ones this wait saw: a builder that started again
// reads the key's last value and none of the runs before it.
step.Reason = fmt.Sprintf("a transport fault on %d runs (%s): %s", max(step.Run, len(agains)+1), strings.Join(append(agains, step.Node), ", "), step.Reason)
}
return step, nil
}
@@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"strings"
"sync"
"testing"
@@ -79,15 +80,33 @@ func TestAnAskerPutsOnceAndReadsTheStep(t *testing.T) {
(Asker{Publisher: h, Waiter: h, Bound: 20 * time.Millisecond, Dropper: drops}).Wait(context.Background(), goAsk("mvl0frv18d1974"))
named, stop := context.WithCancelCause(context.Background())
h.Write(buildstep.Step{ID: "mvl0frv18d1975", Node: "g14", State: buildstep.StepRunning, Started: time.Now()})
seenStep := WithStepSeen(named, func(buildstep.Step) { stop(errors.New("superseded by a newer fingerprint")) })
seenStep := WithStepSeen(named, func(buildstep.Step) { stop(Withdrawn{Why: "superseded by a newer fingerprint"}) })
(Asker{Publisher: h, Waiter: h, Bound: time.Hour, Dropper: drops}).Wait(seenStep, goAsk("mvl0frv18d1975"))
shut, off := context.WithCancel(context.Background())
off()
(Asker{Publisher: h, Waiter: h, Bound: time.Hour, Dropper: drops}).Wait(shut, goAsk("mvl0frv18d1977"))
(Asker{Publisher: &FakeAsks{}, Waiter: &FakeAsks{}, Bound: time.Hour, Dropper: drops}).Wait(named, goAsk("mvl0frv18d1976"))
// Review 1 (finding 1): a cause that is no withdrawal (the builder's check bound passing, which waits on) drops nothing.
bounded, passed := context.WithCancelCause(context.Background())
passed(errors.New("the check bound passed"))
(Asker{Publisher: &FakeAsks{}, Waiter: &FakeAsks{}, Bound: time.Hour, Dropper: drops}).Wait(bounded, goAsk("mvl0frv18d1978"))
if got := strings.Join(drops.got, "|"); !strings.Contains(got, "mvl0frv18d1974") || !strings.Contains(got, "mvl0frv18d1975 superseded by a newer fingerprint") || !strings.Contains(got, "mvl0frv18d1976 superseded by a newer fingerprint") || len(drops.got) != 3 {
t.Errorf("drops %q, want the ask never given again, the superseded one and the one no node took, and no other", got)
}
// Review 1: a wait that starts after the first runs (a builder started again) reads the last value only, and its end still says
// how many runs; a drop that failed is said, not discarded.
late := &FakeAsks{}
late.Write(buildstep.Step{ID: "mvl0frv18d1979", Node: "shanks", State: buildstep.StepFailed, Passing: true, Run: 3, Reason: "lookup proxy: i/o timeout"})
if st, err := (Asker{Publisher: late, Waiter: late, Bound: time.Minute}).Wait(context.Background(), goAsk("mvl0frv18d1979")); err != nil || !strings.Contains(st.Reason, "a transport fault on 3 runs") {
t.Errorf("a failed step of run 3 read alone says %q (%v), want it names 3 runs", st.Reason, err)
}
var said []string
bad, off2 := context.WithCancelCause(context.Background())
off2(Withdrawn{Why: "withdrawn"})
(Asker{Publisher: &FakeAsks{}, Waiter: &FakeAsks{}, Bound: time.Hour, Dropper: failingDrops{}, Logf: func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) }}).Wait(bad, goAsk("mvl0frv18d1980"))
if len(said) != 1 || !strings.Contains(said[0], "mvl0frv18d1980") || !strings.Contains(said[0], "no holder") {
t.Errorf("a drop that failed said %q, want it named with its cause", said)
}
// review round 2 item 1: an ask that waits in its stream for a free slot is not bounded; once a node writes its step
// running, the step is bounded from its start, and the error names the node.
g := &FakeAsks{}
@@ -294,3 +313,10 @@ func (d *seatDrops) DropAsk(_ context.Context, a buildstep.StepAsk, why string)
d.got = append(d.got, strings.TrimSpace(a.ID+" "+why))
return nil
}
// failingDrops is an AskDropper whose verb is not answered.
type failingDrops struct{}
func (failingDrops) DropAsk(context.Context, buildstep.StepAsk, string) error {
return errors.New("no holder answered")
}
@@ -301,6 +301,9 @@ type byHand struct{ why string }
func (h byHand) Error() string { return "asked again by hand: " + h.why }
// Unwrap makes the ask asked again by hand a withdrawal of the old ask: its asker drops it (ADR 0664 §4).
func (h byHand) Unwrap() error { return buildrun.Withdrawn{Why: h.Error()} }
// answer waits for the step of the lane's ask, and asks it again while it can no longer be answered (Gone) or a person
// asked it again (AskAgain); it answers the step and the ask that has it.
func (b *Builder) answer(ctx context.Context, l *lane, ask sdkstep.StepAsk, asked *askNotes, module, fp string) (sdkstep.Step, sdkstep.StepAsk, error) {
@@ -121,6 +121,9 @@ type Builder struct {
waiting map[string]map[string]chan string
// hands are the requests to build a key again by hand that wait for the claim and its asks (BuildAgain), by build key.
hands map[string]*handWatch
// builds are the ends that withdraw the asks of a key's build in flight, by build key (ADR 0664 §4): a newer fingerprint of
// the module takes its older one's build out, and the asker drops that build's asks.
builds map[string]context.CancelCauseFunc
}
type planned struct {
@@ -139,6 +142,7 @@ func (b *Builder) init() {
b.known, b.inflight, b.again, b.plans, b.tried = map[string]meshcatalog.ModulesByModuleValue{}, map[string]bool{}, map[string]meshcatalog.ModulesByModuleValue{}, map[string]planned{}, map[string]bool{}
b.published, b.held, b.waiting = map[string]bool{}, map[string]bool{}, map[string]map[string]chan string{}
b.hands = map[string]*handWatch{}
b.builds = map[string]context.CancelCauseFunc{}
})
}
@@ -161,13 +165,25 @@ func (b *Builder) Module(ctx context.Context, key string, value []byte) {
return
}
b.mu.Lock()
old, had := b.known[key]
b.known[key] = m
var superseded context.CancelCauseFunc
if had && old.Fp2 != "" && old.Fp2 != m.Fp2 {
if ok, err := builds.BuilderBuildKey(key, old.Fp2); err == nil {
superseded = b.builds[ok]
}
}
if bk, err := builds.BuilderBuildKey(key, m.Fp2); err == nil && m.Fp2 != "" {
// A fingerprint the trunk carries is built from the trunk's commit, whoever asked for it first.
b.trunkFP[bk] = true
delete(b.via, bk)
}
b.mu.Unlock()
if superseded != nil {
// A newer fingerprint of the module: the build of the older one in flight is taken out, and its asks are withdrawn
// from the build seats (ADR 0664 §4).
superseded(buildrun.Withdrawn{Why: "the build of " + key + " at " + old.Fp2 + " was superseded by the fingerprint " + m.Fp2})
}
b.consider(ctx, key, m)
}
@@ -498,7 +514,20 @@ func (b *Builder) try(ctx context.Context, module string, m meshcatalog.ModulesB
// any other, so the controller has a done key to register it from (issue 1613, ADR 0478 clause 1, ADR 0501 clause 1).
result = builds.BuilderBuild{State: builds.BuildDone, Step: "nothing to build: the manifest names no artifact"}
} else {
result = b.build(ctx, module, m, plan, planErr, prior, only, b.notes(cl))
run, withdraw := context.WithCancelCause(ctx)
b.mu.Lock()
b.builds[key] = withdraw
b.mu.Unlock()
result = b.build(run, module, m, plan, planErr, prior, only, b.notes(cl))
b.mu.Lock()
delete(b.builds, key)
b.mu.Unlock()
withdraw(nil)
var w buildrun.Withdrawn
if ctx.Err() == nil && errors.As(context.Cause(run), &w) {
// Taken out, not stopped: the key says so, and the asks are withdrawn (the asker dropped them).
result = builds.BuilderBuild{State: builds.BuildFailed, Error: keyError([]string{"withdrawn: " + w.Why})}
}
}
if ctx.Err() != nil {
return
@@ -624,6 +624,7 @@ func TestTwoInstancesStartingTogetherEndWithOneDoneKey(t *testing.T) { // 1607/A
}
func TestAModulesValueThatArrivesWhileItsKeyIsBeingBuiltIsLookedAtAgainWhenItEnds(t *testing.T) { // 1607/AC6 (no event is dropped, no timer)
t.Run("a newer fingerprint withdraws the build of the older one", aNewerFingerprintWithdrawsTheBuildOfTheOlderOne)
r := newRig(t, goBundle)
r.bm.fails["ace/build_go_build"] = "go build of tool: boom"
r.bm.hold, r.bm.entered = make(chan struct{}), make(chan struct{}, 8)
@@ -640,6 +641,60 @@ func TestAModulesValueThatArrivesWhileItsKeyIsBeingBuiltIsLookedAtAgainWhenItEnd
}
}
// ADR 0664 §4 (issue 1748, review 1 finding 2): a newer fingerprint of a module takes the build of its older one out: the key
// says so, and the asker drops the ask the build waits on, carrying the cause. The first ask's step runs and never ends.
func aNewerFingerprintWithdrawsTheBuildOfTheOlderOne(t *testing.T) {
r := newRig(t, goBundle)
drops := &seatDrops{}
var asked int32
running := make(chan struct{})
r.asks.Answer = func(a buildstep.StepAsk) buildstep.Step {
if atomic.AddInt32(&asked, 1) == 1 {
defer close(running)
return buildstep.Step{ID: a.ID, Node: "ace", State: buildstep.StepRunning, Started: time.Now()}
}
return r.answer(a)
}
r.b.Asks = buildrun.Asker{Publisher: r.asks, Waiter: r.asks, Bound: 2 * time.Second, Dropper: drops}
r.b.Module(context.Background(), "m", moduleValue(t, "modules/m"))
<-running
fp2 := "src2:" + hex64('2')
data, err := meshcatalogclient.EncodeModule(meshcatalog.ModulesByModuleValue{Repository: "novox/mesh-catalog", Commit: "7a74cee83f88e6585a5511c3cc53bdf0f5303d20", Fp: "src1:" + hex64('f'), Fp2: fp2,
Manifest: mdigest('a'), Path: "modules/m", Toolchains: CatalogToolchains(agreedToolchains)})
if err != nil {
t.Fatal(err)
}
r.b.Module(context.Background(), "m", data)
r.b.Wait()
old := valueOf(t, r.st, r.key)
if old.State != builds.BuildFailed || !strings.Contains(old.Error, "superseded by the fingerprint "+fp2) {
t.Errorf("the older key = %+v, want failed, naming the newer fingerprint that superseded it", old)
}
first := r.asks.Asks()[0].ID
if got := drops.get(); len(got) != 1 || !strings.HasPrefix(got[0], first+" ") || !strings.Contains(got[0], "superseded by the fingerprint "+fp2) {
t.Errorf("dropped %q, want the first ask withdrawn with the cause", got)
}
}
// seatDrops keeps the asks an Asker withdrew: "<id> <why>".
type seatDrops struct {
mu sync.Mutex
got []string
}
func (d *seatDrops) DropAsk(_ context.Context, a buildstep.StepAsk, why string) error {
d.mu.Lock()
defer d.mu.Unlock()
d.got = append(d.got, a.ID+" "+why)
return nil
}
func (d *seatDrops) get() []string {
d.mu.Lock()
defer d.mu.Unlock()
return append([]string(nil), d.got...)
}
func TestAKeyIsBuiltOnlyWithTheToolchainItsFingerprintNamesAndNothingIsWrittenOtherwise(t *testing.T) { // 1615/AC4
r := newRig(t, goBundle)
old := map[string]builds.ToolchainUsed{"go": {Version: "go1.26.0", Recipe: "1"}}
@@ -10,6 +10,7 @@ import (
"git.novox.be/novox/mesh-delivery/contracts/go/clients/meshdeliverycheckerclient"
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog"
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker"
"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/buildrun"
"git.novox.be/novox/mesh-delivery/sdk/buildstep"
)
@@ -70,7 +71,7 @@ const AskAgainTool = "checker_ask_again"
const AskedAgainWords = EndedWords + "asked again by hand (" + AskAgainTool + "): "
// errAskedAgain is the cause a running check is ended with when its verdict was asked again by hand.
var errAskedAgain = errors.New("its verdict was asked again by hand (" + AskAgainTool + ")")
var errAskedAgain = buildrun.Withdrawn{Why: "its verdict was asked again by hand (" + AskAgainTool + ")"}
// IsAskedAgain says whether a verdict was written by AskAgainTool.
func IsAskedAgain(v meshdeliverychecker.VerdictsByGroupHeadsValue) bool {
@@ -18,6 +18,7 @@ import (
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog"
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdelivery"
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker"
"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/buildrun"
"git.novox.be/novox/mesh-delivery/sdk/buildstep"
"git.novox.be/novox/mesh-sdk/go/keys"
)
@@ -177,6 +178,9 @@ func (s superseded) Error() string {
return EndedWords + "superseded by " + s.by
}
// Unwrap makes a check ended so a withdrawal of its asks: the asker drops what it put on the build nodes (ADR 0664 §4, §5).
func (s superseded) Unwrap() error { return buildrun.Withdrawn{Why: s.Error()} }
func (ch *Checker) init() {
ch.once.Do(func() {
ch.known, ch.inflight, ch.again, ch.tried = map[string]meshcatalog.CandidatesByGroupValue{}, map[string]bool{}, map[string]bool{}, map[string]bool{}
@@ -16,6 +16,7 @@ import (
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog"
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdelivery"
"git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker"
"git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/buildrun"
"git.novox.be/novox/mesh-delivery/sdk/buildstep"
"git.novox.be/novox/mesh-sdk/go/keys"
)
@@ -371,14 +372,34 @@ func TestARepositoryWithNoTrunkCannotBeJudgedLoudlyUntilItsTrunkIsKnown(t *testi
ch.init()
run, end := context.WithCancelCause(context.Background())
ch.live["g.heads"] = liveCheck{group: "g", end: end}
// The check's asker is waiting on an ask of its run when the candidate goes (review 1, finding 7).
drops := &checkDrops{}
fake := &buildrun.FakeAsks{}
done := make(chan struct{})
go func() {
defer close(done)
(buildrun.Asker{Publisher: fake, Waiter: fake, Bound: time.Hour, Dropper: drops}).Wait(run, buildstep.StepAsk{ID: "mvl0frv18d2000", Language: buildstep.AskGo, Kind: buildstep.AskCheck})
}()
ch.Forget("g")
<-done
var s superseded
if cause := context.Cause(run); !errors.As(cause, &s) || s.by != "" || errors.Is(cause, context.Canceled) {
t.Errorf("the check was ended with %v, want superseded with no heads, not a plain cancel", cause)
}
if len(drops.got) != 1 || !strings.Contains(drops.got[0], "mvl0frv18d2000") || !strings.Contains(drops.got[0], "no candidate holds these heads") {
t.Errorf("the asker dropped %q, want its ask withdrawn with the check's cause", drops.got)
}
})
}
// checkDrops keeps the asks an asker dropped: "<id> <why>".
type checkDrops struct{ got []string }
func (d *checkDrops) DropAsk(_ context.Context, a buildstep.StepAsk, why string) error {
d.got = append(d.got, a.ID+" "+why)
return nil
}
func countWith(lines []string, words string) int {
n := 0
for _, l := range lines {