amadan
instance moves P0: one moves pass at a time; lift only the fence the worker still holds
Review finding (Opus): the fast converge and the instances loop both call runMovesPass with no shared lock, so a pass holding a record list read before another pass's cut could lift that cut's fence through the at-rest branch, and its unconditional delete could orphan a newer generation. The worker now serialises passes and forgets a fence with CompareAndDelete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Human
Prompt
Can we take carlos/platform #175 ?
From this commit's own message. The prompt ledger has no entries for it — which is what a squashed, rebased or amended commit looks like, and what every commit looks like in a repo without a ledger.
Summary
Serialises the moves pass and makes fence bookkeeping compare-and-delete.
2 files changed,
+51
−2
internal/host/movespass.go+9 −2internal/host/movespass_test.go+42 −0
CI passed — run details
diff --git a/internal/host/movespass.go b/internal/host/movespass.go| index 8c00bc28b..6ec19b166 100644 |
| --- a/internal/host/movespass.go |
| +++ b/internal/host/movespass.go |
| @@ -51,7 +51,12 @@ import ( |
| // the once-per-host status nudge, and the per-host trouble throttle. |
| // RunAgent writes the pointer once; both converge loops read it. |
| type movesWorker struct { |
| - cfg moveCfg |
| + cfg moveCfg |
| + // pass runs one movesPass at a time. The fast converge and the |
| + // instances loop both call runMovesPass, and two concurrent passes |
| + // could drive one cut twice, or lift a fence from a record list read |
| + // before the cut that installed it. |
| + pass sync.Mutex |
| mu sync.Mutex |
| mirrors map[string]lease.MirrorStore // dstRegion → client pair |
| nudged sync.Map // host → true: status→quiesce hop nudged |
| @@ -183,6 +188,8 @@ type moveCfg struct { |
| // movesPass runs one round over every in-flight move this box owns. |
| func movesPass(ctx context.Context, s release.Store, w *movesWorker) error { |
| + w.pass.Lock() |
| + defer w.pass.Unlock() |
| cfg := w.cfg |
| recs, err := provision.ListAllRecords(ctx, s) |
| if err != nil { |
| @@ -267,7 +274,7 @@ func (w *movesWorker) liftFences(recs []provision.Record) { |
| return true |
| } |
| w.cfg.LiftFence(host, gen) |
| - w.fenced.Delete(host) |
| + w.fenced.CompareAndDelete(host, gen) |
| log.Printf("moves: %s: wake fence lifted", host) |
| return true |
| }) |
diff --git a/internal/host/movespass_test.go b/internal/host/movespass_test.go| index 5b75366a2..9fca637a1 100644 |
| --- a/internal/host/movespass_test.go |
| +++ b/internal/host/movespass_test.go |
| @@ -411,6 +411,48 @@ func TestMovesPassCutFencesWakesUntilTheRouteRetires(t *testing.T) { |
| } |
| } |
| +// A fence on a host whose record is at rest here again (a move back landed) |
| +// is lifted even though this box holds the row: the row serves now. |
| +func TestMovesPassLiftsTheFenceOfARecordAtRestHere(t *testing.T) { |
| + h := newMoveHarness(t) |
| + host := "a.go.example.com" |
| + h.record(t, host) // region ie, no move: at rest on this box |
| + lifted := uint64(0) |
| + h.cfg.LiftFence = func(_ string, gen uint64) bool { lifted = gen; return true } |
| + h.cfg.Route = func(string) (*registry.Route, error) { return ®istry.Route{Host: host, Status: "ready"}, nil } |
| + w := h.worker() |
| + w.fenced.Store(host, uint64(42)) |
| + if err := movesPass(context.Background(), h.s, w); err != nil { |
| + t.Fatal(err) |
| + } |
| + if lifted != 42 { |
| + t.Fatalf("lifted generation %d, want 42", lifted) |
| + } |
| + if _, ok := w.fenced.Load(host); ok { |
| + t.Fatal("the worker still remembers a lifted fence") |
| + } |
| +} |
| + |
| +// Passes run one at a time: the fast converge and the instances loop both |
| +// call runMovesPass, and a pass that lifted fences from a record list read |
| +// before another pass's cut would reopen wakes mid-cut. |
| +func TestMovesPassRunsOneAtATime(t *testing.T) { |
| + h := newMoveHarness(t) |
| + w := h.worker() |
| + w.pass.Lock() |
| + done := make(chan error, 1) |
| + go func() { done <- movesPass(context.Background(), h.s, w) }() |
| + select { |
| + case <-done: |
| + t.Fatal("a second pass ran while the first held the worker") |
| + case <-time.After(50 * time.Millisecond): |
| + } |
| + w.pass.Unlock() |
| + if err := <-done; err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| // A fence that cannot be installed stops the cut before it looks at the |
| // child: nothing is mirrored, purged or flipped. |
| func TestMovesPassCutStopsWhenTheFenceFails(t *testing.T) { |