diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go index 8db30456..ed7c21af 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/named_test.go @@ -59,6 +59,19 @@ func TestTheTrunkIsTheDirectoriesTheForgeListsTrees(t *testing.T) { // 1900/AC3 if fp, err := ownBuildVia(own, "n1")(context.Background()); err != nil || fp != "src2:abc" || asked != "seat:node-engine.composition@n1" { t.Errorf("own build = %q %v asked %q", fp, err, asked) } + // Issue 1921: the start during a replacement. What runs still names the old build, the want (composed by the node itself) + // the successor, which this process is: it fixes the successor. A want that is not self, or is removed, leaves what runs. + for _, row := range []struct{ name, answer, want string }{ + {"replacing: runs old, want new and self", `{"runs":{"fp":"src2:old"},"want":{"fp":"src2:new","self":true}}`, "src2:new"}, + {"a want the node does not compose", `{"runs":{"fp":"src2:old"},"want":{"fp":"src2:new"}}`, "src2:old"}, + {"a removed want", `{"runs":{"fp":"src2:old"},"want":{"fp":"src2:new","self":true,"removed":true}}`, "src2:old"}, + {"no build runs yet, want self", `{"want":{"fp":"src2:new","self":true}}`, "src2:new"}, + } { + ask := func(string, any) (json.RawMessage, error) { return json.RawMessage(row.answer), nil } + if fp, err := ownBuildVia(ask, "n1")(context.Background()); err != nil || fp != row.want { + t.Errorf("%s: own build = %q %v, want %q", row.name, fp, err, row.want) + } + } none := func(string, any) (json.RawMessage, error) { return json.RawMessage(`{}`), nil } if _, err := ownBuildVia(none, "n1")(context.Background()); err == nil { t.Error("an engine that says nothing runs gave a build") diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/ownbuild.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/ownbuild.go index 79a31448..cf7bb311 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/ownbuild.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/ownbuild.go @@ -25,10 +25,21 @@ func ownBuildVia(call toolCaller, node string) func(ctx context.Context) (string Runs *struct { FP string `json:"fp"` } `json:"runs"` + Want *struct { + FP string `json:"fp"` + Self bool `json:"self"` + Removed bool `json:"removed"` + } `json:"want"` } if err := json.Unmarshal(raw, &a); err != nil { return "", fmt.Errorf("the answer of %s is not a composition: %w", compositionVerb, err) } + // While the engine replaces the indexer, this process is the successor: what runs still names the old build, and the want, + // composed by the node itself (self), names this one. The start fixes that build (quick fix, ADR 0639 ยง7: nothing is learnt + // later and nothing waits on a timer); a process of the old build that is started again in that window would take it too. + if a.Want != nil && a.Want.Self && !a.Want.Removed && a.Want.FP != "" { + return a.Want.FP, nil + } if a.Runs == nil || a.Runs.FP == "" { return "", errors.New("the engine says no fingerprint of the indexer runs") } diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index b8bfc4df..46f7d37d 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -121,6 +121,7 @@ type Indexer struct { filling atomic.Bool // a FillTaken runs in the background (KickFill) fillAgain atomic.Bool // a kick arrived while it ran atHeads map[string]map[string]delivery.PublishedPackages // a member head's published Go modules, by repository@head (candidatePublished) + failedAt map[string]string // the head of a repository whose packages the forge could not list, by repository: said once, not asked again until the head changes // OwnBuild answers the fingerprint of the build this program runs, from the node-engine's seat; nil knows none. OwnBuild func(ctx context.Context) (string, error) underSeats bool // a dry run inside judge.WithSeats: its judge passes do not hold the seat list (verdicts.go) diff --git a/modules/mesh-delivery-indexer/internal/index/packages.go b/modules/mesh-delivery-indexer/internal/index/packages.go index de1f0e73..b7cb7a6d 100644 --- a/modules/mesh-delivery-indexer/internal/index/packages.go +++ b/modules/mesh-delivery-indexer/internal/index/packages.go @@ -297,6 +297,12 @@ func (ix *Indexer) candidatePublished(ctx context.Context, members map[string]st ix.mu.Lock() got, ok := ix.atHeads[at] ix.mu.Unlock() + ix.mu.Lock() + failed := ix.failedAt[repo] == head + ix.mu.Unlock() + if failed { + continue // said once for this head; the candidate takes the published version until the head changes + } if !ok { var ms []string ms = append(ms, modules[repo]...) @@ -307,10 +313,17 @@ func (ix *Indexer) candidatePublished(ctx context.Context, members map[string]st } var err error if got, err = ix.packagesAt(ctx, repo, head, ms, imports[repo]); err != nil { - ix.logf("%s at the head %s: its packages are not read, so the candidate takes their published version: %v", repo, head, err) + ix.logf("%s at the head %s: its packages are not read, so the candidate takes their published version (said once for this head): %v", repo, head, err) + ix.mu.Lock() + if ix.failedAt == nil { + ix.failedAt = map[string]string{} + } + ix.failedAt[repo] = head + ix.mu.Unlock() continue } ix.mu.Lock() + delete(ix.failedAt, repo) if ix.atHeads == nil || len(ix.atHeads) >= maxHeadsKept { ix.atHeads = map[string]map[string]delivery.PublishedPackages{} } diff --git a/modules/mesh-delivery-indexer/internal/index/pending.go b/modules/mesh-delivery-indexer/internal/index/pending.go index cd34f376..ccc5673f 100644 --- a/modules/mesh-delivery-indexer/internal/index/pending.go +++ b/modules/mesh-delivery-indexer/internal/index/pending.go @@ -559,6 +559,10 @@ func (ix *Indexer) TakeInAtStart(ctx context.Context) (Applied, string, error) { if errors.Is(err, process.ErrNotYet) { return a, "the pending state is not on the bus yet: nothing waits", nil } + if err == nil && len(a.Written) == 0 { + // A start that takes nothing in says so, with the build it learnt (the wait's own build is in the line holdFor says). + ix.logf("take-in: at the start, this program runs build %s and took nothing in: %s", own, words) + } return a, words, err } @@ -978,6 +982,10 @@ func (ix *Indexer) holdFor(ctx context.Context, ready []PendingEntry, names, own for _, old := range saidWaits(ended) { ix.sayTakeIn(ctx, old) } + if said && !changed { + // Nothing to say on the bus, but the start decided nothing and a reader of the journal sees why: the build learnt, the build the wait names. + ix.logf("take-in: %d merges, oldest %s, still name the indexer build %s, this program runs %s: they wait, as already said", len(ready), t.Key, names, own) + } if !said || changed { ix.logf("take-in: %d merges, oldest %s, name the indexer build %s, not %s, which runs: they wait for it", len(ready), t.Key, names, own) if refused != "" { diff --git a/modules/mesh-delivery-indexer/internal/index/pending_test.go b/modules/mesh-delivery-indexer/internal/index/pending_test.go index 7f787eb7..f750a58c 100644 --- a/modules/mesh-delivery-indexer/internal/index/pending_test.go +++ b/modules/mesh-delivery-indexer/internal/index/pending_test.go @@ -281,6 +281,18 @@ func TestTakeIn(t *testing.T) { // 1585/T3-T13 (ADR 0633) n.FixOwnBuild(ctx) return n } + // A start that decides nothing says so (issue 1921): the build it learnt, the build the held wait names, also when the + // wait was said before; and indexer_pending names the known build and since when. + oldBuild, _ := b.ix.OwnBuild(ctx) + oldStart := mk(func(context.Context) (string, error) { return oldBuild, nil }) + b.logs.Reset() + if _, words, err := oldStart.TakeInAtStart(ctx); err != nil || len(b.keys()) != 2 || !strings.Contains(b.logs.String(), "still name the indexer build") || + !strings.Contains(b.logs.String(), "at the start, this program runs build") { + t.Fatalf("a start that holds: %q %v, journal %q: want the build learnt and the build the wait names said", words, err, b.logs.String()) + } + if answer, err := PendingTools(b.pending, b.ix.OwnBuildState, nil, nil)[0].Run(map[string]any{}); err != nil || answer.(takein.PendingAnswer).OwnBuild != oldBuild || answer.(takein.PendingAnswer).OwnBuildSince.IsZero() { + t.Errorf("indexer_pending = %+v %v, want the known build and since", answer, err) + } asked := 0 next := mk(func(context.Context) (string, error) { asked++; return "", errors.New("not yet") }) if r := next.TakeInPendingByHand(ctx, "m.nothing", "lead@shanks", "a test"); r.Stored || !strings.Contains(r.Answer, "nothing waits") { diff --git a/modules/mesh-delivery-indexer/internal/index/pending_tools.go b/modules/mesh-delivery-indexer/internal/index/pending_tools.go index 733cb70a..f0d24c1a 100644 --- a/modules/mesh-delivery-indexer/internal/index/pending_tools.go +++ b/modules/mesh-delivery-indexer/internal/index/pending_tools.go @@ -53,7 +53,7 @@ func PendingTools(pending, ownBuild Store, ask func(key string, value []byte) (T "merges, since when it waits, the modules it holds back (holds), the merges whose message the forge bridge's stream no " + "longer holds (lost: they are not taken in; indexer_drop_pending drops them) and the last take-in event said of it. Empty waits: " + "every merge is taken in. The watchman raises take-in-waits past 60 minutes for a revision, 30 for a group; " + - "indexer_take_in_pending and indexer_drop_pending steer a wait. own-build-unknown, when set, says the indexer cannot learn the build it " + + "indexer_take_in_pending and indexer_drop_pending steer a wait. own-build says which build the indexer knows it runs and own-build-since since when; own-build-unknown, when set, says the indexer cannot learn the build it " + "runs (since when): it fixes it once at its start, so every merge waits (a take-in.pending that says so) until the module is started again.", Input: map[string]any{"type": "object", "properties": map[string]any{}}, Run: func(map[string]any) (any, error) { @@ -69,8 +69,13 @@ func PendingTools(pending, ownBuild Store, ask func(key string, value []byte) (T if ownBuild != nil { if v, _, found, err := ownBuild.Get(context.Background(), OwnBuildKey); err == nil && found { var o OwnBuild - if json.Unmarshal(v, &o) == nil && !o.Known { - answer.OwnBuildUnknown, answer.OwnBuildSince = o.Why, o.Since + if json.Unmarshal(v, &o) == nil { + answer.OwnBuildSince = o.Since + if o.Known { + answer.OwnBuild = o.FP + } else { + answer.OwnBuildUnknown = o.Why + } } } } diff --git a/modules/mesh-delivery-indexer/internal/index/previews_test.go b/modules/mesh-delivery-indexer/internal/index/previews_test.go index 3edcb9ef..63060d55 100644 --- a/modules/mesh-delivery-indexer/internal/index/previews_test.go +++ b/modules/mesh-delivery-indexer/internal/index/previews_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "os" "runtime" "slices" @@ -404,6 +405,28 @@ func TestAPreviewHeardBeforeItsPullRequestGivesTheFingerprintWhenTheCandidateIsW } }) } + // Issue 1921: a head whose packages the forge cannot list is said once and asked once, however many passes read it; the + // entry is dropped when the repository's head changes, and the new head is asked. + t.Run("a failing head is said and asked once, until the head changes", func(t *testing.T) { + ix, _, _ := previewBed(t, false) + var logs strings.Builder + ix.Logf = func(format string, args ...any) { logs.WriteString(fmt.Sprintf(format, args...) + "\n") } + asked := 0 + ix.Files = func(context.Context, string, string, []string) (map[string]map[string]string, error) { + asked++ + return nil, errors.New("the forge does not answer") + } + held := map[string]delivery.Module{"x": {Inputs2: &delivery.KeptInputs2{Packages: []string{"git.novox.be/novox/mesh-sdk/go"}}}} + for _, head := range []string{h1, h1, h2} { + ix.candidatePublished(context.Background(), map[string]string{"novox/mesh-sdk": head}, held) + if head == h1 && (asked != 1 || strings.Count(logs.String(), "its packages are not read") != 1) { + t.Fatalf("after head %s: asked %d, journal %q: want one ask and one line", head, asked, logs.String()) + } + } + if asked != 2 || strings.Count(logs.String(), "its packages are not read") != 2 { + t.Errorf("after a new head: asked %d, journal %q: want it asked and said again", asked, logs.String()) + } + }) } func TestAPreviewNamingNoManifestOfTheModulesDirectoryGivesAReason(t *testing.T) { diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts.go b/modules/mesh-delivery-indexer/internal/index/verdicts.go index 1758f740..7f931a39 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts.go @@ -396,6 +396,7 @@ func (ix *Indexer) FixOwnBuild(ctx context.Context) { ix.logf("judge: the build this program runs is not known (%v), so every merge waits and a manifest with a field it does not know waits and is not refused; it is learnt only at the start, so the indexer is started again", err) } else { st.Known, st.FP = true, fp + ix.logf("judge: this program runs the indexer build %s, fixed here at its start", fp) } ix.ownMu.Lock() ix.ownFP, ix.ownSince, ix.ownWhy = fp, st.Since, st.Why diff --git a/modules/mesh-delivery-indexer/takein/takein.go b/modules/mesh-delivery-indexer/takein/takein.go index af9ba44f..4ad0ca9e 100644 --- a/modules/mesh-delivery-indexer/takein/takein.go +++ b/modules/mesh-delivery-indexer/takein/takein.go @@ -254,6 +254,8 @@ type PendingAnswer struct { // (a take-in.pending with the same words) and the indexer is started again to learn it. Empty when the build is known. OwnBuildUnknown string `json:"own-build-unknown,omitempty"` OwnBuildSince time.Time `json:"own-build-since,omitzero"` + // OwnBuild is the build the running indexer knows it runs, fixed at its start; OwnBuildSince is since when. Empty when unknown. + OwnBuild string `json:"own-build,omitempty"` } // DecodePendingAnswer reads indexer_pending's answer, refusing a wait that is not a revision or a group, or names no