From 1de89cd702048f7d760967962db8f733ceeedee8 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 11:33:51 +0200 Subject: [PATCH] Name a walk of nothing-to-move merges instead of letting it go (issue 529) The review showed the give-the-word rule skipped a failed or missing check and group order, and let go moves nothing judged; to-be 47 has no such path. Such a walk is now said, by stalled with walks, as held by nothing, why. --- .../cmd/mesh-delivery/answers.go | 113 ++++-------------- .../mesh-delivery/cmd/mesh-delivery/holder.go | 2 - .../cmd/mesh-delivery/issue529_test.go | 60 ++-------- 3 files changed, 36 insertions(+), 139 deletions(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/answers.go b/modules/mesh-delivery/cmd/mesh-delivery/answers.go index f26c59a7..3323a5c5 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/answers.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/answers.go @@ -423,27 +423,17 @@ func (h *Holder) words() []func() { return asks } -// --- a walk answering only merges that moved nothing --------------------------------------------------------- - // endedMovingNothing is whether a delivery ended delivered with nothing for a walk to move: merged, its plan // moving no module the mesh holds, and no walk followed. func endedMovingNothing(d *Delivery) bool { return d.State == Delivered && d.MergedAs != "" && d.Walk == nil && d.MovesNothing() } -// answeredMovingNothing are the deliveries a waiting walk answers when every merge it answers ended delivered -// with nothing for a walk to move; nil when any merge it answers is held by a live delivery, ended otherwise, or -// is not known to this owner. Under the lock. -// -// novox/hq issues 362 (the nzbgeek case, 2026-10-11) and 529: a merge whose plan adds a module and sends it -// nowhere is delivered at once, but the controller may still open a walk for it — when the module was placed on a -// machine after its plan was made. No live delivery then holds the walk, and nothing gave it its word: it waited -// 39 minutes for a person. Nothing is left for a person to decide about such a walk, so this owner gives it. -func (h *Holder) answeredMovingNothing(w Walk) []*Delivery { - if w.Release != nil || w.IsBatch() || !w.Open() || !w.Seen().Waits { - return nil - } - var out []*Delivery +// endedAsNothingToMove are the deliveries a walk answers that ended delivered as nothing for a walk to move (novox/hq +// issues 362, 529: the nzbgeek case); empty when it answers any other. Such a walk is held by nothing: said, never +// let go by this owner. Under the lock. +func (h *Holder) endedAsNothingToMove(w Walk) []string { + var out []string for _, d := range h.deliveries { _, answers := walkAnswers(w, d) if !answers && (d.Walk == nil || d.Walk.ID != w.ID) { @@ -452,77 +442,12 @@ func (h *Holder) answeredMovingNothing(w Walk) []*Delivery { if !endedMovingNothing(d) { return nil } - out = append(out, d) + out = append(out, d.ID) } - if len(out) == 0 { - return nil - } - // Every merge it names is one of them: a merge not heard of, or not yet ended, is not this owner's to answer. - if w.Delivery != nil { - for _, m := range w.Delivery.Merges { - found := false - for _, d := range out { - if strings.EqualFold(d.Repository, m.Repository) && sameCommit(d.MergedAs, m.Commit) { - found = true - break - } - } - if !found { - return nil - } - } - } - sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID }) + sort.Strings(out) return out } -// nothingToMoveWords are the asks of the word for each waiting walk that answers only merges ended as nothing for -// a walk to move (answeredMovingNothing). Under the lock. -func (h *Holder) nothingToMoveWords() []func() { - ids := make([]string, 0, len(h.walks)) - for id := range h.walks { - ids = append(ids, id) - } - sort.Strings(ids) - var asks []func() - for _, walk := range ids { - if h.asking[walk] { - continue - } - ended := h.answeredMovingNothing(h.walks[walk].walk) - if len(ended) == 0 { - continue - } - var names []string - for _, d := range ended { - names = append(names, d.ID) - } - why := "it answers only " + strings.Join(names, ", ") + ", delivered with nothing for a walk to move: " + - "no person has anything to decide (novox/hq issue 529)" - h.asking[walk] = true - walk := walk - asks = append(asks, func() { - err := h.Controller.Deliver(walk, why) - h.mu.Lock() - defer h.mu.Unlock() - delete(h.asking, walk) - if err != nil { - h.Logf("[mesh-delivery] the walk %s could not be let go yet: %v", walk, err) - return - } - h.Logf("[mesh-delivery] the walk %s was let go: %s", walk, why) - // Asked once: the record kept says it went, until the controller's next record says more. - if s, known := h.walks[walk]; known && s.walk.Delivery != nil { - wd, now := *s.walk.Delivery, h.Now() - wd.Go, wd.By, wd.Why = &now, byOwner, why - s.walk.Delivery = &wd - h.walks[walk] = s - } - }) - } - return asks -} - // WalksWaiting is `stalled` asked with `walks` (novox/hq issue 529): for each walk waiting for this owner's word, // what holds it, in the shape of a stalled line naming the walk — each held delivery it carries (State held, its // reason and the verb that releases it), else why its word was not given at the last tick (State published), else @@ -557,13 +482,17 @@ func (h *Holder) WalksWaiting() []StalledLine { case h.waitsWhy[id] != "": out = append(out, StalledLine{ID: IDOf(w.Repository, w.Commit), State: Published, Walk: id, For: waited, Bound: "0s", H2: "none: it goes at its turn", Says: "not let go yet: " + h.waitsWhy[id]}) - case len(h.answeredMovingNothing(w)) > 0: - // Its word is given at the next tick (nothingToMoveWords). case !live: + says := "no delivery that has not ended follows it or is answered by it, so nothing gives it its word: " + + "`mesh-delivery.show` for the deliveries of " + w.Repository + if ended := h.endedAsNothingToMove(w); len(ended) > 0 { + // The nzbgeek case (novox/hq issues 362, 529): its merges' deliveries ended as nothing for a walk to + // move, yet the walk moves modules — nothing judged those, so nothing here lets it go. + says = "held by nothing: its merges' deliveries ended as nothing-to-move (" + strings.Join(ended, ", ") + + "), yet the walk moves " + strings.Join(walkModules(w), ", ") + " — nothing gives it its word" + } out = append(out, StalledLine{ID: IDOf(w.Repository, w.Commit), State: Unheld, Walk: id, For: waited, - Bound: "0s", H2: "none: no delivery holds the walk — the operator's", - Says: "no delivery that has not ended follows it or is answered by it, so nothing gives it its word: " + - "`mesh-delivery.show` for the deliveries of " + w.Repository}) + Bound: "0s", H2: "none: no delivery holds the walk — the operator's", Says: says}) } } sort.Slice(out, func(i, j int) bool { @@ -574,3 +503,13 @@ func (h *Holder) WalksWaiting() []StalledLine { }) return out } + +// walkModules are the modules a walk's record names, sorted. +func walkModules(w Walk) []string { + out := make([]string, 0, len(w.Modules)) + for m := range w.Modules { + out = append(out, m) + } + sort.Strings(out) + return out +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index dc0d1355..554ee6a1 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -1032,8 +1032,6 @@ func (h *Holder) plan() []func() { } // The word, once per walk, when every live delivery it carries is at its turn. asks = append(asks, h.words()...) - // A waiting walk that answers only merges ended as nothing for a walk to move (novox/hq issues 362, 529). - asks = append(asks, h.nothingToMoveWords()...) // Waiting walks no live delivery holds: listed by `stalled` (issue 362). h.seeOrphans(now) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go index 60c8a111..f7778a59 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue529_test.go @@ -48,62 +48,22 @@ func nzbWalk(created time.Time, merges ...WalkMerge) Walk { return w } -// **A walk answering only merges that ended as nothing for a walk to move is given its word** (novox/hq issues 362 -// and 529): no live delivery holds it, no person has anything to decide, and nothing else would ever let it go. -func TestAWalkAnsweringOnlyNothingToMoveMergesIsGivenItsWord(t *testing.T) { +// **A walk answering only merges that ended as nothing for a walk to move is named as held by nothing** (novox/hq +// issues 362 and 529): never let go by this owner — nothing judged what it moves — and said, by `stalled` with +// `walks`, with why, so the controller's `plans` says so instead of pointing nowhere. +func TestAWalkOfNothingToMoveMergesIsNamedAsHeldByNothing(t *testing.T) { w := nzbgeekMerged(t) w.later(90 * time.Second) w.h.WalkMoved(w.ctl.put(nzbWalk(w.now, WalkMerge{Repository: media, Commit: mergeNzb}))) w.settleAll() - if len(w.ctl.delivers) != 1 || w.ctl.delivers[0] != nzbWalkID { - t.Fatalf("the walk answering only #18 was let go %v; stalled says %+v", w.ctl.delivers, w.h.Stalled()) - } - w.h.WalkMoved(w.ctl.walk(nzbWalkID)) - w.later(3 * time.Minute) - w.settleAll() - for _, l := range w.h.Stalled() { - if l.Walk == nzbWalkID { - t.Fatalf("stalled still lists the walk: %+v", l) - } - } - if d := w.delivery(IDOf(media, headNzb)); d.State != Delivered || d.Walk != nil { - t.Fatalf("#18 is %s on %v: an ended delivery is never tied to a walk", d.State, d.Walk) - } -} - -// The same walk from a controller that names no merges: it answers the merge of its own commit. -func TestAWalkOfANothingToMoveMergesOwnCommitIsGivenItsWord(t *testing.T) { - w := nzbgeekMerged(t) - w.later(90 * time.Second) - w.h.WalkMoved(w.ctl.put(nzbWalk(w.now))) - w.settleAll() - if len(w.ctl.delivers) != 1 || w.ctl.delivers[0] != nzbWalkID { - t.Fatalf("the walk of #18's commit was let go %v", w.ctl.delivers) - } -} - -// Never for a walk that also answers a merge this owner holds no ended nothing-to-move delivery of: a merge not -// heard yet, or one that ended otherwise, is not a word this owner may give. -func TestAWalkAnsweringAnotherMergeIsNotGivenTheWordForANothingToMoveOne(t *testing.T) { - w := nzbgeekMerged(t) - w.later(90 * time.Second) - w.h.WalkMoved(w.ctl.put(nzbWalk(w.now, WalkMerge{Repository: media, Commit: mergeNzb}, - WalkMerge{Repository: media, Commit: "efefefef00002121"}))) - w.settleAll() if len(w.ctl.delivers) != 0 { - t.Fatalf("a walk answering a merge not heard was let go: %v", w.ctl.delivers) + t.Fatalf("a walk no live delivery holds was let go: %v", w.ctl.delivers) } -} - -// Its word is said: the log of the owner and the walk's why name the merges that ended as nothing to move. -func TestTheWordForANothingToMoveWalkSaysWhy(t *testing.T) { - w := nzbgeekMerged(t) - w.h.WalkMoved(w.ctl.put(nzbWalk(w.now, WalkMerge{Repository: media, Commit: mergeNzb}))) - w.settleAll() - walk := w.ctl.walk(nzbWalkID) - if walk.Delivery == nil || !strings.Contains(walk.Delivery.Why, "nothing for a walk to move") || - !strings.Contains(walk.Delivery.Why, IDOf(media, headNzb)) { - t.Fatalf("the word says %+v", walk.Delivery) + lines := w.h.WalksWaiting() + if len(lines) != 1 || lines[0].Walk != nzbWalkID || lines[0].State != Unheld || + !strings.Contains(lines[0].Says, "ended as nothing-to-move") || !strings.Contains(lines[0].Says, IDOf(media, headNzb)) || + !strings.Contains(lines[0].Says, "nzbget") { + t.Fatalf("the walk is said as %+v", lines) } }