Merge pull request 'mesh-delivery: a group whose members all ended ends, never delivering (hq issue 442)' (#197) from fix/442-a-group-whose-members-ended-ends into main
This commit was merged in pull request #197.
This commit is contained in:
@@ -1327,6 +1327,25 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func(
|
||||
h.keepGroup(g)
|
||||
})
|
||||
}
|
||||
// A superseded member of a closed group: whether a later delivered walk on its trunk carried every module it
|
||||
// moves, ADR 0270's covered test, kept on it as CoveredBy. Its own state stays superseded — a final state never
|
||||
// moves — and its group counts it as reached (novox/hq issue 442).
|
||||
if g.Closed {
|
||||
for _, d := range members {
|
||||
if d.State != Superseded || d.CoveredBy != "" {
|
||||
continue
|
||||
}
|
||||
var ask []string
|
||||
if d.CoveredBy, ask = h.coverOf(d); d.CoveredBy != "" {
|
||||
h.keep(d)
|
||||
}
|
||||
for _, in := range ask {
|
||||
if a := h.askContains(d, in, now); a != nil {
|
||||
asks = append(asks, a)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
// Its state, said when it changes.
|
||||
if state, why := GroupState(g, members); state != g.Said {
|
||||
g.Said = state
|
||||
@@ -1398,6 +1417,10 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() {
|
||||
}
|
||||
switch {
|
||||
case d.State == Failed || d.State == Stopped || d.State == Superseded:
|
||||
// A superseded member is an end here, and it stops the members after it even when a later delivered
|
||||
// walk carried it. GroupState counts such a member as reached (novox/hq issue 442), so for a carried
|
||||
// member the two do not agree: the members after it are stopped all the same, and the group then reads
|
||||
// stopped for them. That gap is novox/hq issue 445, not closed here.
|
||||
for _, later := range ordered[i+1:] {
|
||||
if a := h.stopInGroup(byID[later.ID], fmt.Sprintf("its group %s stopped: %s %s", g.ID, d.ID, d.State)); a != nil {
|
||||
asks = append(asks, a)
|
||||
@@ -1467,11 +1490,32 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() {
|
||||
}
|
||||
}
|
||||
|
||||
// GroupState is a group's state, derived from its members and its composed check — never set apart.
|
||||
// GroupState is a group's state, derived from its members and its composed check — never set apart (novox/hq
|
||||
// ADR 0239 decision 4, which lists checking, rejected, ready, delivering, delivered, failed and stopped).
|
||||
//
|
||||
// It extends that list for a superseded member, which ADR 0239 does not name (novox/hq issue 442: five closed
|
||||
// groups read "delivering, 1 of 2 delivered" for days with nothing in progress). A superseded member is an end.
|
||||
// It counts as reached when a later delivered walk on its trunk carried every module it moves (CoveredBy, ADR
|
||||
// 0270's covered test); a closed group whose members all ended is then delivered, and otherwise stopped, naming
|
||||
// the member. A group with nothing in progress never reads delivering.
|
||||
//
|
||||
// groupTurns does not yet agree for a carried member: it stops the members after any superseded one, carried or
|
||||
// not, and a stopped member makes the group read stopped here. That gap is novox/hq issue 445.
|
||||
func GroupState(g *Group, members []*Delivery) (string, string) {
|
||||
count := map[State]int{}
|
||||
reached, ended := 0, 0
|
||||
var uncovered []string
|
||||
for _, d := range members {
|
||||
count[d.State]++
|
||||
if d.State.Final() {
|
||||
ended++
|
||||
}
|
||||
switch {
|
||||
case d.State.Reached(), d.State == Superseded && d.CoveredBy != "":
|
||||
reached++
|
||||
case d.State == Superseded:
|
||||
uncovered = append(uncovered, d.ID)
|
||||
}
|
||||
}
|
||||
n := len(members)
|
||||
switch {
|
||||
@@ -1479,10 +1523,12 @@ func GroupState(g *Group, members []*Delivery) (string, string) {
|
||||
return "failed", "a member failed"
|
||||
case count[Stopped] > 0 && g.Closed:
|
||||
return "stopped", "a member was stopped"
|
||||
case n > 0 && count[Delivered]+count[Retired]+count[Covered] == n:
|
||||
case n > 0 && reached == n:
|
||||
return "delivered", "every member is delivered"
|
||||
case g.Closed && n > 0 && ended == n:
|
||||
return "stopped", supersededWhy + strings.Join(uncovered, ", ")
|
||||
case g.Closed:
|
||||
return "delivering", fmt.Sprintf("%d of %d delivered", count[Delivered]+count[Covered], n)
|
||||
return "delivering", fmt.Sprintf("%d of %d delivered", reached, n)
|
||||
case len(g.Cycle) > 0:
|
||||
return "rejected", cycleSaid(g)
|
||||
case count[Rejected] > 0:
|
||||
@@ -1496,6 +1542,10 @@ func GroupState(g *Group, members []*Delivery) (string, string) {
|
||||
return "checking", fmt.Sprintf("%d of %d member(s) ready", count[Ready], n)
|
||||
}
|
||||
|
||||
// supersededWhy begins why a group whose members all ended is stopped. It is kept short: the forge's status line,
|
||||
// "group <id> stopped: <why>", is clipped at 140 characters, and the member's id must fit in it whole.
|
||||
const supersededWhy = "superseded, not carried by a later delivery: "
|
||||
|
||||
func groupStatusState(s string) string {
|
||||
switch s {
|
||||
case "ready", "delivered":
|
||||
|
||||
@@ -0,0 +1,201 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// novox/hq issue 442: five closed groups read "delivering, 1 of 2 delivered" for days, each beside a member
|
||||
// whose walk a later walk of the same trunk superseded. Nothing in them was still in progress. A superseded
|
||||
// member counts as reached when a later delivered walk on the same trunk carried every module it moves (ADR
|
||||
// 0270's covered test); a closed group whose members all ended reads delivered when every one reached the
|
||||
// machines, and stopped otherwise — never delivering.
|
||||
//
|
||||
// The shapes are the live groups' as mesh-delivery showed them on 2026-10-11: feat/delivery-checks-verb (the
|
||||
// superseded member last in the order) and fix/assign-says-why-a-module-is-not-there (the superseded member
|
||||
// first).
|
||||
func TestAGroupWhoseMembersEndedEnds(t *testing.T) {
|
||||
const cat, ctl, lab = "novox/mesh-catalog", "novox/mesh-controller", "novox/mesh-lab"
|
||||
at := time.Date(2026, 10, 7, 19, 4, 10, 0, time.UTC)
|
||||
|
||||
// superseded is a member merged and walked, its walk superseded by a later walk of the same trunk.
|
||||
superseded := func(repo, sha, merged, group string, modules ...string) *Delivery {
|
||||
return &Delivery{ID: IDOf(repo, sha), Repository: repo, Commit: sha, Number: 109, Base: "main",
|
||||
Branch: group, Group: group, State: Superseded, Since: at.Add(time.Minute), Created: at.Add(-time.Hour),
|
||||
MergedAs: merged, MergedAt: at, Plan: planMoving(repo, modules...), Check: &Verdict{Gate: "pass"},
|
||||
Walk: &WalkSeen{ID: "plan-" + merged[:6], Commit: merged, State: walkSuperseded,
|
||||
Note: "superseded at tier 0 by plan-later (" + repo + " at 68aeee2c); planned there again"},
|
||||
Transitions: []Transition{{At: at.Add(time.Minute), From: Delivering, To: Superseded, Event: EvSuperseded,
|
||||
Why: "a newer delivery to the same trunk took over its walk"}}}
|
||||
}
|
||||
// delivered is the other member, delivered by its own walk.
|
||||
delivered := func(repo, sha, merged, group string) *Delivery {
|
||||
return &Delivery{ID: IDOf(repo, sha), Repository: repo, Commit: sha, Number: 117, Base: "main", Branch: group,
|
||||
Group: group, State: Delivered, Since: at.Add(-10 * time.Minute), Created: at.Add(-2 * time.Hour),
|
||||
MergedAs: merged, MergedAt: at.Add(-time.Hour), Plan: planMoving(repo, "mesh-controller"),
|
||||
Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-" + merged[:6], Commit: merged, State: walkDone}}
|
||||
}
|
||||
// later is the later merge of the same repository whose walk took the member's over and finished, sending
|
||||
// each module given to every machine running it.
|
||||
later := func(repo, merged string, modules ...string) *Delivery {
|
||||
d := &Delivery{ID: IDOf(repo, "c0c0c0c0c0c0c0c0"), Repository: repo, Commit: "c0c0c0c0c0c0c0c0", Number: 110,
|
||||
Base: "main", State: Delivered, Since: at.Add(30 * time.Minute), Created: at.Add(-time.Hour),
|
||||
MergedAs: merged, MergedAt: at.Add(time.Minute), Plan: planMoving(repo, modules...), Check: &Verdict{Gate: "pass"},
|
||||
Walk: &WalkSeen{ID: "plan-later", Commit: merged, State: walkDone}}
|
||||
for _, m := range modules {
|
||||
d.Steps = append(d.Steps, Step{Machine: "novox", Module: m, State: StepPassed},
|
||||
Step{Machine: "the rest running it", Module: m, State: StepSent})
|
||||
}
|
||||
return d
|
||||
}
|
||||
group := func(id string, order ...string) *Group {
|
||||
return &Group{ID: id, Members: append([]string(nil), order...), Closed: true, Order: order, Said: "delivering",
|
||||
OrderOf: map[string]string{}}
|
||||
}
|
||||
|
||||
const checks, assign = "feat/delivery-checks-verb", "fix/assign-says-why-a-module-is-not-there"
|
||||
catID, ctlID := IDOf(cat, "13894744a701fe59"), IDOf(ctl, "1fdff00794e95470")
|
||||
asCtlID, asLabID := IDOf(ctl, "8adb7f1a051941e9"), IDOf(lab, "33e63ee5b47a7552")
|
||||
for _, c := range []struct {
|
||||
name string
|
||||
kept []*Delivery
|
||||
group *Group
|
||||
contains bool // what the forge says of the later merge containing the superseded member's
|
||||
want string
|
||||
whySays string
|
||||
}{
|
||||
{"superseded last, a later delivered walk carried every module it moves",
|
||||
[]*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"),
|
||||
delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks),
|
||||
later(cat, "68aeee2c00000000", "gitea", "mesh-delivery")},
|
||||
group(checks, ctlID, catID), true, "delivered", "every member"},
|
||||
{"superseded last, the later walk carried one of its two modules",
|
||||
[]*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"),
|
||||
delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks),
|
||||
later(cat, "68aeee2c00000000", "mesh-delivery")},
|
||||
group(checks, ctlID, catID), true, "stopped", "not carried by a later delivery: "},
|
||||
{"superseded last, the forge says the later merge does not contain it",
|
||||
[]*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"),
|
||||
delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks),
|
||||
later(cat, "68aeee2c00000000", "gitea", "mesh-delivery")},
|
||||
group(checks, ctlID, catID), false, "stopped", "not carried by a later delivery: "},
|
||||
{"superseded first, a later delivered walk carried every module it moves",
|
||||
[]*Delivery{superseded(ctl, "8adb7f1a051941e9", "a5f53ef9ebdf3650", assign, "build-agent", "mesh-controller", "route-proxy"),
|
||||
delivered(lab, "33e63ee5b47a7552", "77777777aaaaaaaa", assign),
|
||||
later(ctl, "3f68a49500000000", "build-agent", "mesh-controller", "route-proxy")},
|
||||
group(assign, asCtlID, asLabID), true, "delivered", "every member"},
|
||||
{"superseded first, nothing later delivered",
|
||||
[]*Delivery{superseded(ctl, "8adb7f1a051941e9", "a5f53ef9ebdf3650", assign, "build-agent", "mesh-controller", "route-proxy"),
|
||||
delivered(lab, "33e63ee5b47a7552", "77777777aaaaaaaa", assign)},
|
||||
group(assign, asCtlID, asLabID), true, "stopped", "not carried by a later delivery: "},
|
||||
{"still in progress: a member delivering beside a superseded one",
|
||||
func() []*Delivery {
|
||||
d := delivered(ctl, "1fdff00794e95470", "85d664438f87f885", checks)
|
||||
d.State, d.Walk.State = Delivering, walkRolling
|
||||
return []*Delivery{superseded(cat, "13894744a701fe59", "9d4bbbd2bd271049", checks, "gitea", "mesh-delivery"), d}
|
||||
}(),
|
||||
group(checks, ctlID, catID), true, "delivering", "0 of 2"},
|
||||
} {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
store := newMemStore()
|
||||
for _, d := range c.kept {
|
||||
if err := store.PutDelivery(d); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if err := store.PutGroup(c.group); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
forge := newFakeForge()
|
||||
forge.contains[c.kept[0].MergedAs+" in "+"68aeee2c00000000"] = c.contains
|
||||
forge.contains[c.kept[0].MergedAs+" in "+"3f68a49500000000"] = c.contains
|
||||
w := newWorldOver(t, store, newFakeController(), forge, at.Add(4*24*time.Hour))
|
||||
if err := w.h.Reconcile(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
w.settleAll()
|
||||
g := w.h.groups[c.group.ID]
|
||||
state, why := GroupState(g, w.h.membersOf(g))
|
||||
if state != c.want || !strings.Contains(why, c.whySays) {
|
||||
t.Fatalf("the group reads %s (%s), want %s saying %q", state, why, c.want, c.whySays)
|
||||
}
|
||||
// The forge is asked once per pair, however many ticks follow.
|
||||
for range 3 {
|
||||
w.later(2 * time.Minute)
|
||||
w.settleAll()
|
||||
}
|
||||
w.forge.mu.Lock()
|
||||
asked := map[string]int{}
|
||||
for _, a := range w.forge.containsAsks {
|
||||
asked[a]++
|
||||
}
|
||||
w.forge.mu.Unlock()
|
||||
for pair, n := range asked {
|
||||
if n != 1 {
|
||||
t.Fatalf("the forge was asked %d times whether %s", n, pair)
|
||||
}
|
||||
}
|
||||
// What was learned survives a read-back, and the group reads the same with no new ask.
|
||||
sup := w.delivery(c.kept[0].ID)
|
||||
again := newWorldOver(t, store, newFakeController(), forge, w.now.Add(time.Hour))
|
||||
if err := again.h.Reconcile(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
again.settleAll()
|
||||
back := again.delivery(c.kept[0].ID)
|
||||
if back.CoveredBy != sup.CoveredBy || len(back.ContainedIn) != len(sup.ContainedIn) {
|
||||
t.Fatalf("read back, covered by %q and contained in %v; was %q and %v", back.CoveredBy, back.ContainedIn,
|
||||
sup.CoveredBy, sup.ContainedIn)
|
||||
}
|
||||
for k, v := range sup.ContainedIn {
|
||||
if back.ContainedIn[k] != v {
|
||||
t.Fatalf("read back, contained in %s is %v, was %v", k, back.ContainedIn[k], v)
|
||||
}
|
||||
}
|
||||
if s2, _ := GroupState(again.h.groups[c.group.ID], again.h.membersOf(again.h.groups[c.group.ID])); s2 != state {
|
||||
t.Fatalf("read back, the group reads %s, was %s", s2, state)
|
||||
}
|
||||
forge.mu.Lock()
|
||||
n := len(forge.containsAsks)
|
||||
forge.mu.Unlock()
|
||||
if want := len(asked); c.want != "delivering" && n != want {
|
||||
t.Fatalf("read back, the forge was asked again: %d asks, was %d", n, want)
|
||||
}
|
||||
// Each member keeps its own end: the group's label changes, no delivery is moved by hand.
|
||||
for _, d := range c.kept {
|
||||
if got := w.state(d.ID); got != d.State {
|
||||
t.Fatalf("%s moved from %s to %s", d.ID, d.State, got)
|
||||
}
|
||||
}
|
||||
// What the group said reached every member's head, once it ended.
|
||||
if c.want != "delivering" {
|
||||
w.forge.mu.Lock()
|
||||
s := w.forge.statuses[c.kept[0].Repository+"@"+c.kept[0].Commit+" mesh/delivery-group"]
|
||||
w.forge.mu.Unlock()
|
||||
if !strings.Contains(s, c.want) {
|
||||
t.Fatalf("mesh/delivery-group on the superseded member's head says %q", s)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The forge's mesh/delivery-group line, "group <id> stopped: <why>", is clipped at 140 characters: for each of
|
||||
// the five live groups of novox/hq issue 442, the superseded member's id fits in it whole.
|
||||
func TestAStoppedGroupsLineNamesItsSupersededMemberWhole(t *testing.T) {
|
||||
for group, member := range map[string]string{
|
||||
"feat/delivery-checks-verb": "novox/mesh-catalog@13894744a701",
|
||||
"feat/machine-network-health": "novox/mesh-controller@b8bbf9c79fd8",
|
||||
"fix/373-one-message-per-condition": "novox/mesh-catalog@8dfc19c9f7c8",
|
||||
"fix/assign-says-why-a-module-is-not-there": "novox/mesh-controller@8adb7f1a0519",
|
||||
"fix/needs-you-from-the-console": "novox/mesh-controller@2e6a9b1efc05",
|
||||
} {
|
||||
g := &Group{ID: group, Members: []string{member, "novox/hq@39585e3bb2ea"}, Closed: true}
|
||||
state, why := GroupState(g, []*Delivery{{ID: member, State: Superseded}, {ID: "novox/hq@39585e3bb2ea", State: Delivered}})
|
||||
line := clip("group "+group+" "+state+": "+why, 140)
|
||||
if state != "stopped" || !strings.HasSuffix(line, member) {
|
||||
t.Fatalf("group %s: the line %q (%d characters) does not end in %s", group, line, len(line), member)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user