From f7ba413c1026a47a83877e409414e10c93437442 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 03:41:52 +0200 Subject: [PATCH] mesh-delivery: a member on its carrier's walk takes its turn, and a waiting member says for whom Under ADR 0276 the merge that carries a superseded member is often folded into the next member's own walk, which waits for that member's turn: waiting there waited for itself until the carrier's bound. The members on that walk now take their turn, as members sharing one walk do. A member that waits says which merge it waits for and until when, and `stalled` uses that bound; a carrier still in a batch is bounded by the window and one walk; a member every module of which was carried is reached, never stopped with no reason (novox/hq issue 445, review of #203). --- .../mesh-delivery/cmd/mesh-delivery/holder.go | 165 +++++-- .../cmd/mesh-delivery/issue445_test.go | 414 +++++++++++++----- .../mesh-delivery/cmd/mesh-delivery/model.go | 5 + .../mesh-delivery/cmd/mesh-delivery/table.go | 7 + 4 files changed, 444 insertions(+), 147 deletions(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 20c37bcb..b75f537e 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -1397,6 +1397,25 @@ func (h *Holder) inOrder(g *Group, members []*Delivery) []Member { // walk carried it, holds the members after it while a later merge can still carry it, and stops them, saying // why, only when none can (novox/hq issue 445). func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { + waits := map[string]carryWait{} + asks := h.turnsOf(g, members, waits) + // What each member's turn waits on a carrier for is kept on it, and cleared once it no longer does, so `show` + // and `stalled` name the carrier and until when (ADR 0276 decision 9, novox/hq issue 445). + for _, d := range members { + w := waits[d.ID] + if d.State != Published || !d.LetGo.IsZero() { + w = carryWait{} // only a member still waiting for its turn waits for a carrier + } + if d.TurnAwaits != w.said || !d.TurnAwaitsUntil.Equal(w.until) { + d.TurnAwaits, d.TurnAwaitsUntil = w.said, w.until + h.keep(d) + } + } + return asks +} + +// turnsOf is groupTurns' walk through the order; waits receives, by member, the carrier its turn waits for. +func (h *Holder) turnsOf(g *Group, members []*Delivery, waits map[string]carryWait) []func() { ordered := h.inOrder(g, members) byID := map[string]*Delivery{} for _, d := range members { @@ -1426,15 +1445,32 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { // merge into its trunk that can carry it is open, within that merge's own bound, the members after it // wait, as for a member still delivering (ADR 0239 decision 4: the next starts when the one before is // delivered). Only when none can carry it are they stopped, saying why (novox/hq issue 445). - awaited, why := h.carryAwaited(d, h.Now()) - if awaited != "" { + w := h.carryAwaited(d, h.Now()) + if w.covered() { + continue // every module delivered by a later merge the forge says contains it: reached + } + if w.why != "" { + for _, later := range ordered[i+1:] { + if a := h.stopInGroup(byID[later.ID], fmt.Sprintf("its group %s stopped: %s superseded, and %s", g.ID, + d.ID, w.why)); a != nil { + asks = append(asks, a) + } + } return asks } + w.said = fmt.Sprintf("its turn in group %s waits for %s, superseded, to be carried by %s, until %s", g.ID, + d.ID, w.said, w.until.UTC().Format(time.RFC3339)) for _, later := range ordered[i+1:] { - if a := h.stopInGroup(byID[later.ID], fmt.Sprintf("its group %s stopped: %s superseded, and %s", g.ID, d.ID, - why)); a != nil { - asks = append(asks, a) - } + waits[later.ID] = w + } + // Under ADR 0276 a member is superseded when its waiting walk was folded into a successor that does not + // answer its merge, and the member after it was folded into the same successor: the carrier is then in + // the next member's own walk, which waits for that member's turn. Waiting there would wait for itself + // until the carrier's bound. That walk delivers both, in the group's order among its tiers (decision 5), + // so the members on it take their turn as members sharing one walk do; a member on another walk waits. + if shared := w.waitingWalk(); shared != "" && sharing == "" { + sharing = shared + continue } return asks case d.State == Failed || d.State == Stopped: @@ -1475,6 +1511,7 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { } if a := h.turn(d, fmt.Sprintf("its turn in group %s (%d of %d)", g.ID, i+1, len(ordered))); a != nil { asks, sharing = append(asks, a), d.Walk.ID + delete(waits, d.ID) continue } return asks @@ -1491,45 +1528,96 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() { // 0270), and without this bound the members after it would wait on that answer for ever (novox/hq issue 445). const containsGrace = 30 * time.Minute -// carryAwaited says which later merges into a superseded member's trunk may still carry every module it moves — -// the cover GroupState counts as reached (ADR 0270's covered test, novox/hq issue 442) — or, when none can, why. -// A later merge of the same repository into the same trunk on a passing check, whose plan moves the module and -// whose merge commit the forge has not said leaves this one out, can carry it: while it is merged and not yet -// ended, not held for a person, and within its own state's bound (ADR 0239 decision 9, ADR 0276 decision 9: every -// wait has a bound); once delivered, while the forge has not yet said whether it contains this merge, within -// containsGrace. So the wait is never longer than the carrier's own bound; a carrier superseded in turn hands the -// wait to the later merge that took it over, which is a carrier of its own (novox/hq issue 445). -func (h *Holder) carryAwaited(d *Delivery, now time.Time) (string, string) { - if d.MergedAs == "" || d.MergedAt.IsZero() || d.Plan == nil || len(d.Plan.Moved) == 0 { - return "", "nothing records which modules it moves, so no later merge can carry it" +// readyCarrierBound is how long a merged carrier with no walk yet may be awaited from its merge. Its state, ready, +// has no bound of its own: it waits in a batch, which closes at the latest ten minutes after its first merge +// (ADR 0276 decision 1, `merge-window-at-most`), and then behind at most the one open walk, within that walk's +// bound (decisions 4 and 9). A merge no walk answers is held after `unwalked`, and a held one is no carrier. +const readyCarrierBound = 10*time.Minute + 2*time.Hour + +// carryWait is what a superseded member's group waits for: the later merges that may still carry it, said, and +// the moment the wait ends at the latest; or why none can carry it. Neither means every module it moves was +// delivered by a later merge the forge says contains it: covered. +type carryWait struct { + by []*Delivery + said string + until time.Time + why string +} + +func (w carryWait) covered() bool { return len(w.by) == 0 && w.why == "" } + +// waitingWalk is the walk of an awaited carrier that has not started: one waiting for this owner's word. +func (w carryWait) waitingWalk() string { + for _, c := range w.by { + if !c.State.Final() && c.Walk != nil && c.Walk.Waits && !c.Walk.Started() { + return c.Walk.ID + } } - var awaited []string + return "" +} + +// carryAwaited says which later merges into a superseded member's trunk may still carry every module it moves — +// the cover GroupState counts as reached (ADR 0270's covered test, novox/hq issue 442) — and until when, or, +// when none can, why. A later merge of the same repository into the same trunk on a passing check, whose plan +// moves the module and whose merge commit the forge has not said leaves this one out, can carry it while it is +// merged and not yet ended, not held for a person, and within its bound (ADR 0239 decision 9, ADR 0276 decision +// 9: every wait has a bound): its state's, or readyCarrierBound from its merge while no walk carries it yet; once +// delivered, while the forge has not yet said whether it contains this merge, within containsGrace. A carrier +// superseded in turn ends; the later merge that took it over is a carrier of its own (novox/hq issue 445). +func (h *Holder) carryAwaited(d *Delivery, now time.Time) carryWait { + if d.MergedAs == "" || d.MergedAt.IsZero() || d.Plan == nil || len(d.Plan.Moved) == 0 { + return carryWait{why: "nothing records which modules it moves, so no later merge can carry it"} + } + var w carryWait + seen := map[string]bool{} +modules: for _, module := range d.Plan.Moved { found := false + var latest time.Time // the last moment a carrier of this module may still be awaited var not []string for _, c := range h.deliveries { if !mayCarry(c, d, module) { continue } contained, known := d.ContainedIn[c.MergedAs] + var until time.Time switch { case known && !contained: not = append(not, c.ID+" does not contain its merge") + continue case c.State == Delivered && carries(c, d, module) && known: - found = true // covered for this module; another module is what is still awaited - case c.State == Delivered && carries(c, d, module) && now.Sub(c.Since) <= containsGrace: - found = true - awaited = append(awaited, c.ID+" (delivered; whether it contains its merge is asked of the forge)") + continue modules // delivered and contained: this module is carried case c.State == Delivered && carries(c, d, module): - not = append(not, fmt.Sprintf("the forge did not say within %s whether %s contains its merge", containsGrace, c.ID)) + until = c.Since.Add(containsGrace) + if now.After(until) { + not = append(not, fmt.Sprintf("the forge did not say within %s whether %s contains its merge", containsGrace, c.ID)) + continue + } case c.State.Final() || c.State == Held: - // ended without carrying it, or held for a person: no carrier to wait for - case pastBound(c, now): - b, _ := boundOf(c) - not = append(not, fmt.Sprintf("%s is past its bound (%s %s)", c.ID, c.State, b.For)) + continue // ended without carrying it, or held for a person: no carrier to wait for + case c.State == Ready: + until = c.MergedAt.Add(readyCarrierBound) + if now.After(until) { + not = append(not, fmt.Sprintf("%s is merged and no walk carried it within %s", c.ID, readyCarrierBound)) + continue + } default: - found = true - awaited = append(awaited, fmt.Sprintf("%s (%s)", c.ID, c.State)) + b, bounded := boundOf(c) + if pastBound(c, now) { + not = append(not, fmt.Sprintf("%s is past its bound (%s %s)", c.ID, c.State, b.For)) + continue + } + if until = now.Add(Bounds[Delivering].For); bounded && b.For > 0 { + until = boundSince(c).Add(b.For) + } + } + found = true + if until.After(latest) { + latest = until + } + if !seen[c.ID] { + seen[c.ID] = true + w.by = append(w.by, c) } } if !found { @@ -1538,11 +1626,24 @@ func (h *Holder) carryAwaited(d *Delivery, now time.Time) (string, string) { sort.Strings(not) why += ": " + strings.Join(not, "; ") } - return "", why + return carryWait{why: why} + } + // The wait ends when the first module runs out of carriers, whatever the others still have. + if w.until.IsZero() || latest.Before(w.until) { + w.until = latest } } - sort.Strings(awaited) - return strings.Join(slices.Compact(awaited), ", "), "" + sort.Slice(w.by, func(i, j int) bool { return w.by[i].ID < w.by[j].ID }) + var said []string + for _, c := range w.by { + s := fmt.Sprintf("%s (%s", c.ID, c.State) + if c.Walk != nil { + s += ", walk " + c.Walk.ID + } + said = append(said, s+")") + } + w.said = strings.Join(said, ", ") + return w } // mayCarry is whether c is a later merge that could carry module for d, by what is known before its walk ends: of diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue445_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue445_test.go index 3784fefe..7215ca0f 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue445_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue445_test.go @@ -8,154 +8,338 @@ import ( // novox/hq issue 445: in a delivery group ordered [A, B], A's walk was superseded by the walk of a later merge C // of A's repository that does not answer A's merge. groupTurns stopped B at once, although C's walk could still -// carry A's modules and A then counts as reached (ADR 0270 decision 2, issue 442). While a later walk that can -// carry A is open, B waits; once that walk is delivered and covers A, B takes its turn; when no later merge can -// carry A, or the one that could is past its own bound, B is stopped, saying why. +// carry A's modules and A then counts as reached (ADR 0270 decision 2, issue 442). While a later merge that can +// carry A is open, B waits, saying for which merge and until when; once that merge's walk is delivered and covers +// A, B takes its turn; when C's walk is B's own (ADR 0276 folds both into one successor), B takes its turn on it; +// when no later merge can carry A, or the one that could is past its own bound, B is stopped, saying why. // // The shape is the live group fix/assign-says-why-a-module-is-not-there as mesh-delivery showed it on // 2026-10-11 (issue 442's test): the controller's member superseded first in the order, the lab's after it. -func TestAMemberAfterACarriableSupersededOneWaitsForItsCarrier(t *testing.T) { - const ctl, lab, group = "novox/mesh-controller", "novox/mesh-lab", "fix/assign-says-why-a-module-is-not-there" - at := time.Date(2026, 10, 7, 19, 4, 10, 0, time.UTC) - modules := []string{"build-agent", "mesh-controller", "route-proxy"} - const mergedA, mergedB, mergedC = "a5f53ef9ebdf3650", "77777777aaaaaaaa", "3f68a49500000000" - aID, bID, cID := IDOf(ctl, "8adb7f1a051941e9"), IDOf(lab, "33e63ee5b47a7552"), IDOf(ctl, "3f68a495c0c0c0c0") - // A world in which A's walk was superseded by C's, C's walk runs, and B waits for its turn. - open := func(t *testing.T) *world { - a := &Delivery{ID: aID, Repository: ctl, Commit: "8adb7f1a051941e9", Number: 109, Base: "main", Branch: group, - Group: group, State: Superseded, Since: at.Add(4 * time.Minute), Created: at.Add(-time.Hour), MergedAs: mergedA, - MergedAt: at, Plan: planMoving(ctl, modules...), Check: &Verdict{Gate: "pass"}, - Walk: &WalkSeen{ID: "plan-a5f53e", Commit: mergedA, State: walkSuperseded, Branch: "main", - Note: "superseded at tier 0 by plan-later (" + ctl + " at 3f68a495); planned there again"}, - Transitions: []Transition{{At: at.Add(4 * time.Minute), From: Delivering, To: Superseded, Event: EvSuperseded, - Why: "a newer delivery to the same trunk took over its walk"}}} - b := &Delivery{ID: bID, Repository: lab, Commit: "33e63ee5b47a7552", Number: 117, Base: "main", Branch: group, - Group: group, State: Published, Since: at.Add(3 * time.Minute), Created: at.Add(-2 * time.Hour), - MergedAs: mergedB, MergedAt: at.Add(2 * time.Minute), Plan: planMoving(lab, "mesh-lab"), - Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-lab", Commit: mergedB, State: walkBuilding, - Waits: true, WaitedFor: "mesh-delivery", Branch: "main", Created: at.Add(3 * time.Minute)}} - c := &Delivery{ID: cID, Repository: ctl, Commit: "3f68a495c0c0c0c0", Number: 110, Base: "main", - Branch: "fix/later", State: Delivering, Since: at.Add(5 * time.Minute), Created: at.Add(-time.Hour), - MergedAs: mergedC, MergedAt: at.Add(time.Minute), Plan: planMoving(ctl, modules...), - Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-later", Commit: mergedC, State: walkRolling, - WaitedFor: "mesh-delivery", GoBy: byOwner, Branch: "main", Created: at.Add(4 * time.Minute)}} - store := newMemStore() - for _, d := range []*Delivery{a, b, c} { - if err := store.PutDelivery(d); err != nil { - t.Fatal(err) +const ( + g445 = "fix/assign-says-why-a-module-is-not-there" + ctl445, lab445 = "novox/mesh-controller", "novox/mesh-lab" + mergedA, mergedB, mergedC = "a5f53ef9ebdf3650", "77777777aaaaaaaa", "3f68a49500000000" + mergedD = "4d4d4d4d00000000" + headA, headB, headC, headD = "8adb7f1a051941e9", "33e63ee5b47a7552", "3f68a495c0c0c0c0", "4d4d4d4dc0c0c0c0" +) + +var ( + at445 = time.Date(2026, 10, 7, 19, 4, 10, 0, time.UTC) + modules445 = []string{"build-agent", "mesh-controller", "route-proxy"} + aID, bID, cID, dID445 = IDOf(ctl445, headA), IDOf(lab445, headB), IDOf(ctl445, headC), IDOf(ctl445, headD) +) + +// shape445 is A superseded, B published on its walk, and C, the later merge of A's repository, as each case sets it. +type shape445 struct { + a, b, c *Delivery + extra []*Delivery + walks []Walk +} + +func newShape445() *shape445 { + at := at445 + s := &shape445{} + s.a = &Delivery{ID: aID, Repository: ctl445, Commit: headA, Number: 109, Base: "main", Branch: g445, Group: g445, + State: Superseded, Since: at.Add(4 * time.Minute), Created: at.Add(-time.Hour), MergedAs: mergedA, MergedAt: at, + Plan: planMoving(ctl445, modules445...), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-a5f53e", Commit: mergedA, State: walkSuperseded, Branch: "main", + Note: "superseded at tier 0 by plan-later (" + ctl445 + " at 3f68a495); planned there again"}, + Transitions: []Transition{{At: at.Add(4 * time.Minute), From: Published, To: Superseded, Event: EvSuperseded, + Why: "a newer delivery to the same trunk took over its walk"}}} + s.b = &Delivery{ID: bID, Repository: lab445, Commit: headB, Number: 117, Base: "main", Branch: g445, Group: g445, + State: Published, Since: at.Add(3 * time.Minute), Created: at.Add(-2 * time.Hour), MergedAs: mergedB, + MergedAt: at.Add(2 * time.Minute), Plan: planMoving(lab445, "mesh-lab"), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-lab", Commit: mergedB, State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery", + Branch: "main", Created: at.Add(3 * time.Minute)}} + s.c = &Delivery{ID: cID, Repository: ctl445, Commit: headC, Number: 110, Base: "main", Branch: "fix/later", + State: Delivering, Since: at.Add(5 * time.Minute), Created: at.Add(-time.Hour), MergedAs: mergedC, + MergedAt: at.Add(time.Minute), Plan: planMoving(ctl445, modules445...), Check: &Verdict{Gate: "pass"}, + Walk: &WalkSeen{ID: "plan-later", Commit: mergedC, State: walkRolling, WaitedFor: "mesh-delivery", GoBy: byOwner, + Branch: "main", Created: at.Add(4 * time.Minute)}} + went := at.Add(5 * time.Minute) + s.walks = []Walk{ + {ID: "plan-a5f53e", Repository: ctl445, Branch: "main", Commit: mergedA, Created: at.Add(30 * time.Second), + State: walkSuperseded, Note: s.a.Walk.Note, Modules: map[string]*WalkModule{}}, + {ID: "plan-lab", Repository: lab445, Branch: "main", Commit: mergedB, Created: at.Add(3 * time.Minute), + State: walkBuilding, Tiers: [][]string{{"mesh-lab"}}, Modules: map[string]*WalkModule{"mesh-lab": {}}, + Delivery: &WalkDelivery{Awaits: "mesh-delivery"}}, + ctlWalk("plan-later", mergedC, at.Add(4*time.Minute), walkRolling, &WalkDelivery{Awaits: "mesh-delivery", + Go: &went, By: byOwner}), + } + return s +} + +// ctlWalk is a walk of the controller's repository at a merge commit, moving A's modules. +func ctlWalk(id, commit string, created time.Time, state string, wd *WalkDelivery) Walk { + w := Walk{ID: id, Repository: ctl445, Branch: "main", Commit: commit, Created: created, State: state, + Tiers: [][]string{modules445}, Modules: map[string]*WalkModule{}, Delivery: wd} + for _, m := range modules445 { + w.Modules[m] = &WalkModule{} + } + return w +} + +// open is the world over the shape, read back and ticked ten minutes after A's merge. +func (s *shape445) open(t *testing.T) *world { + t.Helper() + store := newMemStore() + for _, d := range append([]*Delivery{s.a, s.b, s.c}, s.extra...) { + if d == nil { + continue + } + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + g := &Group{ID: g445, Members: []string{aID, bID}, Closed: true, Order: []string{aID, bID}, Said: "delivering", + OrderOf: map[string]string{}, Check: &GroupCheck{Asked: "check-1", At: at445.Add(-time.Hour), Verdict: "pass", + Summary: "both compose", Commits: commitsOf([]*Delivery{s.a, s.b})}} + if err := store.PutGroup(g); err != nil { + t.Fatal(err) + } + ctlr := newFakeController() + for _, w := range s.walks { + ctlr.put(w) + } + w := newWorldOver(t, store, ctlr, newFakeForge(), at445.Add(10*time.Minute)) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + return w +} + +// end ends a walk in the state given, every module sent to every machine running it. +func end445(w *world, walk, state string) { + r := w.ctl.walk(walk) + sent := w.now + for m := range r.Modules { + r.Modules[m] = &WalkModule{State: "built", SentAt: &sent} + } + r.State = state + w.h.WalkMoved(w.ctl.put(r)) + w.settleAll() +} + +func waits445(t *testing.T, w *world, on string) { + t.Helper() + d := w.delivery(bID) + if d.State != Published || containsString(w.ctl.delivers, "plan-lab") { + t.Fatalf("B is %s, let go %v; want it published and waiting while %s can still carry A", d.State, w.ctl.delivers, on) + } + if !strings.Contains(d.TurnAwaits, on) || !strings.Contains(d.TurnAwaits, aID) || d.TurnAwaitsUntil.IsZero() { + t.Fatalf("B says it waits for %q until %v; want it to name A and %s", d.TurnAwaits, d.TurnAwaitsUntil, on) + } + g := w.h.groups[g445] + if state, why := GroupState(g, w.h.membersOf(g)); state != "delivering" { + t.Fatalf("the group reads %s (%s), want delivering", state, why) + } +} + +func stopped445(t *testing.T, w *world, says string) { + t.Helper() + d := w.delivery(bID) + if d.State != Stopped || len(d.Transitions) == 0 { + t.Fatalf("B is %s, want stopped", d.State) + } + why := d.Transitions[len(d.Transitions)-1].Why + if !strings.Contains(why, aID) || !strings.Contains(why, says) { + t.Fatalf("B was stopped saying %q, want it to name %s and say %q", why, aID, says) + } + if containsString(w.ctl.delivers, "plan-lab") || d.TurnAwaits != "" { + t.Fatalf("B's walk was let go %v, or B still says it waits: %q", w.ctl.delivers, d.TurnAwaits) + } +} + +func letGo445(t *testing.T, w *world, walk string) { + t.Helper() + if d := w.delivery(bID); d.State != Delivering || !containsString(w.ctl.delivers, walk) || d.TurnAwaits != "" { + t.Fatalf("B is %s, let go %v, waiting for %q; want its turn taken on %s", d.State, w.ctl.delivers, d.TurnAwaits, walk) + } +} + +// C's walk runs on its own: B waits for it, says so, and goes on or stops by what comes of it. +func TestAMemberAfterACarriableSupersededOneWaitsForItsCarrier(t *testing.T) { + t.Run("a later walk that can carry A is open: B waits, naming it", func(t *testing.T) { + w := newShape445().open(t) + waits445(t, w, cID) + // Past a published delivery's own 30 minutes, B is not stalled for "its turn or its word": its bound is + // C's, and what `stalled` would say names C. + w.later(Bounds[Published].For + 5*time.Minute) + w.settleAll() + waits445(t, w, cID) + for _, l := range w.h.Stalled() { + if l.ID == bID { + t.Fatalf("B is listed stalled while C's walk is within its bound: %+v", l) } } - g := &Group{ID: group, Members: []string{aID, bID}, Closed: true, Order: []string{aID, bID}, - Said: "delivering", OrderOf: map[string]string{}, - Check: &GroupCheck{Asked: "check-1", At: at.Add(-time.Hour), Verdict: "pass", Summary: "both compose", - Commits: commitsOf([]*Delivery{a, b})}} - if err := store.PutGroup(g); err != nil { - t.Fatal(err) + if b, _ := boundOf(w.delivery(bID)); !strings.Contains(b.Says, cID) || !strings.Contains(b.Says, "until") { + t.Fatalf("B's bound says %q", b.Says) } - ctlr := newFakeController() - ctlr.put(Walk{ID: "plan-a5f53e", Repository: ctl, Branch: "main", Commit: mergedA, Created: at.Add(30 * time.Second), - State: walkSuperseded, Note: a.Walk.Note, Modules: map[string]*WalkModule{}}) - ctlr.put(Walk{ID: "plan-lab", Repository: lab, Branch: "main", Commit: mergedB, Created: at.Add(3 * time.Minute), - State: walkBuilding, Tiers: [][]string{{"mesh-lab"}}, Modules: map[string]*WalkModule{"mesh-lab": {}}, - Delivery: &WalkDelivery{Awaits: "mesh-delivery"}}) - went := at.Add(5 * time.Minute) - later := Walk{ID: "plan-later", Repository: ctl, Branch: "main", Commit: mergedC, Created: at.Add(4 * time.Minute), - State: walkRolling, Tiers: [][]string{modules}, Modules: map[string]*WalkModule{}, - Delivery: &WalkDelivery{Awaits: "mesh-delivery", Go: &went, By: byOwner}} - for _, m := range modules { - later.Modules[m] = &WalkModule{} - } - ctlr.put(later) - w := newWorldOver(t, store, ctlr, newFakeForge(), at.Add(10*time.Minute)) - if err := w.h.Reconcile(); err != nil { - t.Fatal(err) - } - w.settleAll() - return w - } - // end ends C's walk in the state given, every module sent to every machine running it. - end := func(w *world, state string) { - walk := w.ctl.walk("plan-later") - sent := w.now - for _, m := range modules { - walk.Modules[m] = &WalkModule{State: "built", SentAt: &sent} - } - walk.State = state - w.h.WalkMoved(w.ctl.put(walk)) - w.settleAll() - } - waits := func(t *testing.T, w *world) { - t.Helper() - if s := w.state(bID); s != Published || containsString(w.ctl.delivers, "plan-lab") { - t.Fatalf("B is %s, let go %v; want it published and waiting while C's walk can still carry A", s, w.ctl.delivers) - } - g := w.h.groups[group] - if state, why := GroupState(g, w.h.membersOf(g)); state != "delivering" { - t.Fatalf("the group reads %s (%s), want delivering", state, why) - } - } - stopped := func(t *testing.T, w *world, says string) { - t.Helper() - d := w.delivery(bID) - if d.State != Stopped || len(d.Transitions) == 0 { - t.Fatalf("B is %s, want stopped", d.State) - } - why := d.Transitions[len(d.Transitions)-1].Why - if !strings.Contains(why, aID) || !strings.Contains(why, says) { - t.Fatalf("B was stopped saying %q, want it to name %s and say %q", why, aID, says) - } - if containsString(w.ctl.delivers, "plan-lab") { - t.Fatalf("B's walk was let go %v", w.ctl.delivers) - } - } - - t.Run("a later walk that can carry A is open: B waits", func(t *testing.T) { - waits(t, open(t)) }) t.Run("that walk delivered and covers A: B takes its turn", func(t *testing.T) { - w := open(t) + w := newShape445().open(t) w.forge.contains[mergedA+" in "+mergedC] = true - end(w, walkDone) + end445(w, "plan-later", walkDone) if s := w.state(cID); s != Delivered { t.Fatalf("C is %s", s) } if d := w.delivery(aID); d.State != Superseded || d.CoveredBy == "" { t.Fatalf("A is %s, covered by %q", d.State, d.CoveredBy) } - if s := w.state(bID); s != Delivering || !containsString(w.ctl.delivers, "plan-lab") { - t.Fatalf("B is %s, let go %v; want its turn taken", s, w.ctl.delivers) - } + letGo445(t, w, "plan-lab") }) t.Run("the forge says the later merge does not contain A: B is stopped", func(t *testing.T) { - w := open(t) + w := newShape445().open(t) w.forge.contains[mergedA+" in "+mergedC] = false - end(w, walkDone) - stopped(t, w, "can carry") + end445(w, "plan-later", walkDone) + stopped445(t, w, "can carry") }) t.Run("the later walk failed: B is stopped", func(t *testing.T) { - w := open(t) - end(w, walkFailed) + w := newShape445().open(t) + end445(w, "plan-later", walkFailed) if s := w.state(cID); s != Failed { t.Fatalf("C is %s", s) } - stopped(t, w, "can carry") + stopped445(t, w, "can carry") }) t.Run("the later walk stays open past its own bound: B is stopped", func(t *testing.T) { - w := open(t) + w := newShape445().open(t) w.later(Bounds[Delivering].For - 10*time.Minute) w.settleAll() - waits(t, w) + waits445(t, w, cID) w.later(10 * time.Minute) w.settleAll() - stopped(t, w, "past its bound") + stopped445(t, w, "past its bound") }) t.Run("the forge never says whether the delivered walk contains A: B is stopped after the grace", func(t *testing.T) { - w := open(t) - end(w, walkDone) - waits(t, w) + w := newShape445().open(t) + end445(w, "plan-later", walkDone) + waits445(t, w, cID) w.later(containsGrace + time.Minute) w.settleAll() - stopped(t, w, "did not say") + stopped445(t, w, "did not say") }) } + +// The review's shape of #203 (ADR 0276): A's waiting walk and B's were folded into one successor, plan-2, that +// answers C's merge and B's and not A's. The carrier is in B's own walk, which waits for B's turn: B takes its +// turn there, and the one walk delivers C and B; A is covered once it is done. +func TestAMemberWhoseWalkCarriesTheSupersededOneBeforeItTakesItsTurn(t *testing.T) { + s := newShape445() + s.c.State, s.c.Walk = Published, &WalkSeen{ID: "plan-2", Commit: mergedC, State: walkBuilding, Waits: true, + WaitedFor: "mesh-delivery", Branch: "main", Created: at445.Add(4 * time.Minute)} + s.b.Walk = &WalkSeen{ID: "plan-2", Commit: mergedC, State: walkBuilding, Waits: true, WaitedFor: "mesh-delivery", + Branch: "main", Created: at445.Add(4 * time.Minute)} + two := ctlWalk("plan-2", mergedC, at445.Add(4*time.Minute), walkBuilding, &WalkDelivery{Awaits: "mesh-delivery", + Merges: []WalkMerge{{Repository: ctl445, Commit: mergedC}, {Repository: lab445, Commit: mergedB}}}) + two.Tiers = [][]string{modules445, {"mesh-lab"}} + two.Modules["mesh-lab"] = &WalkModule{} + s.walks = []Walk{s.walks[0], two} + w := s.open(t) + letGo445(t, w, "plan-2") + if s := w.state(cID); s != Delivering { + t.Fatalf("C is %s", s) + } + w.forge.contains[mergedA+" in "+mergedC] = true + end445(w, "plan-2", walkDone) + g := w.h.groups[g445] + if state, why := GroupState(g, w.h.membersOf(g)); state != "delivered" { + t.Fatalf("the group reads %s (%s), want delivered", state, why) + } +} + +// A carrier superseded in turn hands the wait to the later merge that took it over: C's waiting walk is folded +// into D's, which answers only D; B waits for D, and goes once D's walk covers A. +func TestTheWaitFollowsACarrierSupersededInTurn(t *testing.T) { + s := newShape445() + s.c.State, s.c.Walk = Published, &WalkSeen{ID: "plan-2", Commit: mergedC, State: walkBuilding, Waits: true, + WaitedFor: "mesh-delivery", Branch: "main", Created: at445.Add(4 * time.Minute)} + s.walks[2] = ctlWalk("plan-2", mergedC, at445.Add(4*time.Minute), walkBuilding, &WalkDelivery{Awaits: "mesh-delivery", + Merges: []WalkMerge{{Repository: ctl445, Commit: mergedC}}}) + s.extra = []*Delivery{{ID: dID445, Repository: ctl445, Commit: headD, Number: 111, Base: "main", Branch: "fix/later-still", + State: Ready, Since: at445.Add(6 * time.Minute), Created: at445.Add(-time.Hour), MergedAs: mergedD, + MergedAt: at445.Add(6 * time.Minute), Plan: planMoving(ctl445, modules445...), Check: &Verdict{Gate: "pass"}}} + w := s.open(t) + // C is a lone delivery at its turn; the fake controller lets plan-2 go. It is folded all the same, as the + // review's shape has it, before it started. + two := w.ctl.walk("plan-2") + two.Delivery.Go = nil + two.State, two.Note = walkSuperseded, "superseded at tier 0 by plan-3 ("+ctl445+" at 4d4d4d4d); planned there again" + w.h.WalkMoved(w.ctl.put(ctlWalk("plan-3", mergedD, at445.Add(7*time.Minute), walkBuilding, + &WalkDelivery{Awaits: "mesh-delivery", Merges: []WalkMerge{{Repository: ctl445, Commit: mergedD}}}))) + w.h.WalkMoved(w.ctl.put(two)) + w.settleAll() + if s := w.state(cID); s != Superseded { + t.Fatalf("C is %s", s) + } + waits445(t, w, dID445) + if strings.Contains(w.delivery(bID).TurnAwaits, cID) { + t.Fatalf("B still waits for C: %q", w.delivery(bID).TurnAwaits) + } + w.forge.contains[mergedA+" in "+mergedD] = true + end445(w, "plan-3", walkDone) + if d := w.delivery(aID); d.CoveredBy == "" || !strings.Contains(d.CoveredBy, dID445) { + t.Fatalf("A is covered by %q", d.CoveredBy) + } + letGo445(t, w, "plan-lab") +} + +// A merged carrier still in an assembling batch has no state bound of its own: it is awaited for +// readyCarrierBound from its merge, and then B is stopped. +func TestACarrierInABatchIsAwaitedWithinTheWindowAndOneWalk(t *testing.T) { + s := newShape445() + s.c.State, s.c.Walk = Ready, nil + closes := at445.Add(3 * time.Minute) + s.walks[2] = Walk{ID: "plan-batch", Repository: ctl445, Branch: "main", Commit: mergedC, State: walkQueued, + Created: at445.Add(2 * time.Minute), Delivery: &WalkDelivery{Awaits: "mesh-delivery", Batch: &WalkBatch{ + ClosesAt: &closes, Behind: "plan-other"}, Merges: []WalkMerge{{Repository: ctl445, Commit: mergedC}}}} + w := s.open(t) + waits445(t, w, cID) + if want := s.c.MergedAt.Add(readyCarrierBound); !w.delivery(bID).TurnAwaitsUntil.Equal(want) { + t.Fatalf("B waits until %v, want %v", w.delivery(bID).TurnAwaitsUntil, want) + } + w.later(readyCarrierBound) + w.settleAll() + stopped445(t, w, "no walk carried it") +} + +// A member whose every module a later delivered merge carried, the forge saying it contains the member's merge, +// is reached even before CoveredBy is written on it: never a stop with an empty reason. +func TestACarriedMemberNotYetMarkedCoveredIsReached(t *testing.T) { + s := newShape445() + s.c.State, s.c.Since = Delivered, at445.Add(9*time.Minute) + s.c.Walk.State = walkDone + for _, m := range modules445 { + s.c.Steps = append(s.c.Steps, Step{Machine: "novox", Module: m, State: StepPassed}, + Step{Machine: stepTheRest, Module: m, State: StepSent}) + } + s.a.ContainedIn = map[string]bool{mergedC: true} + s.walks = s.walks[:2] + store := newMemStore() + for _, d := range []*Delivery{s.a, s.b, s.c} { + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + g := &Group{ID: g445, Members: []string{aID, bID}, Closed: true, Order: []string{aID, bID}, Said: "delivering", + Check: &GroupCheck{Verdict: "pass", Commits: commitsOf([]*Delivery{s.a, s.b})}} + if err := store.PutGroup(g); err != nil { + t.Fatal(err) + } + w := newWorldOver(t, store, newFakeController(), newFakeForge(), at445.Add(10*time.Minute)) + w.h.mu.Lock() + gg := w.h.groups[g445] + if w.h.deliveries[aID].CoveredBy != "" { + t.Fatal("A is marked covered already: the case is not the one under test") + } + w.h.turning = map[string]*turnWant{} + w.h.groupTurns(gg, w.h.membersOf(gg)) + b := w.h.deliveries[bID] + turned := w.h.turning["plan-lab"] != nil && containsString(w.h.turning["plan-lab"].ids, bID) + w.h.mu.Unlock() + if b.State != Published || !turned { + t.Fatalf("B is %s, at its turn %v; want its turn taken", b.State, turned) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 5f9964fb..9221ff13 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -65,6 +65,11 @@ type Delivery struct { // (novox/hq issue 319); Lifted why what held it for a person no longer holds (issue 316). CoveredBy string `json:"covered_by,omitempty"` Lifted string `json:"lifted,omitempty"` + // TurnAwaits says, for a group member whose turn waits for a later merge to carry a superseded member before it, + // which merges and until when; TurnAwaitsUntil is that moment, which bounds the wait (ADR 0276 decision 9, + // novox/hq issue 445). `show` shows both, and `stalled` says them past that moment. + TurnAwaits string `json:"turn_awaits,omitempty"` + TurnAwaitsUntil time.Time `json:"turn_awaits_until,omitempty"` // ContainedIn is the forge's answer, by a later merge commit, to whether that commit contains this one's // merge (gitea_contains): asked once per pair, and kept (ADR 0270). ContainedIn map[string]bool `json:"contained_in,omitempty"` diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index ce8fa550..b2740e75 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -501,6 +501,13 @@ func boundOf(d *Delivery) (Bound, bool) { if d.waitsForItsOwnWalk() { return rewalkBound, true } + if d.State == Published && d.TurnAwaits != "" && !d.TurnAwaitsUntil.IsZero() { + // Its turn waits for a later merge to carry a member before it: bounded by that carrier's own bound, and + // said with it, never the generic "waits for its turn or its word" (novox/hq issue 445). + b := Bounds[Published] + b.For, b.Says = max(b.For, d.TurnAwaitsUntil.Sub(boundSince(d))), d.TurnAwaits + return b, true + } b, bounded := Bounds[d.State] return b, bounded }