moves cut: the scoped-holder refusal ends the step even when its write conflicts
Fix round 1, ruling on concern 2. refuseHolder (ruling T5-lease) had the fall-through refuseUnarmed had: it returned the transition's result, so a refusal whose write lost to another writer (ErrMoveConflict, answered (false, nil)) let requested go on to the admission and admitted to the cut guard and the step to stopping. It now returns the refusal and the write's outcome apart, and both callers end the step on a refusal. A "holder-refusal" fault point before the write lets the test inject the conflicting writer. TestCutHolderRefusalEndsTheStepOnAConflict: at requested and at admitted, no later step, no admission, no lease beyond the requested round's own, no stop and no seal.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+78
−10
internal/host/movescut.go+18 −10internal/host/movescutguard_test.go+60 −0
diff --git a/internal/host/movescut.go b/internal/host/movescut.go| index a0bbcb165..3251bc138 100644 |
| --- a/internal/host/movescut.go |
| +++ b/internal/host/movescut.go |
| @@ -240,8 +240,8 @@ func (c *cutRun) requested(ctx context.Context) (bool, error) { |
| } |
| return c.backToReady(ctx, "the inputs pinned at readiness have changed") |
| } |
| - if refused, err := c.refuseHolder(ctx); refused || err != nil { |
| - return refused, err |
| + if refused, wrote, err := c.refuseHolder(ctx); refused || err != nil { |
| + return wrote, err |
| } |
| now := cfg.Now() |
| budget := target.CutBudgetS |
| @@ -261,8 +261,8 @@ func (c *cutRun) admitted(ctx context.Context) (bool, error) { |
| if !same { |
| return c.backToReady(ctx, "the inputs pinned at readiness have changed") |
| } |
| - if refused, err := c.refuseHolder(ctx); refused || err != nil { |
| - return refused, err |
| + if refused, wrote, err := c.refuseHolder(ctx); refused || err != nil { |
| + return wrote, err |
| } |
| if refused, wrote, err := c.refuseUnarmed(ctx); refused || err != nil { |
| return wrote, err |
| @@ -321,20 +321,28 @@ func (c *cutRun) refuseUnarmed(ctx context.Context) (refused, wrote bool, err er |
| // ready with RefusedReplicaLayout recorded (ruling T5-lease). The |
| // refusal stands as long as the claim does: a passing round pins again, |
| // the next cut is refused the same way, and the operator reads why. |
| -func (c *cutRun) refuseHolder(ctx context.Context) (bool, error) { |
| +// |
| +// refused and wrote are apart, as in refuseUnarmed (fix round 1): a |
| +// refused cut ends the step whether or not the refusal's record write |
| +// landed. A write another writer beat (ErrMoveConflict) writes nothing, |
| +// the pass ends, and the next pass reads the record again. |
| +func (c *cutRun) refuseHolder(ctx context.Context) (refused, wrote bool, err error) { |
| obs, err := c.claim(ctx) |
| if err != nil { |
| - return false, err |
| + return false, false, err |
| } |
| if err := c.holderSupported(*obs.Claim.Holder); err != nil { |
| log.Printf("moves: %s: cut %s refused: %v", c.rec.Host, c.g.ID, err) |
| - ok, err := c.cutTransition(ctx, "the refusal", provision.MoveBackToReadyRefused(c.g, provision.RefusedReplicaLayout)) |
| - if ok { |
| + if err := c.fault("holder-refusal"); err != nil { |
| + return true, false, err |
| + } |
| + wrote, err := c.cutTransition(ctx, "the refusal", provision.MoveBackToReadyRefused(c.g, provision.RefusedReplicaLayout)) |
| + if wrote { |
| nudgeSerial(ctx, c.s) |
| } |
| - return ok, err |
| + return true, wrote, err |
| } |
| - return false, nil |
| + return false, false, nil |
| } |
| // inputsHold compares the record's pin with the current inputs. |
diff --git a/internal/host/movescutguard_test.go b/internal/host/movescutguard_test.go| index d72d50a10..85f59a381 100644 |
| --- a/internal/host/movescutguard_test.go |
| +++ b/internal/host/movescutguard_test.go |
| @@ -399,3 +399,63 @@ func TestCutGuardRefusalEndsTheStepOnAConflict(t *testing.T) { |
| }) |
| } |
| } |
| + |
| +// Fix round 1 (ruling on concern 2): the scoped-holder refusal (ruling |
| +// T5-lease) ends the step whatever its write returns, as the cut guard's |
| +// does. Another writer takes the move back to ready just before the |
| +// refusal's write, so its CAS conflicts; the pass ends there. At admitted |
| +// the step must not go on to the cut guard or to stopping (the target is |
| +// left unarmed so that a step that carried on would reach the guard's |
| +// "refusal" point); at requested it must not go on to the admission. The |
| +// staging lease the requested round takes before the holder check is |
| +// main's order (ruling T5-lease) and is not this refusal's. |
| +func TestCutHolderRefusalEndsTheStepOnAConflict(t *testing.T) { |
| + ctx := context.Background() |
| + for _, phase := range []string{provision.CutRequested, provision.CutAdmitted} { |
| + t.Run(phase, func(t *testing.T) { |
| + h := newCutHarness(t) |
| + h.scoped = true |
| + h.ready(t) |
| + if phase == provision.CutAdmitted { |
| + h.targetPass(t, h.unarmedTarget(t, func(c *moveCfg) { c.OwnLease = nil })) |
| + } |
| + h.cut(t) |
| + g := provision.MoveGuard{ID: p1ID, Incarnation: h.inc} |
| + if phase == provision.CutAdmitted { |
| + now := h.clock.Now() |
| + if _, err := provision.SetRecordMove(ctx, h.s, "bab", "app", p1Host, provision.MoveAdmit(g, now, 120, moves.CutDeadline(now, 120), 3)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + inner, conflicted := h.cfg.cutFault, false |
| + var after []string |
| + h.cfg.cutFault = func(point string) error { |
| + if conflicted { |
| + after = append(after, point) |
| + } |
| + if point == "holder-refusal" && !conflicted { |
| + conflicted = true |
| + if _, err := provision.SetRecordMove(ctx, h.s, "bab", "app", p1Host, provision.MoveBackToReady(g)); err != nil { |
| + t.Errorf("the conflicting write: %v", err) |
| + } |
| + } |
| + return inner(point) |
| + } |
| + acquired, statuses := h.lease.acquired, len(h.statuses) |
| + h.pass(t) |
| + if !conflicted { |
| + t.Fatal("the pass never reached the holder refusal") |
| + } |
| + if len(after) != 0 { |
| + t.Fatalf("the step carried on after the conflicting refusal: %v", after) |
| + } |
| + if phase == provision.CutRequested { |
| + acquired = h.lease.acquired // the round's own, before the holder check |
| + } |
| + h.nothingStopped(t, "", acquired, statuses) |
| + if m := h.move(t); m.CutAt != nil || m.CutBudgetS != 0 { |
| + t.Fatalf("the cut was admitted: %+v", m) |
| + } |
| + }) |
| + } |
| +} |