From a5ba47e23c35acd4072644717d048e0dd1d1a985 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 00:45:00 +0200 Subject: [PATCH 1/4] Name the open walk a change is related to, own path related to all (issue 973) --- cmd/mesh-controller/related.go | 32 +++++++++++++++++++- cmd/mesh-controller/related_test.go | 47 +++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/cmd/mesh-controller/related.go b/cmd/mesh-controller/related.go index fdaefed0..1d9858aa 100644 --- a/cmd/mesh-controller/related.go +++ b/cmd/mesh-controller/related.go @@ -11,7 +11,9 @@ import ( // related when they share a repository, a module, a delivery group's head, or an edge of the module graph // between their modules. A module is every one the walk builds or sends: the modules its merges moved and the // modules that depend on them, not only the ones the merges touched. Unrelated changes may walk beside each -// other (ADR 0304 decision 2, ADR 0282 decision 3); nothing walks so yet, and nothing but tests reads this. +// other (ADR 0304 decision 2, ADR 0282 decision 3); a closed batch is cut beside an open walk it is +// not related to, and a walk about to ask its first tier waits only for a started walk it is related to (issue 973, +// ADR 0407 decision 4). // footprint is what one change reaches: its repositories (lower case), its modules, and its delivery group // heads (the head branch names of its merges). unknown says a merge of it was kept without the modules it @@ -109,3 +111,31 @@ func firstShared(a, b map[string]bool) string { sort.Strings(both) return both[0] } + +// openWalk is a walk open beside a change: its id, what it reaches, and whether it is on the controller's own path. +type openWalk struct { + id string + fp footprint + own bool +} + +// relatedOpen is the first open walk a change is related to, with the reason; empty when it is related to none +// and may walk beside them all (issue 973, ADR 0407 decision 4, ADR 0306 decision 3). +// +// **The controller's own path is related to everything** (decided for issue 973): a walk or batch that replaces +// the controller, the node-engine, the tool runner, the bus or mesh-delivery never runs beside another, and +// nothing runs beside it, whatever the footprints say. +func relatedOpen(cand footprint, candOwn bool, open []openWalk, edges []inventory.Edge) (behind, why string) { + for _, w := range open { + if candOwn { + return w.id, "it is on the controller's own path" + } + if w.own { + return w.id, w.id + " is on the controller's own path" + } + if ok, why := related(cand, w.fp, edges); ok { + return w.id, why + } + } + return "", "" +} diff --git a/cmd/mesh-controller/related_test.go b/cmd/mesh-controller/related_test.go index 764df1d3..41593367 100644 --- a/cmd/mesh-controller/related_test.go +++ b/cmd/mesh-controller/related_test.go @@ -108,3 +108,50 @@ func TestABatchWhoseMovesAreNotKnownIsRelatedToEverything(t *testing.T) { t.Fatalf("a known, unrelated batch read as related: %q", why) } } + +// TestRelatedOpenNamesTheFirstOpenWalkAChangeIsRelatedTo: a change that shares nothing with any open walk is +// held behind none (issue 973, ADR 0407 decision 4); one that shares a repository, a module, a graph edge or a +// group head is held behind the first walk it shares it with, with the reason; one whose moves are not known +// is held behind the first; a walk or change on the controller's own path is related to every other. +func TestRelatedOpenNamesTheFirstOpenWalkAChangeIsRelatedTo(t *testing.T) { + edges := []inventory.Edge{{From: "nzbget", To: "base-image", Kind: inventory.EdgeStandsOn}} + walk := func(id, repo string, own bool, modules ...string) openWalk { + return openWalk{id: id, fp: footprintOfWalk(walkOf(repo, modules...), nil), own: own} + } + one, two := walk("plan-1", "novox/one", false, "notes"), walk("plan-2", "novox/two", false, "nzbget") + cand := func(repo, head string, moves ...string) footprint { + return footprintOfBatch([]inventory.BatchedMerge{heardMerge(repo, head, moves...)}, edges) + } + cases := []struct { + name string + fp footprint + own bool + open []openWalk + behind string + why string + }{ + {"nothing open", cand("novox/x", "fix/a", "app"), false, nil, "", ""}, + {"unrelated to every open walk", cand("novox/x", "fix/a", "app"), false, []openWalk{one, two}, "", ""}, + {"a repository", cand("novox/ONE", "fix/a", "app"), false, []openWalk{two, one}, "plan-1", "both change novox/one"}, + {"a module through its dependents", cand("novox/x", "fix/a", "base-image"), false, []openWalk{one, two}, + "plan-2", "both move nzbget"}, + {"a graph edge", cand("novox/x", "fix/a", "nzbget"), + false, []openWalk{one, walk("plan-3", "novox/y", false, "base-image")}, "plan-3", + "nzbget depends on base-image (stands-on)"}, + {"moves not known", cand("novox/x", "fix/a"), false, []openWalk{one, two}, "plan-1", + "the modules a merge of novox/x moves are not known until its batch is cut"}, + {"the candidate on the own path", cand("novox/x", "fix/a", "app"), true, []openWalk{one}, "plan-1", + "it is on the controller's own path"}, + {"an open walk on the own path", cand("novox/x", "fix/a", "app"), false, + []openWalk{one, walk("plan-4", "novox/mesh-controller", true, "mesh-controller")}, "plan-4", + "plan-4 is on the controller's own path"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + behind, why := relatedOpen(c.fp, c.own, c.open, edges) + if behind != c.behind || why != c.why { + t.Errorf("relatedOpen = %q, %q; want %q, %q", behind, why, c.behind, c.why) + } + }) + } +} -- 2.54.0 From baf1501d815d1febaa557810bbe8c7f123b21891 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 00:46:27 +0200 Subject: [PATCH 2/4] A walk, a retry and a let-go wait only for a started walk they are related to (issue 973) --- cmd/mesh-controller/delivery.go | 5 ++- cmd/mesh-controller/plan_retry.go | 19 +++++++-- cmd/mesh-controller/related.go | 54 ++++++++++++++++++++++++ cmd/mesh-controller/related_test.go | 65 +++++++++++++++++++++++++++++ cmd/mesh-controller/release_plan.go | 46 ++++++++++++-------- 5 files changed, 165 insertions(+), 24 deletions(-) diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 8db2bca5..97500f76 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -136,8 +136,9 @@ func letGo(ctx context.Context, inv *inventory.Inventory, id, by, why string) (i return p, fmt.Errorf("%s was let go by %s at %s already", p.ID, p.Delivery.By, p.Delivery.Go.Local().Format("15:04:05")) } - // **One walk at a time** (novox/hq ADR 0276): the word is taken, and the walk starts once no other walk is - // started (advanceOnce), so the delivery's owner says it once and is not refused. + // **One walk at a time beside a related one** (novox/hq ADR 0276, ADR 0407 decision 4, issue 973): the word is + // taken, and the walk starts once no started walk it is related to is open (advanceOnce), so the delivery's + // owner says it once and is not refused. now := time.Now().UTC() p.Delivery.Go, p.Delivery.By, p.Delivery.Why = &now, by, why p.Note = "let go by " + by + "; its first tier is asked next" diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index acb722c8..fbc075e8 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -197,12 +197,23 @@ func retryRefusal(p inventory.Plan, plans []inventory.Plan) error { return oneWalkAtATime(p, plans) } -// oneWalkAtATime refuses a retry while another walk is open, started or waiting for its word (novox/hq ADR -// 0276): a walk retried beside it would be two walks at once. +// oneWalkAtATime refuses a retry while an open walk, started or waiting for its word, is related to it (novox/hq +// ADR 0276, ADR 0407 decision 4, issue 973): a walk retried beside a related one would be two walks on the same +// things at once. A walk related to none may be retried beside them. The retried walk is judged by the modules it +// plans, which already hold their dependents, and the repositories it carries. func oneWalkAtATime(p inventory.Plan, plans []inventory.Plan) error { + mine := footprintOfWalk(p, nil) for _, q := range plans { - if q.ID != p.ID && q.Open() && q.Release == nil { - return fmt.Errorf("%s is open (%s): one walk at a time — retry %s once it ended", q.ID, q.Named(), p.ID) + if q.ID == p.ID || !q.Open() || q.Release != nil { + continue + } + if ownWalk(p) || ownWalk(q) { + return fmt.Errorf("%s is open (%s) and one of them is on the controller's own path: one walk at a time — "+ + "retry %s once it ended", q.ID, q.Named(), p.ID) + } + if ok, why := related(mine, footprintOfWalk(q, nil), nil); ok { + return fmt.Errorf("%s is open (%s) and %s: one walk at a time beside a related one — retry %s once it ended", + q.ID, q.Named(), why, p.ID) } } return nil diff --git a/cmd/mesh-controller/related.go b/cmd/mesh-controller/related.go index 1d9858aa..fe0dc647 100644 --- a/cmd/mesh-controller/related.go +++ b/cmd/mesh-controller/related.go @@ -1,8 +1,10 @@ package main import ( + "context" "sort" "strings" + "time" "github.com/novox/mesh-controller/internal/inventory" ) @@ -139,3 +141,55 @@ func relatedOpen(cand footprint, candOwn bool, open []openWalk, edges []inventor } return "", "" } + +// ownWalk says a walk is on the controller's own path: its batch says so, or it plans a module of that path +// (issue 973). +func ownWalk(p inventory.Plan) bool { + if p.OwnPath() { + return true + } + for name := range p.Modules { + if _, own := onTheControllersPath[name]; own { + return true + } + } + return false +} + +// startedWalks are the open walks started beside the walk id, made at created: a merge's walk past its wait, not +// the backlog's, with a tier asked, or on its way (let go, or waiting for nobody) before this one. +func startedWalks(plans []inventory.Plan, id string, created time.Time) []inventory.Plan { + var out []inventory.Plan + for _, q := range plans { + if q.ID == id || q.Release != nil || q.Waiting() { + continue + } + if q.Tier > 0 || askedAny(q) || q.Created.Before(created) { + out = append(out, q) + } + } + return out +} + +// openWalkOf is an open walk as a change is judged against it: what it reaches, from its planned modules and the +// merges the record holds for it. +func openWalkOf(ctx context.Context, inv *inventory.Inventory, p inventory.Plan) (openWalk, error) { + kept, err := inv.MergesOf(ctx, p.ID) + if err != nil { + return openWalk{}, err + } + return openWalk{id: p.ID, fp: footprintOfWalk(p, kept), own: ownWalk(p)}, nil +} + +// openWalksOf are openWalkOf for each of plans. +func openWalksOf(ctx context.Context, inv *inventory.Inventory, plans []inventory.Plan) ([]openWalk, error) { + var out []openWalk + for _, p := range plans { + w, err := openWalkOf(ctx, inv, p) + if err != nil { + return nil, err + } + out = append(out, w) + } + return out, nil +} diff --git a/cmd/mesh-controller/related_test.go b/cmd/mesh-controller/related_test.go index 41593367..f45fcdf0 100644 --- a/cmd/mesh-controller/related_test.go +++ b/cmd/mesh-controller/related_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "strings" "testing" + "time" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" @@ -155,3 +156,67 @@ func TestRelatedOpenNamesTheFirstOpenWalkAChangeIsRelatedTo(t *testing.T) { }) } } + +// A walk is on the controller's own path when its batch says so or it plans a module of that path (issue 973). +func TestAWalkOnTheControllersPathIsKnownByItsFlagOrItsModules(t *testing.T) { + if ownWalk(walkOf("novox/one", "notes")) { + t.Error("an ordinary walk read as on the controller's own path") + } + flagged := walkOf("novox/one", "notes") + flagged.Delivery = &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: true}} + if !ownWalk(flagged) { + t.Error("a walk whose batch is own-path read as ordinary") + } + var module string + for m := range onTheControllersPath { + module = m + } + if !ownWalk(walkOf("novox/one", "notes", module)) { + t.Errorf("a walk planning %s read as ordinary", module) + } +} + +// A walk about to ask its first tier waits only for a started walk it is related to (issue 973): started is a +// tier asked, or a walk made before it; a waiting walk, the backlog's and itself are not. +func TestWalksStartedBesideAreTheOpenOnesBeforeIt(t *testing.T) { + at := time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC) + mk := func(id string, created time.Time, tier int) inventory.Plan { + p := walkOf("novox/"+id, id) + p.ID, p.Created, p.Tier, p.State = id, created, tier, inventory.PlanBuilding + return p + } + older, newer, ticking := mk("older", at, 0), mk("newer", at.Add(time.Minute), 0), mk("ticking", at.Add(2*time.Minute), 1) + backlog := mk("backlog", at, 1) + backlog.Release = &inventory.PlanRelease{} + got := startedWalks([]inventory.Plan{older, newer, ticking, backlog, mk("me", at.Add(time.Minute), 0)}, "me", at.Add(time.Minute)) + var ids []string + for _, p := range got { + ids = append(ids, p.ID) + } + if strings.Join(ids, ",") != "older,ticking" { + t.Errorf("started beside me = %v; want older, ticking", ids) + } +} + +// A retry is refused beside an open walk it is related to and allowed beside one it shares nothing with +// (issue 973); a retry on the controller's own path is refused beside any open walk. +func TestARetryWaitsOnlyForAnOpenWalkItIsRelatedTo(t *testing.T) { + failed := walkOf("novox/one", "notes") + failed.ID, failed.State = "plan-1", inventory.PlanFailed + open := func(id, repo string, modules ...string) inventory.Plan { + p := walkOf(repo, modules...) + p.ID, p.State = id, inventory.PlanBuilding + return p + } + if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}); err != nil { + t.Errorf("refused beside an unrelated walk: %v", err) + } + err := oneWalkAtATime(failed, []inventory.Plan{open("plan-3", "novox/three", "notes")}) + if err == nil || !strings.Contains(err.Error(), "plan-3") || !strings.Contains(err.Error(), "both move notes") { + t.Errorf("a retry beside a walk moving the same module: %v", err) + } + failed.Delivery = &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: true}} + if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}); err == nil { + t.Error("an own-path retry went beside an open walk") + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 46cf11fe..99a8fbbd 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -618,23 +618,32 @@ func planBuiltSaying(ctx context.Context, open *stores, module, commit, failed s advanceHeld(ctx, open) } -// startedBeside is the id of a started walk open beside this one — a merge's walk past its wait, not the -// backlog's — or empty: one walk at a time (novox/hq ADR 0276). -func startedBeside(ctx context.Context, inv *inventory.Inventory, id string, created time.Time) (string, error) { +// startedBeside is the id of a started walk open beside this one that it is related to — a merge's walk past +// its wait, not the backlog's — with why, or empty: a walk waits only for a started walk it shares something with +// (novox/hq ADR 0276 decision 4, ADR 0407 decision 4, issue 973). +func startedBeside(ctx context.Context, inv *inventory.Inventory, p inventory.Plan) (behind, why string, err error) { plans, err := inv.OpenPlans(ctx) if err != nil { - return "", err + return "", "", err } - for _, q := range plans { - if q.ID == id || q.Release != nil || q.Waiting() { - continue - } - // Started: a tier asked, or on its way (let go, or waiting for nobody) before this one. - if q.Tier > 0 || askedAny(q) || q.Created.Before(created) { - return q.ID, nil - } + started := startedWalks(plans, p.ID, p.Created) + if len(started) == 0 { + return "", "", nil } - return "", nil + edges, err := inv.Dependencies(ctx) + if err != nil { + return "", "", err + } + open, err := openWalksOf(ctx, inv, started) + if err != nil { + return "", "", err + } + mine, err := openWalkOf(ctx, inv, p) + if err != nil { + return "", "", err + } + behind, why = relatedOpen(mine.fp, mine.own, open, edges) + return behind, why, nil } // deferredNote begins the note of a walk deferred behind another. @@ -793,13 +802,14 @@ func advanceOnce(ctx context.Context, open *stores, p *inventory.Plan, tier := p.Tiers[p.Tier] // Not yet asked: ask — the whole tier, or what of it is still unasked (novox/hq issue 754). if unasked := unaskedOf(*p); len(unasked) > 0 { - // **One walk at a time** (novox/hq ADR 0276): a walk about to ask its first tier while another started - // walk is open waits for that one to end, let go or not, and says so. + // **One walk at a time beside a related one** (novox/hq ADR 0276, ADR 0407 decision 4, issue 973): a walk + // about to ask its first tier while a started walk it is related to is open waits for that one to end, + // let go or not, and says so and why. A started walk it shares nothing with does not hold it. if p.Tier == 0 && !askedInTier(*p) { - if behind, err := startedBeside(ctx, inv, p.ID, p.Created); err != nil { + if behind, why, err := startedBeside(ctx, inv, *p); err != nil { return false, err } else if behind != "" { - note := fmt.Sprintf("%s%s ended — one walk at a time", deferredNote, behind) + note := fmt.Sprintf("%s%s ended — %s", deferredNote, behind, why) changed := p.Note != note p.Note = note return changed, nil @@ -1722,7 +1732,7 @@ func planLineSaid(p inventory.Plan, now time.Time, pause pauseView, bound time.D return fmt.Sprintf("%s · %s, %s, for %s", name, where, waitingSaid(p, waits[p.ID]), since) } if deferred(p) { - // Nor is waiting for the walk before it to end (one walk at a time, novox/hq ADR 0276). + // Nor is waiting for the walk before it to end (a related walk, novox/hq ADR 0276, ADR 0407). return fmt.Sprintf("%s · %s, %s, for %s", name, where, p.Note, since) } if waiting, paused := pausedWaiting(p, pause, now); paused { -- 2.54.0 From 300d3c1bf466ada06c4523246a48c18d0dbeb93f Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 00:49:53 +0200 Subject: [PATCH 3/4] Cut a closed batch beside the open walks it shares nothing with, queue a related one with the reason, fold only related waiting walks (issue 973) --- cmd/mesh-controller/batches.go | 165 ++++++++++++++++++------- cmd/mesh-controller/batches_test.go | 18 +-- cmd/mesh-controller/beside_test.go | 106 ++++++++++++++++ cmd/mesh-controller/lone_merge_test.go | 10 +- internal/inventory/plans.go | 2 + testdata/store-tests | 3 + 6 files changed, 248 insertions(+), 56 deletions(-) create mode 100644 cmd/mesh-controller/beside_test.go diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index e5d3ea50..ee000b88 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -27,9 +27,10 @@ import ( // commit in it, with at most one commit per repository, the latest of its branch, which contains the earlier // ones. The walk names every merge it answers. // -// - One walk at a time. Merges that come while a walk runs join the next batch. A walk that started is never -// superseded; one still waiting for its delivery's word is folded into the next batch's walk, which -// answers every merge it answered — the only supersession left. +// - One walk at a time beside a related one (ADR 0407 decision 4, issue 973). Merges that come while a walk +// runs join the next batch; a closed batch that shares nothing with an open walk is cut beside it. A walk +// that started is never superseded; one still waiting for its delivery's word is folded into the walk of a +// batch related to it, which answers every merge it answered — the only supersession left. // - A merge heard after a later merge of its branch was put in a walk is answered by that walk: a batch // still assembling, an open walk that builds everything it moves, or a walk done that did, which then // names it at once. Otherwise it joins the next batch. @@ -39,8 +40,9 @@ import ( // // **A merge waits only for a batch it shares something with** (novox/hq ADR 0304 decision 1, ADR 0306 decisions // 2 and 3, issue 525): it joins a batch it is related to, and one that is related to none and whose delivery -// group has no member still to merge is a batch whose window is closed at once (batchToJoin). Walks stay one at -// a time: such a batch is cut when no walk is open, and queued behind the one that is otherwise. +// group has no member still to merge is a batch whose window is closed at once (batchToJoin). Such a batch is cut +// at once, beside any open walk it shares nothing with, and queued behind the first one it is related to +// (cutUnrelated). // // A merge on the controller's own path (a module whose walk waits for nobody's word) never shares a batch with // one that waits for mesh-delivery's: each kind has a batch of its own, an own-path batch is cut first and folds @@ -340,8 +342,8 @@ func ownPath(touches []inventory.Entry) bool { // names why): a new batch, which waits for the window, beside the ones it shares nothing with; its words say // why. // - none, and no member of its group is still to merge: a new batch whose window is closed at once. Cut at -// once means only that: walks stay one at a time (ADR 0276 decision 4), so with a walk open the batch is -// queued behind it as any closed batch is. +// once means only that: with a walk open that it is related to the batch is queued behind it as any closed +// batch is (ADR 0276 decision 4, ADR 0407 decision 4); beside one it shares nothing with it is cut. func batchToJoin(ctx context.Context, inv *inventory.Inventory, heard inventory.BatchedMerge, entries []inventory.Entry, read map[string][]inventory.ReadRepository, now time.Time, own bool, waits string) (inventory.Plan, string, error) { edges, err := inv.Dependencies(ctx) @@ -406,7 +408,9 @@ func relatedBatches(ctx context.Context, inv *inventory.Inventory, merges []inve } carry := newFootprint() if !own { - // An own-path batch folds no waiting walk (cutBatchesHeld); a catalogue batch may fold every one. + // An own-path batch folds no waiting walk (cutBatchesHeld); a catalogue batch folds those it is related to + // (cutUnrelated). Every one is counted here, which only makes a merge join a batch more readily: it waits + // for the window, never walks beside what it shares something with (issue 973). plans, err := inv.OpenPlans(ctx) if err != nil { return nil, nil, nil, err @@ -544,6 +548,11 @@ func newBatch(ctx context.Context, inv *inventory.Inventory, first inventory.Bat // keepBatch writes a batch as its merges and the clock say it now: the commits it holds, the merges they // answer, its window, and assembling or queued behind a walk. Saved only when that changed. func keepBatch(ctx context.Context, inv *inventory.Inventory, b *inventory.Plan, now time.Time, behind string) error { + return keepBatchBecause(ctx, inv, b, now, behind, "") +} + +// keepBatchBecause is keepBatch for a batch queued behind a walk it is related to, which its words say (issue 973). +func keepBatchBecause(ctx context.Context, inv *inventory.Inventory, b *inventory.Plan, now time.Time, behind, why string) error { merges, err := inv.MergesOf(ctx, b.ID) if err != nil { return err @@ -574,13 +583,13 @@ func keepBatch(ctx context.Context, inv *inventory.Inventory, b *inventory.Plan, cutAtOnce = was.CutAtOnce // the merge's, until another joins it (ADR 0304 decision 1) waits = was.Waits // its group's, as said when it was made (ADR 0306 decision 2) } - b.Delivery.Batch = &inventory.PlanBatch{ClosesAt: closes, AtMost: latest, Behind: behind, Own: b.OwnPath(), + b.Delivery.Batch = &inventory.PlanBatch{ClosesAt: closes, AtMost: latest, Behind: behind, BehindWhy: why, Own: b.OwnPath(), Closed: closed, Withheld: withheld, CutAtOnce: cutAtOnce, Waits: waits} b.State = inventory.PlanAssembling if behind != "" && windowClosed(*b, now) { b.State = inventory.PlanQueued } else { - b.Delivery.Batch.Behind = "" + b.Delivery.Batch.Behind, b.Delivery.Batch.BehindWhy = "", "" } b.Note = batchWords(*b, now) if unNoted(*b) == before { @@ -761,9 +770,9 @@ func batchCutter(ctx context.Context, open *stores) { } // cutBatchesHeld takes the step that is due, for a caller holding the plans: after a failed walk, the search -// for the merge that brought the failure; with no walk open, the next merge of that search walked alone, or a -// closed batch cut into one walk with the waiting walks folded in; with a walk open, a closed batch said -// queued behind it. +// for the merge that brought the failure; with no walk open, the next merge of that search walked alone; else +// each closed batch cut into a walk with the waiting walks it is related to folded in, unless it is related to a +// walk open, behind which it is said queued (cutUnrelated). While the search runs everything waits. func cutBatchesHeld(ctx context.Context, open *stores, now time.Time) error { inv := open.inventory if err := searchAfterFailures(ctx, inv); err != nil { @@ -780,8 +789,7 @@ func cutBatchesHeld(ctx context.Context, open *stores, now time.Time) error { if err != nil { return err } - var started *inventory.Plan - var waiting []inventory.Plan + var started, waiting []inventory.Plan for i := range plans { p := plans[i] if p.Release != nil { @@ -789,8 +797,8 @@ func cutBatchesHeld(ctx context.Context, open *stores, now time.Time) error { } if p.Waiting() { waiting = append(waiting, p) - } else if started == nil { - started = &plans[i] + } else { + started = append(started, p) } } batches, err := inv.Batches(ctx) @@ -805,46 +813,113 @@ func cutBatchesHeld(ctx context.Context, open *stores, now time.Time) error { } return nil } - if started != nil { - // **One walk at a time** (ADR 0276 decision 4): every batch waits behind it, whatever its window says. - return keepAll(started.ID) - } if len(alone) > 0 { + // A failed walk's search walks its merges alone, one at a time: everything waits for it (kept as it was, + // issue 973). + if len(started) > 0 { + return keepAll(started[0].ID) + } if len(waiting) > 0 { // The waiting walk is the one open; the search walks after it, and the batches wait behind both. return keepAll(waiting[0].ID) } return walkAlone(ctx, open, alone[0], now) } - if err := keepAll(""); err != nil { + return cutUnrelated(ctx, open, batches, started, waiting, now) +} + +// cutUnrelated cuts the closed batches that share nothing with an open walk, each into a walk beside the open +// ones, and says the others queued behind the first walk they are related to (novox/hq ADR 0407 decision 4, +// issue 973). A batch cut in this look counts as open for the batches after it. +// +// - An own-path batch goes first and folds no waiting walk; a catalogue batch folds the waiting walks it is +// related to, and those it shares nothing with keep waiting. +// - **The controller's own path is related to everything** (decided for issue 973): a walk or batch that +// replaces the controller, the node-engine, the tool runner, the bus or mesh-delivery never runs beside +// another, and nothing runs beside it. +// - A batch whose window is still open is assembling; one related to an open walk is queued behind it, and +// ADR 0276 decision 4 stands for it: nothing supersedes a walk that started. +func cutUnrelated(ctx context.Context, open *stores, batches, started, waiting []inventory.Plan, now time.Time) error { + inv := open.inventory + sort.SliceStable(batches, func(i, j int) bool { return batches[i].OwnPath() && !batches[j].OwnPath() }) + var entries []inventory.Entry + var read map[string][]inventory.ReadRepository + var edges []inventory.Edge + judged := false + openWalks, err := openWalksOf(ctx, inv, started) + if err != nil { return err } - // The oldest batch whose window closed is cut, an own-path one before a catalogue one (it goes first, and - // the catalogue batch queues behind it rather than being cut and overtaken). Several of a kind may be open - // since a merge that shares nothing is cut at once (ADR 0304 decision 1): walks stay one at a time, so one - // is cut per look, and once its walk started the others are queued behind it. - sort.SliceStable(batches, func(i, j int) bool { return batches[i].OwnPath() && !batches[j].OwnPath() }) for i := range batches { b := &batches[i] if !windowClosed(*b, now) { - continue - } - if b.OwnPath() { - // A walk on the controller's own path folds no walk waiting for its word (decided during the build, - // 2026-10-10): that walk keeps waiting beside it, and starts once this one ended (advanceOnce). - waiting = nil - } - if err := cutBatch(ctx, open, b, waiting, now); err != nil { - return err - } - // The batches left wait behind the walk just cut when it started; the next look does the same. - if b.Open() && !b.Waiting() { - if batches, err = inv.Batches(ctx); err != nil { + if err := keepBatch(ctx, inv, b, now, ""); err != nil { return err } - return keepAll(b.ID) + continue + } + if !judged { + var err error + if entries, err = inv.Catalogued(ctx); err != nil { + return err + } + if read, err = readForPlanning(ctx, inv); err != nil { + return err + } + if edges, err = inv.Dependencies(ctx); err != nil { + return err + } + judged = true + } + held, err := inv.MergesOf(ctx, b.ID) + if err != nil { + return err + } + fp := footprintAsCut(held, entries, read, edges) + var fold, keep []inventory.Plan + if !b.OwnPath() { + for _, w := range waiting { + ww, err := openWalkOf(ctx, inv, w) + if err != nil { + return err + } + if ok, _ := related(fp, ww.fp, edges); ok { + fold = append(fold, w) + fp = unionFootprint(fp, ww.fp) + } else { + keep = append(keep, w) + } + } + } + if behind, why := relatedOpen(fp, b.OwnPath(), openWalks, edges); behind != "" { + if err := keepBatchBecause(ctx, inv, b, now, behind, why); err != nil { + return err + } + continue + } + if err := keepBatch(ctx, inv, b, now, ""); err != nil { + return err + } + if err := cutBatch(ctx, open, b, fold, now); err != nil { + return err + } + waiting = keep + cut, err := inv.PlanByID(ctx, b.ID) + if err != nil { + return err + } + if !cut.Open() { + continue + } + w, err := openWalkOf(ctx, inv, cut) + if err != nil { + return err + } + if cut.Waiting() { + waiting = append(waiting, cut) + } else { + openWalks = append(openWalks, w) } - return nil } return nil } @@ -1396,7 +1471,11 @@ func batchWords(b inventory.Plan, now time.Time) string { grouped += " (on the controller's own path)" } if b.State == inventory.PlanQueued { - return fmt.Sprintf("queued behind %s; grouped: %s; plan not yet calculated", w.Behind, grouped) + because := "" + if w.BehindWhy != "" { + because = " (" + w.BehindWhy + ")" + } + return fmt.Sprintf("queued behind %s%s; grouped: %s; plan not yet calculated", w.Behind, because, grouped) } if w.CutAtOnce { return fmt.Sprintf("cut at once, sharing nothing with a batch assembling; grouped: %s; plan not yet calculated", diff --git a/cmd/mesh-controller/batches_test.go b/cmd/mesh-controller/batches_test.go index 9e6bba74..7b105f69 100644 --- a/cmd/mesh-controller/batches_test.go +++ b/cmd/mesh-controller/batches_test.go @@ -334,8 +334,8 @@ func captured(t *testing.T, run func() error) string { return b.String() } -// One walk at a time: a merge heard while a walk runs joins the next batch, which is cut when the walk ends; -// the walk that started is never superseded. +// A merge heard during a walk it is related to (the same repository, issue 973) joins the next batch, which is +// cut when the walk ends; the walk that started is never superseded (ADR 0276 decision 4, ADR 0407 decision 4). func TestAMergeDuringAWalkJoinsTheNextBatch(t *testing.T) { open := windowed(t) asksWithPaths(t) @@ -346,7 +346,7 @@ func TestAMergeDuringAWalkJoinsTheNextBatch(t *testing.T) { t.Fatalf("the first batch was not cut: %+v", ws) } first := ws[0] - hear(t, open, repoMerge("two", "c2", t0.Add(2*time.Minute)), t0.Add(2*time.Minute)) + hear(t, open, repoMerge("one", "c2", t0.Add(2*time.Minute)), t0.Add(2*time.Minute)) cutAt(t, open, t0.Add(5*time.Minute)) ws, bs := walks(t, open) if len(ws) != 1 || len(bs) != 1 || bs[0].State != inventory.PlanQueued || bs[0].Delivery.Batch.Behind != first.ID { @@ -391,7 +391,7 @@ func TestAWaitingWalkIsFoldedIntoTheNextBatch(t *testing.T) { t.Fatalf("the walk does not wait for its word: %+v", ws) } waiting := ws[0] - hear(t, open, repoMerge("two", "c2", t0.Add(2*time.Minute)), t0.Add(2*time.Minute)) + hear(t, open, repoMerge("one", "c2", t0.Add(2*time.Minute)), t0.Add(2*time.Minute)) cutAt(t, open, t0.Add(2*time.Minute+90*time.Second)) ws, bs := walks(t, open) if len(bs) != 0 || len(ws) != 2 { @@ -405,7 +405,7 @@ func TestAWaitingWalkIsFoldedIntoTheNextBatch(t *testing.T) { if folded.State != inventory.PlanSuperseded || folded.Delivery.TakenOverBy != newer.ID { t.Fatalf("the waiting walk is %s, taken over by %q", folded.State, folded.Delivery.TakenOverBy) } - if len(newer.Delivery.Merges) != 2 || newer.CommitOf("novox/one") != "c1" || newer.CommitOf("novox/two") != "c2" { + if len(newer.Delivery.Merges) != 2 || newer.CommitOf("novox/one") != "c2" { t.Fatalf("the newer walk answers %+v", newer.Delivery.Merges) } if _, in := newer.Modules["one"]; !in { @@ -662,8 +662,8 @@ func TestALateMergeOfAOneModuleRepositoryIsNamed(t *testing.T) { } } -// One walk at a time holds against the delivery's word too: a waiting walk let go beside a started one takes the -// word and starts once that one ended, asking nothing before. +// A related walk waits for the delivery's word too: a waiting walk let go beside a started one it is related to +// (the same repository, issue 973) takes the word and starts once that one ended, asking nothing before. func TestAWalkLetGoBesideAStartedOneStartsOnceItEnded(t *testing.T) { open := windowed(t) asked := asksWithPaths(t) @@ -671,8 +671,8 @@ func TestAWalkLetGoBesideAStartedOneStartsOnceItEnded(t *testing.T) { hear(t, open, repoMerge("one", "c1", t0), t0) cutAt(t, open, t0.Add(2*time.Minute)) started, _ := walks(t, open) - waiting := inventory.Plan{ID: "plan-waiting", Repository: "novox/two", Branch: "main", Commit: "d1", Created: t0, - State: inventory.PlanBuilding, Tiers: [][]string{{"two"}}, Modules: map[string]*inventory.PlanModule{"two": {}}, + waiting := inventory.Plan{ID: "plan-waiting", Repository: "novox/one", Branch: "main", Commit: "d1", Created: t0, + State: inventory.PlanBuilding, Tiers: [][]string{{"one"}}, Modules: map[string]*inventory.PlanModule{"one": {}}, Delivery: &inventory.PlanDelivery{Awaits: catalogue.DeliverySeat}} if err := open.inventory.SavePlan(ctx, &waiting); err != nil { t.Fatal(err) diff --git a/cmd/mesh-controller/beside_test.go b/cmd/mesh-controller/beside_test.go new file mode 100644 index 00000000..a4c0237d --- /dev/null +++ b/cmd/mesh-controller/beside_test.go @@ -0,0 +1,106 @@ +package main + +import ( + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// An unrelated closed batch is cut into a walk beside an open one, and that walk asks its first tier at once +// (issue 973, novox/hq ADR 0407 decision 4): a hotfix is never held by an unrelated delivery. +func TestAnUnrelatedBatchWalksBesideAnOpenWalk(t *testing.T) { + open := windowed(t) + asksWithPaths(t) + ctx := t.Context() + hear(t, open, repoMerge("one", "c1", t0), t0) + cutAt(t, open, t0.Add(90*time.Second)) + if ws, _ := walks(t, open); len(ws) != 1 || !ws[0].Open() { + t.Fatalf("the first walk is not open: %+v", ws) + } + heard := t0.Add(100 * time.Second) + hear(t, open, lone(repoMerge("two", "c2", heard)), heard) + advanceHeld(ctx, open) + ws, bs := walks(t, open) + if len(ws) != 2 || len(bs) != 0 { + t.Fatalf("%d walk(s) and %d batch(es); want the unrelated batch cut beside the open walk", len(ws), len(bs)) + } + for _, w := range ws { + if !w.Open() || w.Waiting() || deferred(w) || !askedAny(w) { + t.Errorf("%s (%s) is not walking: open %v, deferred %v, asked %v, note %q", w.ID, w.Named(), w.Open(), + deferred(w), askedAny(w), w.Note) + } + } +} + +// A closed batch related to an open walk is queued behind it with the reason, while one that shares nothing is +// cut beside it; the walk that started is never superseded (ADR 0276 decision 4, issue 973). +func TestARelatedBatchWaitsBehindTheOpenWalk(t *testing.T) { + open := windowed(t) + asksWithPaths(t) + ctx := t.Context() + hear(t, open, repoMerge("one", "c1", t0), t0) + cutAt(t, open, t0.Add(90*time.Second)) + first := func() inventory.Plan { ws, _ := walks(t, open); return ws[len(ws)-1] }() + heard := t0.Add(100 * time.Second) + hear(t, open, lone(repoMerge("one", "c2", heard)), heard) + hear(t, open, lone(repoMerge("three", "c3", heard.Add(time.Second))), heard.Add(time.Second)) + ws, bs := walks(t, open) + if len(ws) != 2 || len(bs) != 1 { + t.Fatalf("%d walk(s) and %d batch(es); want the unrelated merge walking and the related one queued", len(ws), len(bs)) + } + b := bs[0] + words := batchWords(b, heard) + if b.State != inventory.PlanQueued || b.Delivery.Batch.Behind != first.ID || b.CommitOf("novox/one") != "c2" || + !strings.Contains(words, "queued behind "+first.ID) || !strings.Contains(words, "both change novox/one") { + t.Fatalf("the related batch is %s, behind %q, reading %q", b.State, b.Delivery.Batch.Behind, words) + } + if got, _ := open.inventory.PlanByID(ctx, first.ID); !got.Open() { + t.Fatalf("the started walk was %s by the related merge", got.State) + } + first.State = inventory.PlanDone + if err := open.inventory.SavePlan(ctx, &first); err != nil { + t.Fatal(err) + } + cutAt(t, open, heard.Add(2*time.Second)) + if _, bs := walks(t, open); len(bs) != 0 { + t.Fatalf("the queued batch was not cut once the walk it waited for ended: %+v", bs) + } +} + +// A batch on the controller's own path never walks beside another walk, and nothing walks beside it, whatever +// the two share (issue 973): it replaces the controller, so it goes alone. +func TestAnOwnPathBatchNeverWalksBeside(t *testing.T) { + open := windowed(t) + asksWithPaths(t) + ctx := t.Context() + hear(t, open, repoMerge("one", "c1", t0), t0) + cutAt(t, open, t0.Add(90*time.Second)) + ws, _ := walks(t, open) + first := ws[0] + heard := t0.Add(100 * time.Second) + hear(t, open, lone(repoMerge("mesh-controller", "k1", heard)), heard) + ws, bs := walks(t, open) + if len(ws) != 1 || len(bs) != 1 || !bs[0].OwnPath() || bs[0].State != inventory.PlanQueued || + bs[0].Delivery.Batch.Behind != first.ID || !strings.Contains(batchWords(bs[0], heard), "own path") { + t.Fatalf("an own-path batch beside an open walk: %d walk(s), batches %+v", len(ws), bs) + } + first.State = inventory.PlanDone + if err := open.inventory.SavePlan(ctx, &first); err != nil { + t.Fatal(err) + } + cutAt(t, open, heard.Add(time.Second)) + ws, _ = walks(t, open) + if len(ws) != 2 || !ws[0].Open() || ws[0].CommitOf("novox/mesh-controller") != "k1" { + t.Fatalf("the own-path batch was not cut once the walk ended: %+v", ws) + } + own := ws[0] + // Unrelated to the own-path walk by every footprint, a merge still waits for it. + later := heard.Add(time.Minute) + hear(t, open, lone(repoMerge("two", "c2", later)), later) + ws, bs = walks(t, open) + if len(ws) != 2 || len(bs) != 1 || bs[0].State != inventory.PlanQueued || bs[0].Delivery.Batch.Behind != own.ID { + t.Fatalf("a merge beside the own-path walk: %d walk(s), batches %+v", len(ws), bs) + } +} diff --git a/cmd/mesh-controller/lone_merge_test.go b/cmd/mesh-controller/lone_merge_test.go index 7ab1dbc3..344c12a5 100644 --- a/cmd/mesh-controller/lone_merge_test.go +++ b/cmd/mesh-controller/lone_merge_test.go @@ -227,8 +227,9 @@ func TestAMergeRelatedToTwoBatchesFoldsThem(t *testing.T) { } } -// With a walk open, a batch cut at once still waits for it, said queued behind it (walks stay one at a time, ADR -// 0276 decision 4); it is cut once that walk ended, with no window to wait for. +// With a walk open that it is related to (the same repository, issue 973), a batch cut at once still waits for +// it, said queued behind it (ADR 0276 decision 4, ADR 0407 decision 4); it is cut once that walk ended, with no +// window to wait for. func TestACutAtOnceBatchQueuesBehindAnOpenWalk(t *testing.T) { open := windowed(t) groupsWaiting(t, "pr/c1") @@ -241,7 +242,7 @@ func TestACutAtOnceBatchQueuesBehindAnOpenWalk(t *testing.T) { } first := ws[0] heard := t0.Add(100 * time.Second) - hear(t, open, lone(repoMerge("two", "c2", heard)), heard) + hear(t, open, lone(repoMerge("one", "c2", heard)), heard) ws, bs := walks(t, open) if len(ws) != 1 || len(bs) != 1 || bs[0].State != inventory.PlanQueued || bs[0].Delivery.Batch.Behind != first.ID || !bs[0].Delivery.Batch.CutAtOnce { @@ -280,7 +281,8 @@ func TestAStopWalksTheOthersAgainInABatchOfTheirOwnWhenTheyShareNothing(t *testi cutAt(t, open, t0.Add(2*time.Minute)) ws, _ := walks(t, open) walk := ws[0] - hear(t, open, repoMerge("two", "two1cccccccc", t0.Add(3*time.Minute)), t0.Add(3*time.Minute)) + // Related to the walk by its repository (queued behind it), and to nothing the stop walks again. + hear(t, open, catalogueMerge("cat2cccccccc", "notes", t0.Add(3*time.Minute)), t0.Add(3*time.Minute)) _, bs := walks(t, open) if len(bs) != 1 { t.Fatalf("the merge heard while the walk ran is in %d batches", len(bs)) diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index 4b158c7e..dd6b2d1b 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -119,6 +119,8 @@ type PlanBatch struct { ClosesAt time.Time `json:"closes_at"` AtMost time.Time `json:"at_most"` Behind string `json:"behind,omitempty"` + // BehindWhy says why the batch waits behind that walk: what the two share (issue 973). + BehindWhy string `json:"behind_why,omitempty"` // Own says the batch holds merges on the controller's own path, whose walk waits for nobody's word: such // a merge never shares a batch with one that waits for mesh-delivery's (ADR 0276, decided during the // build, 2026-10-10). diff --git a/testdata/store-tests b/testdata/store-tests index 2bdde702..4c2f6a7e 100644 --- a/testdata/store-tests +++ b/testdata/store-tests @@ -128,3 +128,6 @@ ./cmd/mesh-controller TestAGroupsCompositionIntoAReleaseBranchIsJudgedByItsOwnController ./cmd/mesh-controller TestAGroupsCompositionJudgedWithTheValidatorIntoAReleaseBranchIsJudgedByItsOwnController ./cmd/mesh-controller TestAssigningTheClientIssuesItsAccount +./cmd/mesh-controller TestAnUnrelatedBatchWalksBesideAnOpenWalk +./cmd/mesh-controller TestARelatedBatchWaitsBehindTheOpenWalk +./cmd/mesh-controller TestAnOwnPathBatchNeverWalksBeside -- 2.54.0 From 6355176fdab00ebe8d82793f5619740212dbf244 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 17 Oct 2026 01:00:40 +0200 Subject: [PATCH 4/4] The ssh-client test offers the forge from another node, as the catalogue's ssh-client now requires forge-account (issue 985) --- internal/catalogue/ssh_client_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/catalogue/ssh_client_test.go b/internal/catalogue/ssh_client_test.go index 06ef03ca..7ed6b215 100644 --- a/internal/catalogue/ssh_client_test.go +++ b/internal/catalogue/ssh_client_test.go @@ -11,9 +11,16 @@ import ( // whole of ~/.ssh/config.d/00-mesh, every other node with its account. ~/.ssh and ~/.ssh/config.d // are created 0700 and owned by the account; the operator's own config below the region is kept. func TestSSHClientOwnsTheOperatorsSSHConfig(t *testing.T) { - shelf := shelf(catalogueManifest(t, "ssh-client")) + shelf := shelf(catalogueManifest(t, "ssh-client"), catalogueManifest(t, "gitea")) + // ssh-client requires forge-account since mesh-catalog #340 (novox/hq issue 955), answered by the forge's + // holder on another node; this test is about the operator's config, so the forge is offered from there + // (issue 985). + forge := World{Offered: map[string][]Provider{ + "forge-account": {{Node: "marge", At: "marge.internal", Module: "gitea", Serves: map[string]any{ + "ssh_host": "marge.internal", "ssh_port": 222, "known_host": "[marge.internal]:222 ssh-ed25519 AAAA", + "ssh_base": "ssh://git@marge.internal:222/", "https_base": "https://git.example.org/"}}}}} got, err := Resolve(shelf, []string{"ssh-client"}, - Node{Name: "homer", At: "homer.internal", Account: "jo", AccountHome: "/home/jo"}, World{}) + Node{Name: "homer", At: "homer.internal", Account: "jo", AccountHome: "/home/jo"}, forge) if err != nil { t.Fatal(err) } -- 2.54.0