amadan

carlos / platform Public

moves pass: log a retry for the source's recovery and revert (final review I3)

A record that resolve --retry leaves at rolling_back or reverting is sent to
the source's recovery or revert before phase1MoveRound, so the pass never
called observeMove for it and retry-<epoch> was never logged. The pass now
observes those records before it dispatches them. The regression drives a
stuck rollback and a stuck revert through MoveRetryRecovery and real
passes, and expects one retry event each.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude d760c4092238c5cb2de7fa82a51a79f94a022aa5 parent 14c7100
2 files changed, +71 −1
  • internal/host/movespass.go +7 −1
  • internal/host/movesretry_test.go +64 −0
diff --git a/internal/host/movespass.go b/internal/host/movespass.go
index 678ab1a6f..ad3d3aed2 100644
--- a/internal/host/movespass.go
+++ b/internal/host/movespass.go
@@ -786,6 +786,10 @@ func movesPass(ctx context.Context, s release.Store, w *movesWorker) error {
// Plan 4 Task 5): the record's region is the source's again,
// and this box is the move's source by the post-flip
// participant rule (J10), whether or not it still has the row.
+ // The transitions another writer made (an operator's retry,
+ // a declared loss) are logged first: this branch never
+ // reaches phase1MoveRound (final review I3).
+ w.observeMove(ctx, s, rec)
if err := w.recoverSource(ctx, s, rec); err != nil {
w.logTrouble(rec.Host, err)
continue
@@ -798,7 +802,9 @@ func movesPass(ctx context.Context, s release.Store, w *movesWorker) error {
// the record never flipped, and this box is the move's
// source by the same participant rule (J10). Its entry
// gives up execution through the cut, which reads the
- // fleet; the rest does not need it.
+ // fleet; the rest does not need it. An operator's retry is
+ // logged first, as for the recovery above (final review I3).
+ w.observeMove(ctx, s, rec)
var f *moveFleet
if loadFleet() {
f = fleet
diff --git a/internal/host/movesretry_test.go b/internal/host/movesretry_test.go
index 57cb51d41..16bbc04b8 100644
--- a/internal/host/movesretry_test.go
+++ b/internal/host/movesretry_test.go
@@ -259,3 +259,67 @@ func TestRetryLeavesTheClaimAndTheRowAlone(t *testing.T) {
t.Fatalf("a retry of a move that is not stuck: %v", err)
}
}
+
+// countEvent counts event in events.
+func countEvent(events []string, event string) int {
+ n := 0
+ for _, e := range events {
+ if e == event {
+ n++
+ }
+ }
+ return n
+}
+
+// Final review I3: a retry's event comes from the box's own pass. After
+// `move resolve --retry` (the claim-backed MoveRetryRecovery, which advances
+// the epoch) the record is rolling_back or reverting, which movesPass sends
+// to the source's recovery or revert before phase1MoveRound; the pass still
+// logs retry-<epoch>, once, however many passes see it.
+func TestRetryEventIsLoggedByThePass(t *testing.T) {
+ t.Run("a rollback's source recovery", func(t *testing.T) {
+ h := newRecoverHarness(t, false)
+ h.released(t)
+ if err := os.WriteFile(filepath.Join(h.sdir, p1Host, "upload.txt"), []byte("changed on the source"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ h.srcPass(t)
+ if m := h.read(t, p1Host).Move; m.State != provision.MoveStuck {
+ t.Fatalf("not stuck: %+v", m)
+ }
+ h.retry(t)
+ if m := h.read(t, p1Host).Move; m.State != provision.MoveRollingBack || m.RecoveryRetryEpoch != 1 {
+ t.Fatalf("after the retry: %+v", m)
+ }
+ h.srcPass(t)
+ h.srcPass(t)
+ if n := countEvent(h.sEvents, "retry"); n != 1 {
+ t.Fatalf("retry events %d, want one: %v", n, h.sEvents)
+ }
+ })
+
+ t.Run("a revert", func(t *testing.T) {
+ h := newRevertHarness(t)
+ h.fromStopped(t)
+ h.arm(t)
+ if err := os.WriteFile(filepath.Join(h.cfg.DataDir, p1Host, "upload.txt"), []byte("changed while stopped"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ h.pass(t)
+ if m := h.move(t); m.State != provision.MoveStuck {
+ t.Fatalf("not stuck: %+v", m)
+ }
+ g := provision.MoveGuard{ID: p1ID, Incarnation: h.inc}
+ if _, err := provision.SetRecordMove(context.Background(), h.s, "bab", "app", p1Host, provision.MoveRetryRecovery(g)); err != nil {
+ t.Fatal(err)
+ }
+ if m := h.move(t); m.State != provision.MoveReverting || m.RecoveryRetryEpoch != 1 {
+ t.Fatalf("after the retry: %+v", m)
+ }
+ h.pass(t)
+ h.pass(t)
+ if n := countEvent(h.events, "retry"); n != 1 {
+ t.Fatalf("retry events %d, want one: %v", n, h.events)
+ }
+ })
+}