From 2e567b9b30d433139db43e87b5c4333956c94933 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 18:01:33 +0200 Subject: [PATCH] =?UTF-8?q?Let=20a=20key=20whose=20lock=20names=20an=20unp?= =?UTF-8?q?ublished=20package=20version=20wait=20queued,=20naming=20it,=20?= =?UTF-8?q?and=20build=20it=20once=20the=20builder's=20own=20done=20key=20?= =?UTF-8?q?publishes=20it=20(issue=201587,=20B7,=20ADR=200500=20=C2=A76)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prefix 'waits for: ' is kept in the builder until mesh-sdk's delivery.WaitsFor lands on feat/r4e-sdk. --- .../internal/build/claim.go | 11 +++ .../internal/build/r4e_test.go | 55 ++++++++++++ .../internal/build/run.go | 86 ++++++++++++++++--- 3 files changed, 142 insertions(+), 10 deletions(-) diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go index 576a450..6a6659f 100644 --- a/modules/mesh-delivery-builder/internal/build/claim.go +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -72,6 +72,10 @@ type ClaimOptions struct { // built as a candidate's, which never pushes or publishes (ADR 0487 clause 12, ADR 0498 clauses 5 and 6); on the trunk it // is taken again to publish them, an image only when an image builder runs (ImageBuilder). Trunk bool + // Published says whether a package version `@` is published: one of the builder's done keys names it as + // a package's reference. A key queued waiting for a version (BuilderBuild.Waits) is taken only once it is (ADR 0500 + // clause 6). Nil means none is. + Published func(ref string) bool } // Unpublished is whether a done build holds an image built without a push or a package built without a publish: it has @@ -139,6 +143,8 @@ func Unreferenced(b delivery.BuilderBuild) bool { // clause 2, ADR 0498 clauses 4 and 5) or for digests that differ (buildrun.DigestsDifferPrefix, clause 4), // which wait for another fingerprint, and one failed for want of an image builder (NoImageBuilderPrefix), which waits for // another fingerprint or a node that runs the image build module (ClaimOptions.ImageBuilder); +// - a key queued waiting for a package version (Waits) is claimed building as a new revision once that version is +// published (ClaimOptions.Published, ADR 0500 clause 6), and not before; // - a key left queued or building by this builder is taken as it stands (ADR 0478 clause 5: a stopped builder takes its // own again when it starts), and so is one left by another builder when ClaimOptions.TakeOver (clause 6). func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string, opt ClaimOptions) (*Claim, error) { @@ -176,6 +182,11 @@ func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string return nil, nil case cur.State == delivery.BuildFailed: return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) }) + case cur.State == delivery.BuildQueued && cur.Waits != "": + if opt.Published == nil || !opt.Published(cur.Waits) { + return nil, nil // the package it waits for is not published yet + } + return c.write(ctx, key, building, func(data []byte) (uint64, error) { return c.Store.Update(ctx, key, data, rev) }) case cur.By == c.By || opt.TakeOver: return &Claim{Key: key, Rev: rev, Build: cur}, nil } diff --git a/modules/mesh-delivery-builder/internal/build/r4e_test.go b/modules/mesh-delivery-builder/internal/build/r4e_test.go index 3205bb7..b0246c5 100644 --- a/modules/mesh-delivery-builder/internal/build/r4e_test.go +++ b/modules/mesh-delivery-builder/internal/build/r4e_test.go @@ -244,3 +244,58 @@ func TestAPublishThatFailsVersionTakenOnTheTrunkKeepsThePrefixFirst(t *testing.T t.Errorf("build = %+v, want failed opening %q", v, delivery.VersionTaken) } } + +// sdkModule adds to the rig a module "sdk" whose manifest (digest b) holds the TypeScript package lib, published as ref. +func sdkModule(t *testing.T, r *rig, ref string) []byte { + t.Helper() + r.mf.byDigest[mdigest('b')] = catalogue.Manifest{Module: "sdk", Build: &catalogue.Build{Artifacts: []catalogue.Artifact{tsPackage}}} + r.bm.packages["lib"] = ref + data, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-sdk", Commit: "8a74cee83f88e6585a5511c3cc53bdf0f5303d20", FP2: "src2:" + hex64('5'), + Manifest: mdigest('b'), Path: ".", Toolchains: agreedToolchains}) + if err != nil { + t.Fatal(err) + } + return data +} + +func TestAModuleWhoseLockNamesAnUnpublishedPackageWaitsQueuedAndIsBuiltOncePublished(t *testing.T) { // B7, ADR 0500 clause 6 + const ref = "@novox/mesh-sdk@0.1.15" + r := newRig(t, tsBundle) + for _, n := range []string{"ace", "novox"} { + r.bm.fails[n+"/build_node_fetch"] = waitsFor + ref + ": the lock names a version the registry does not hold" + } + v := r.consider(t, "modules/m") + if v.State != delivery.BuildQueued || v.Waits != ref || v.Error != "" { + t.Fatalf("build = %+v, want queued, waiting for %s, not failed", v, ref) + } + delete(r.bm.fails, "ace/build_node_fetch") + delete(r.bm.fails, "novox/build_node_fetch") + writes := r.st.writes + r.b.tried = map[string]bool{} + r.consider(t, "modules/m") + if r.st.writes != writes { + t.Fatalf("a key waiting for an unpublished package was claimed (%d writes, was %d)", r.st.writes, writes) + } + r.b.Module(context.Background(), "sdk", sdkModule(t, r, ref)) // the package's trunk key is built and published + r.b.Wait() + if v = valueOf(t, r.st, r.key); v.State != delivery.BuildDone || v.Waits != "" { + t.Errorf("build = %+v, want built once the package it waits for is published", v) + } +} + +func TestAKeyWaitingForAPackageAlreadyPublishedIsBuiltWhenTheBuilderStarts(t *testing.T) { // B7: the scan at start + const ref = "@novox/mesh-sdk@0.1.15" + r := newRig(t, tsBundle) + putBuild(t, r.st, r.key, delivery.BuilderBuild{State: delivery.BuildQueued, By: "novox.builder", Manifest: manifestD, Waits: ref, Step: "waits"}) + sdkKey, _ := delivery.BuilderBuildKey("sdk", "src2:"+hex64('5')) + putBuild(t, r.st, sdkKey, delivery.BuilderBuild{State: delivery.BuildDone, By: "novox.builder", Manifest: mdigest('b'), Step: "built", + Artifacts: []delivery.BuiltArtifact{{Name: "lib", Kind: "package", Digest: digest, Reference: ref}}}) + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + if err := r.b.Sweep(context.Background()); err != nil { + t.Fatal(err) + } + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone { + t.Errorf("build = %+v, want built: the package it waits for was published before the builder started", v) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 6754c17..c3acd58 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -63,6 +63,9 @@ type Builder struct { // whose second-node build failed is held back until it moves (issue 1664). epoch uint64 secondAt map[string]uint64 // by build key: the epoch a second-node build of a one-node key failed at + // published are the package versions `@` a done key of this bucket names as a package's reference: a key + // waiting for one is built once it is here (ADR 0500 clause 6). + published map[string]bool } type planned struct { @@ -79,7 +82,7 @@ func (b *Builder) init() { b.sem = make(chan struct{}, limit) b.cands, b.via, b.trunkFP = map[string]delivery.Candidate{}, map[string]via{}, map[string]bool{} b.known, b.inflight, b.again, b.plans, b.tried = map[string]delivery.Module{}, map[string]bool{}, map[string]delivery.Module{}, map[string]planned{}, map[string]bool{} - b.secondAt = map[string]uint64{} + b.secondAt, b.published = map[string]uint64{}, map[string]bool{} if b.Places == nil { b.Places = &buildrun.Places{} } @@ -126,10 +129,11 @@ func (b *Builder) NodesChanged(ctx context.Context) { b.Reconsider(ctx) } -// Sweep closes, at start, every key left building for a fingerprint its module no longer has (issue 1660, ADR 0478 -// clause 6): the key can never be built, since nothing asks for it, and would stay building for ever. It fails the key -// naming who left it. A key of the fingerprint the module has now is not touched: the build takes it over. Call it -// once the modules bucket has been read, so every module is known. +// Sweep reads, at start, every key of the bucket: it keeps the package versions done keys name as published (a key +// waiting for one is then looked at again, ADR 0500 clause 6), and closes every key left building for a fingerprint its +// module no longer has (issue 1660, ADR 0478 clause 6): the key can never be built, since nothing asks for it, and would +// stay building for ever. It fails the key naming who left it. A key of the fingerprint the module has now is not +// touched: the build takes it over. Call it once the modules bucket has been read, so every module is known. func (b *Builder) Sweep(ctx context.Context) error { b.init() keys, err := b.Claimer.Store.Keys(ctx) @@ -146,16 +150,18 @@ func (b *Builder) Sweep(ctx context.Context) error { } } b.mu.Unlock() + newly := false for _, key := range keys { - if current[key] { - continue - } value, rev, found, err := b.Claimer.Store.Get(ctx, key) if err != nil || !found { continue } cur, err := delivery.DecodeBuilderBuild(value) - if err != nil || cur.State != delivery.BuildBuilding { + if err != nil { + continue + } + newly = b.notePublished(cur) || newly + if current[key] || cur.State != delivery.BuildBuilding { continue } closed := delivery.BuilderBuild{State: delivery.BuildFailed, By: b.Claimer.By, Manifest: cur.Manifest, Step: cur.Step, @@ -169,9 +175,36 @@ func (b *Builder) Sweep(ctx context.Context) error { } b.logf("%s: closed, left building by %s for a fingerprint the module no longer has", key, cur.By) } + if newly { + // A key read before the scan may wait for a package the scan found published (ADR 0500 clause 6). + b.Reconsider(ctx) + } return nil } +// notePublished keeps the package versions a done build names as references, and says whether one was new. +func (b *Builder) notePublished(v delivery.BuilderBuild) bool { + if v.State != delivery.BuildDone { + return false + } + b.mu.Lock() + defer b.mu.Unlock() + newly := false + for _, a := range v.Artifacts { + if a.Kind == catalogue.ArtifactPackage && a.Reference != "" && !b.published[a.Reference] { + b.published[a.Reference] = true + newly = true + } + } + return newly +} + +func (b *Builder) isPublished(ref string) bool { + b.mu.Lock() + defer b.mu.Unlock() + return b.published[ref] +} + // Forget takes a module out (its key was deleted). func (b *Builder) Forget(key string) { b.init() @@ -298,7 +331,7 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { // A second build that failed is not tried again until the nodes change (issue 1664). return (!failedBefore || epoch > failedAt) && b.moreNodes(modules) } - cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, MoreNodes: moreNodes, ImageBuilder: b.imageBuilderRuns, Trunk: trunk}) + cl, err := b.Claimer.Claim(ctx, module, m.FP2, m.Manifest, ClaimOptions{TakeOver: first, MoreNodes: moreNodes, ImageBuilder: b.imageBuilderRuns, Trunk: trunk, Published: b.isPublished}) if err != nil { b.logf("%s: %v", module, err) return @@ -352,6 +385,13 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { if result.State == delivery.BuildFailed { b.logf("%s: failed: %s", cl.Key, result.Error) } + if result.State == delivery.BuildQueued { + b.logf("%s: waits for the package %s to be published", cl.Key, result.Waits) + } + if b.notePublished(result) { + // A key may wait for the package this build published (ADR 0500 clause 6). + b.Reconsider(ctx) + } } // stale is why the module's fingerprint is not built now, or empty when it is: the toolchain each language of the plan @@ -485,6 +525,11 @@ func (b *Builder) build(ctx context.Context, module string, m delivery.Module, p } } if len(errs) > 0 { + if ref := waitsOf(errs); ref != "" { + // The module's lock names a version of a package the registry does not hold yet: the key waits for it, queued, and + // is not failed (ADR 0500 clause 6). + return delivery.BuilderBuild{State: delivery.BuildQueued, Waits: ref, Step: "waits for the package " + ref + " to be published"} + } return failed("%s", keyError(errs)) } var artifacts []delivery.BuiltArtifact @@ -562,6 +607,27 @@ func stored(kind string) bool { return kind == catalogue.ArtifactBundle || kind == catalogue.ArtifactArchive || kind == catalogue.ArtifactUpstream } +// waitsFor opens the reason of a fetch step whose module's lock names a package version the registry does not hold, then +// `@` (the Node build module's, ADR 0500 clause 6). +const waitsFor = "waits for: " + +// waitsOf is the package version a lane's error says the module waits for, or "" when none does or what it names is not +// `@`. +func waitsOf(errs []string) string { + for _, e := range errs { + at := strings.Index(e, waitsFor) + if at < 0 { + continue + } + ref, _, _ := strings.Cut(e[at+len(waitsFor):], " ") + ref = strings.TrimRight(ref, ":;,") + if _, err := delivery.EncodeBuilderBuild(delivery.BuilderBuild{State: delivery.BuildQueued, Waits: ref}); err == nil { + return ref + } + } + return "" +} + // keyPrefixes are the errors a build module's step may open its reason with that the key's error must open with too, so // the claim reads them (delivery.FailedUntilChanged), first the ones not built again. var keyPrefixes = []string{delivery.NotPinned, delivery.VersionTaken, delivery.RefusedKind, delivery.NotStored}