diff --git a/modules/mesh-delivery-builder/buildrun/asks.go b/modules/mesh-delivery-builder/buildrun/asks.go index d9a0c35b..bbc793fb 100644 --- a/modules/mesh-delivery-builder/buildrun/asks.go +++ b/modules/mesh-delivery-builder/buildrun/asks.go @@ -102,7 +102,8 @@ func (k Asker) dropped(a buildstep.StepAsk, why string) { // PutTry is how long one publish of an ask waits for its stream's answer before it is made again (Put). var PutTry = 10 * time.Second -// Put puts a on its stream: the seat's accept subject of its language and kind. The put is a kept call (novox/hq ADR 0668 +// Put puts a on its stream: the seat's accept subject of its language and kind, under the builder's name as asker (design 47 G +// §481, §600: `accept..builder`). The put is a kept call (novox/hq ADR 0668 // §1, §2; issue 1966, issue 1748 note 27): it carries the ask's id as its message id, so the stream takes it once, and it // is made again through every transport fault (a deadline, no responders) within busretry.Bound, each try bounded by // PutTry, until ctx ends. A put that still meets one is an error opening with buildstep.TransportFaultWords, so the key it @@ -112,7 +113,7 @@ func (k Asker) Put(ctx context.Context, a buildstep.StepAsk) error { if err != nil { return err } - subject, err := buildstep.AskSubject(a.Language, a.Kind) + subject, err := buildstep.AskSubjectBy(a.Language, a.Kind, buildstep.AskerBuilder) if err != nil { return err } diff --git a/modules/mesh-delivery-builder/buildrun/asks_test.go b/modules/mesh-delivery-builder/buildrun/asks_test.go index a460f924..ed127a56 100644 --- a/modules/mesh-delivery-builder/buildrun/asks_test.go +++ b/modules/mesh-delivery-builder/buildrun/asks_test.go @@ -30,7 +30,8 @@ func TestAnAskerPutsOnceAndReadsTheStep(t *testing.T) { f := &FakeAsks{Answer: func(a buildstep.StepAsk) buildstep.Step { return buildstep.Step{ID: a.ID, Node: "ace", State: buildstep.StepPassed, Result: []byte(`{"artifacts":[]}`)} }} - k := Asker{Publisher: f, Waiter: f} + subjects := &subjectsOf{inner: f} + k := Asker{Publisher: subjects, Waiter: f} var seen, also []buildstep.Step // issue 1890: a waiter that asks to see each value of the step sees it // Issue 1921: a second seen on the same context (a check's begin bound inside a try) is added, never in place of the first. both := WithStepSeen(WithStepSeen(context.Background(), func(s buildstep.Step) { seen = append(seen, s) }), @@ -39,6 +40,10 @@ func TestAnAskerPutsOnceAndReadsTheStep(t *testing.T) { if err != nil || step.Node != "ace" || len(f.Asks()) != 1 || len(seen) != 1 || seen[0].Node != "ace" || len(also) != 1 { t.Fatalf("step %+v, %v, asks %d, seen %+v and %+v", step, err, len(f.Asks()), seen, also) } + // Design 47 G §481, §600: the ask is put under the builder's name, accept..builder. + if len(subjects.seen) != 1 || subjects.seen[0] != "mesh.seat.node-build-go.accept."+goAsk("x").Kind+".builder" { + t.Fatalf("the ask was put on %v", subjects.seen) + } f.Answer = func(a buildstep.StepAsk) buildstep.Step { return buildstep.Step{ID: a.ID, Node: "shanks", State: buildstep.StepRefused, Reason: "go 1.27 installed"} } @@ -381,3 +386,14 @@ func (p *faultyPuts) PublishAsk(ctx context.Context, subject, id string, data [] } return p.inner.PublishAsk(ctx, subject, id, data) } + +// subjectsOf records the subject each ask is published on. +type subjectsOf struct { + inner AskPublisher + seen []string +} + +func (p *subjectsOf) PublishAsk(ctx context.Context, subject, id string, data []byte) error { + p.seen = append(p.seen, subject) + return p.inner.PublishAsk(ctx, subject, id, data) +} diff --git a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/held.go b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/held.go index acdbd9fe..f8cb1e46 100644 --- a/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/held.go +++ b/modules/mesh-delivery-builder/cmd/mesh-delivery-builder/held.go @@ -115,7 +115,7 @@ func (h heldOnTheBus) seen(ctx context.Context, a buildstep.StepAsk, later bool) // stream is the build seat's stream of language, its name and the subject of kind on it. func (h heldOnTheBus) stream(ctx context.Context, language, kind string) (jetstream.Stream, string, string, error) { - subject, err := buildstep.AskSubject(language, kind) + subject, err := buildstep.AskSubjectBy(language, kind, buildstep.AskerBuilder) if err != nil { return nil, "", "", err } @@ -286,7 +286,7 @@ func (h heldOnTheBus) Queue(ctx context.Context, language string, limit int) (bu return q, err } for _, kind := range buildstep.AskKinds { - subject, _ := buildstep.AskSubject(language, kind) + subject, _ := buildstep.AskSubjectBy(language, kind, buildstep.AskerBuilder) var ahead []build.Queued if err := onSubject(ctx, stream, subject, func(m build.Queued) bool { ahead = append(ahead, m) diff --git a/modules/mesh-delivery-builder/internal/build/run_test.go b/modules/mesh-delivery-builder/internal/build/run_test.go index 1c471d37..ce3d66f3 100644 --- a/modules/mesh-delivery-builder/internal/build/run_test.go +++ b/modules/mesh-delivery-builder/internal/build/run_test.go @@ -1328,6 +1328,27 @@ func TestABuildAskWaitingInItsStreamIsNotBounded(t *testing.T) { if maxFlight.Load() < 2 { t.Errorf("the values were read one at a time") } + // Design 47 G §484: a step key neither form parses is written in the journal naming it. + var said []string + steps := StateRead{Keys: func() ([]string, error) { + return []string{"ace.mvl0frv18d1957", "builder.ace.mvl0frv18d1958", "a.b.c.d"}, nil + }} + if _, err := (StuckReader{Builds: slow, ReAsks: state(map[string]string{}), Steps: map[string]StateRead{"mesh-build-go": steps}, Now: func() time.Time { return at }, + Logf: func(f string, a ...any) { said = append(said, fmt.Sprintf(f, a...)) }}).Stuck("mod10"); err != nil { + t.Fatal(err) + } + n := 0 + for _, l := range said { + if strings.Contains(l, `"a.b.c.d"`) { + n++ + } + if strings.Contains(l, "ace.mvl0frv18d1957") || strings.Contains(l, "builder.ace.mvl0frv18d1958") { + t.Errorf("a key of one of the two forms is said not to parse: %s", l) + } + } + if n != 1 { + t.Errorf("the key of neither form is named %d times in %q", n, said) + } }) } diff --git a/modules/mesh-delivery-builder/internal/build/stuck.go b/modules/mesh-delivery-builder/internal/build/stuck.go index 04f3fd1a..560bce85 100644 --- a/modules/mesh-delivery-builder/internal/build/stuck.go +++ b/modules/mesh-delivery-builder/internal/build/stuck.go @@ -325,9 +325,15 @@ func (r StuckReader) Stuck(module string) (map[string]any, error) { } byID := make(map[string]string, len(ks)) for _, k := range ks { - if _, _, id, err := buildstep.ParseStepKey(k); err == nil { - byID[id] = k + _, _, id, err := buildstep.ParseStepKey(k) + if err != nil { + // A key neither form parses is named, never passed over in silence (design 47 G §484). + if r.Logf != nil { + r.Logf("builder_stuck: the key %q of the steps of %s is a step key of neither form, .. or .: %v", k, bm, err) + } + continue } + byID[id] = k } stepKeys[bm] = byID } diff --git a/sdk/buildstep/ask.go b/sdk/buildstep/ask.go index 55a63241..6281a77d 100644 --- a/sdk/buildstep/ask.go +++ b/sdk/buildstep/ask.go @@ -137,7 +137,8 @@ func BuildModuleOf(language string) (string, error) { return "mesh-build-" + language, nil } -// AskSubject is the subject a build seat accepts a kind of ask on: the seat's accept subject. +// AskSubject is the subject a build seat accepts a kind of ask on: the seat's accept subject of main's form, which names no +// asker (design 47 G §600, §483). func AskSubject(language, kind string) (string, error) { seat, err := BuildSeat(language) if err != nil { @@ -149,20 +150,44 @@ func AskSubject(language, kind string) (string, error) { return "mesh.seat." + seat + ".accept." + kind, nil } -// ParseAskSubject is the inverse of AskSubject. +// AskerBuilder is the builder's name as an asker: the last token of the subject its asks are put on and the first of its +// step keys (design 47 G §600, §481). +const AskerBuilder = "builder" + +// AskSubjectBy is the subject a build seat accepts a queue's ask of asker on: `mesh.seat..accept..`, the +// asker's name the last token (design 47 G §481, §600). An empty asker is main's subject (AskSubject). +func AskSubjectBy(language, kind, asker string) (string, error) { + subject, err := AskSubject(language, kind) + if err != nil || asker == "" { + return subject, err + } + if !askerOK.MatchString(asker) { + return "", fmt.Errorf("buildstep: %q is not an asker's name", asker) + } + return subject + "." + asker, nil +} + +// ParseAskSubject is the inverse of AskSubject and of AskSubjectBy: the language and the queue of either form. func ParseAskSubject(subject string) (language, kind string, err error) { + language, kind, _, err = ParseAskSubjectBy(subject) + return language, kind, err +} + +// ParseAskSubjectBy is the inverse of AskSubjectBy: the language, the queue and the asker, which is empty for main's form. +func ParseAskSubjectBy(subject string) (language, kind, asker string, err error) { rest, ok := strings.CutPrefix(subject, "mesh.seat.node-build-") if !ok { - return "", "", fmt.Errorf("buildstep: %q is not a build seat's accept subject", subject) + return "", "", "", fmt.Errorf("buildstep: %q is not a build seat's accept subject", subject) } language, kind, ok = strings.Cut(rest, ".accept.") if !ok { - return "", "", fmt.Errorf("buildstep: %q is not a build seat's accept subject", subject) + return "", "", "", fmt.Errorf("buildstep: %q is not a build seat's accept subject", subject) } - if want, err := AskSubject(language, kind); err != nil || want != subject { - return "", "", fmt.Errorf("buildstep: %q is not a build seat's accept subject: %v", subject, err) + kind, asker, _ = strings.Cut(kind, ".") + if want, err := AskSubjectBy(language, kind, asker); err != nil || want != subject { + return "", "", "", fmt.Errorf("buildstep: %q is not a build seat's accept subject: %v", subject, err) } - return language, kind, nil + return language, kind, asker, nil } // AskStream is the work-queue stream of a language's build seat. diff --git a/sdk/buildstep/ask_test.go b/sdk/buildstep/ask_test.go index b394f66f..88558bee 100644 --- a/sdk/buildstep/ask_test.go +++ b/sdk/buildstep/ask_test.go @@ -594,3 +594,29 @@ func TestFetchBuildsResolvedAndToolchainDigestAreRead(t *testing.T) { t.Errorf("toolchain digest: %+v", tc) } } + +// §481, §600: a queue's ask of an asker is put on accept.., and both forms parse. +func TestAskSubjectBy(t *testing.T) { + got, err := AskSubjectBy(AskGo, AskDelivery, AskerBuilder) + if err != nil || got != "mesh.seat.node-build-go.accept.delivery.builder" { + t.Fatalf("%q %v", got, err) + } + for _, c := range []struct{ subject, language, kind, asker string }{ + {got, "go", "delivery", "builder"}, + {"mesh.seat.node-build-node.accept.check", "node", "check", ""}, + } { + l, k, a, err := ParseAskSubjectBy(c.subject) + if err != nil || l != c.language || k != c.kind || a != c.asker { + t.Errorf("%s: %q %q %q %v", c.subject, l, k, a, err) + } + if l, k, err := ParseAskSubject(c.subject); err != nil || l != c.language || k != c.kind { + t.Errorf("%s: ParseAskSubject %q %q %v", c.subject, l, k, err) + } + } + if _, err := AskSubjectBy(AskGo, AskDelivery, "Bad.Name"); err == nil { + t.Error("a bad asker has a subject") + } + if _, _, _, err := ParseAskSubjectBy("mesh.seat.node-build-go.accept.delivery.builder.extra"); err == nil { + t.Error("a subject of four tokens after accept parses") + } +}