A no-move tier waits for no new report, so its walk asks the next tier (issue 648) #309
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: novox/mesh-controller#309
Reference in New Issue
Block a user
Fixes issue 648.
What. In a walk's tier, a build made from the source every machine running it runs (issue 280) is marked no move:
SentAtis set to now and nothing is sent. The wait that gates the next tier then asked each machine running the module for a report at or after max(BuiltAt, SentAt). Nothing was sent, so no report follows, and a machine that does not report again for another reason held the walk for ever. Seen live in plan-1791919354338134213 (mesh-controller #304), tier 1 waiting on build-agent on g14 and shanks.Fix. In the tier's wait, a module the plan marked no move (its
Whysays "no move", no first send, no gate) is judged per machine by the send that last carried its build: the machine runs it once its last report is at or after its last send (Reported.Sent). That is the send whose builds made it no move (unchangedOnEveryreads each machine's last send). A machine that has not answered its last send is still waited on. The mark is read fromWhy, so a plan kept by the controller now running (such as the stuck walk) is read the same; the text is now a constant,noMoveWhy.Tested. New store test
TestANoMoveTierWaitsForNoNewReport(listed in testdata/store-tests): a two-tier plan whose first tier is the build machine, no move on anchor and laptop, both having answered their last send before the build. On main it fails withtier 0, rolling "tier 0 built; waiting for app on anchor, laptop to be applied"; with the fix the walk asks tier 1 in the same step. Its second half shows a machine whose last send is unanswered is still waited on.go test ./cmd/mesh-controlleragainst a throwaway store passes;go vetclean.Delivery
novox/mesh-controller@70885ab3c4a3— delivered since 2026-10-14T00:36:03ZDelivery plan — builds mesh-controller → novox; no bus step
Machines
Transitions
The commit's note under
refs/notes/mesh-plankeeps every transition:git log --notes=mesh-plan.Merge check at
c877a826Judged: head
c877a8267eb9merged into trunkbff5b7f0e4f8; read beside it: mesh-catalog7ed119995f69, mesh-controller-mainbff5b7f0e4f8, mesh-host0e92ef3d1b21, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller, route-proxy)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-controller, route-proxy → ace, novox; no bus step
Run by the build seat on shanks as
build-1791921638316140191(builds --log build-1791921638316140191).Verdict: the fix is sound, but this pull request does not merge: rebase it on main (
ca3f43c4) and resolve the conflict in testdata/store-tests, then recheck.Reviewed at head
c877a826(unchanged since it was asked for).A send still in flight cannot let a tier pass. For a no-move module each machine is asked for a report at or after its own last send (node.sent_at). A send after the last report keeps the machine waiting. The second half of the test shows this ("waiting for app on laptop"). Both times come from the store's clock: node_report.at is the store's now(), and sent_at is set by the controller's store too, so machine clock skew does not enter. A machine that was never sent anything and has reported counts as running, which is right, because no-move means it already runs that build's source. One residual: the report is judged by its time, not by Current (whether it names the last declaration). That is the same rule the ordinary path already uses (issue 256), so it is not a regression.
Spoofing the mark. PlanModule.Why is written only by the controller, inside release_plan.go, gate.go and plan_retry.go. No other writer starts it with "no move", and plan_retry clears Why on a retry. noMove also requires FirstAt == nil and Gate == nil, so a module that was sent or gated after being marked is never read as no move. Nothing outside the controller writes plans.
Rewording. noMove matches the literal prefix "no move", not noMoveWhy. If noMoveWhy is reworded to start differently, the code would drift. I checked this: with noMoveWhy reworded to "unchanged: ...", TestANoMoveTierWaitsForNoNewReport fails, so the reword cannot pass silently. A small suggestion, which does not block: define const noMovePrefix = "no move" and build noMoveWhy from it, so the writer and the reader share one string.
Plans the running controller already saved carry exactly the old text "no move: made from the source every machine running it runs", which noMoveWhy repeats word for word. The stuck walk is therefore read the same way after the upgrade.
The test fails on main. With main's release_plan.go in a merged tree, TestANoMoveTierWaitsForNoNewReport fails at issue648_test.go:71 with: tier 0, rolling "tier 0 built; waiting for app on anchor, laptop to be applied". With the fix it passes.
Merging into main
ca3f43c4. git merge conflicts in testdata/store-tests only. Main appended ten store tests (from #312 and others) at the same place. Keeping both sides resolves it, and release_plan.go merges on its own. The forge says mergeable: false. On the resolved merged tree, sh merge-check.sh passed (exit 0, run niced with MESH_TEST_POSTGRES pointing at a throwaway postgres), and every listed store test passed, including the new one. #312's FirstAt and Rest changes do not touch the no-move path: line 1447 still skips it, because SentAt is set.Checks at
c877a826, as mesh-delivery.checks reports them:mesh/merge-gate: pending. "out of date: judged against trunk
bff5b7f0e4, the trunk is now 372faebf2824". The earlier verdict was "PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller, route-proxy)".mesh/repo-check: pending. Same out-of-date note. The earlier verdict was "PASS — its merge-check.sh passed".
mesh/delivery: pending. "checking"; proposed: waits for its check.
Next step: rebase or merge main into the branch, keeping both sides of testdata/store-tests, and then ask mesh-delivery.recheck. Merge only on the new green verdicts.
c877a8267eto70885ab3c4Merge check at
70885ab3Judged: head
70885ab3c4a3merged into trunkca3f43c400ba; read beside it: mesh-catalogebf7f3773dc7, mesh-controller-mainca3f43c400ba, mesh-host1a2677882380, mesh-tools954e1a53eef9mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller)mesh/repo-check: PASS — its merge-check.sh passedDelivery plan — builds mesh-controller → novox; no bus step
Run by the build seat on ace as
build-1791937373861169106(builds --log build-1791937373861169106).Verdict: ready to merge at
70885ab3. Both required checks are green against trunkca3f43c4, and the earlier conflict and suggestion are both addressed.I re-reviewed at head
70885ab3, rebased onto mainca3f43c4. I read the pull request's diff with the forge's diff tool. That tool shows the whole change against main, so I compared it with what I reviewed atc877a826(review 242).release_plan.go: the no-move wait is the same code as at
c877a826, so the findings of review 242 still hold. The new commit adds const noMovePrefix = "no move" and builds noMoveWhy as noMovePrefix + ": made from the source every machine running it runs". noMove() reads strings.HasPrefix(s.Why, noMovePrefix), so the code that writes the mark and the code that reads it share one string. The full text is unchanged word for word, so plans the running controller already saved are still read the same way.testdata/store-tests: the conflict is resolved by keeping both sides. Main's ten added tests stay, and TestANoMoveTierWaitsForNoNewReport is the last line. The forge now says mergeable.
Checks at
70885ab3, as mesh-delivery.checks reports them (head70885ab3c4merged into trunkca3f43c400, build-1791937373861169106 on ace):mesh/merge-gate: success. "PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mesh-controller)"
mesh/repo-check: success. "PASS — its merge-check.sh passed". Its report names "./cmd/mesh-controller --- PASS: TestANoMoveTierWaitsForNoNewReport (0.20s)" among the store tests run against the store.
mesh/delivery: success. "ready: it delivers once merged".