amadan

carlos / platform Public

hostrelocate: a converted move's bound source fence that cannot be read refuses the worker's pass, never reads as not fenced

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 032db0d32377d9b6248938e14544c2415d1e5262 parent 151261f
2 files changed, +83 −36
  • internal/hostrelocate/worker.go +6 −0
  • internal/hostrelocate/worker_test.go +77 −36
diff --git a/internal/hostrelocate/worker.go b/internal/hostrelocate/worker.go
index 07615a530..0768f578b 100644
--- a/internal/hostrelocate/worker.go
+++ b/internal/hostrelocate/worker.go
@@ -312,6 +312,12 @@ func (w Worker) Step(ctx context.Context, host string) (hostclaim.Observed, erro
if err != nil {
return obs, err
}
+ if ref == nil && c.Op.Journal["fence."+src] != "" {
+ // A converted move bound the source's fence before FenceSource
+ // consumed it. A bound fence that is no longer there is not
+ // "not fenced" (absence is never evidence): refuse the pass.
+ return obs, fmt.Errorf("%w: %s's fence is bound in the journal but cannot be read", ErrRefused, src)
+ }
if ref != nil {
next, err := hostclaim.FenceSource(ctx, w.Claims, obs, id, *ref, w.Discharge(ctx, o.Roster.Boxes[src], record), w.Fences)
if err == nil {
diff --git a/internal/hostrelocate/worker_test.go b/internal/hostrelocate/worker_test.go
index 59f4e414d..514dab20d 100644
--- a/internal/hostrelocate/worker_test.go
+++ b/internal/hostrelocate/worker_test.go
@@ -710,57 +710,98 @@ func (preFlip) CheckMovingBeforeFlip(context.Context, hostclaim.Observed, string
return "ie", nil
}
-// Task 1's fix: a move converted to relocating binds fence.<source> with
-// no observation time, and JournalPin refuses until FenceSource has
-// consumed it. The worker reads the converted source as not yet lost, so
-// its first CAS is FenceSource; the pin then waits FailoverPinDelay past
-// the fence's observed_at, under the location lease.
-func TestWorkerFencesAConvertedMovesSourceBeforeItPins(t *testing.T) {
- f := newFixture(t)
+// convertedMove begins an admitted move of host from src to "far", a box
+// of another region that is not in this fixture's roster, terminates src
+// (observed ten seconds before f.now), and converts the move to
+// relocating as the moves door would. farAcks says whether the move's
+// barrier had heard from far before the source died.
+func convertedMove(f *fixture, farAcks bool) hostclaim.Observed {
+ f.t.Helper()
ctx := context.Background()
- far := strings.Repeat("9", 32) // the move's target, enrolled in uk
- o := f.step(f.claim(), hostclaim.BeginAdmittedMove("mv1", "member", "src", "far", "uk", f.now,
+ far := strings.Repeat("9", 32) // the move target's enrollment
+ f.step(f.claim(), hostclaim.BeginAdmittedMove("mv1", "member", "src", "far", "uk", f.now,
hostclaim.MoveAdmission{AppID: "app", AdmissionID: "mv1", SourceEnrollment: enrollment("src"), TargetEnrollment: far}))
- // The move's barrier, as the move door's boxes left it.
f.ack("src")
- f.participant(hostclaim.AcknowledgeExecution("far", hostclaim.ExecutionAcknowledgement{Op: "mv1", Enrollment: far, BootID: "boot", ControllerID: "ctl", LeaseETag: "lease", Expires: f.now.Add(time.Hour)}))
+ if farAcks {
+ f.participant(hostclaim.AcknowledgeExecution("far", hostclaim.ExecutionAcknowledgement{Op: "mv1", Enrollment: far, BootID: "boot", ControllerID: "ctl", LeaseETag: "lease", Expires: f.now.Add(time.Hour)}))
+ }
f.terminate("src")
f.now = f.now.Add(-time.Minute + 10*time.Second) // ten seconds after the observation
ro, err := f.w.Directory.Read(ctx)
if err != nil {
- t.Fatal(err)
+ f.t.Fatal(err)
}
ref, _, err := f.w.fence(ctx, ro, "src")
if err != nil || ref == nil {
- t.Fatalf("fence: %v %v", ref, err)
+ f.t.Fatalf("fence: %v %v", ref, err)
}
- if o, err = hostclaim.ConvertMovingToRelocating(ctx, f.s, f.claim(), "mv1", *ref, f.w.Fences, preFlip{}); err != nil {
- t.Fatal(err)
- }
- if o.Claim.Op.Journal["fence.src"] == "" || o.Claim.Op.Journal["fence.src.observed_at"] != "" || slices.Contains(o.Claim.Lost, "src") {
- t.Fatalf("not the converted shape this test is about: %+v lost %v", o.Claim.Op.Journal, o.Claim.Lost)
- }
- o = f.worker(nil)
- if o.Claim.Op.Journal["fence.src.observed_at"] == "" || !slices.Contains(o.Claim.Lost, "src") || o.Claim.Op.Phase != hostclaim.PhasePinning || o.Claim.Op.Journal["pin"] != "" {
- t.Fatalf("the worker's first CAS on a converted move is FenceSource: %+v lost %v", o.Claim.Op, o.Claim.Lost)
+ o, err := hostclaim.ConvertMovingToRelocating(ctx, f.s, f.claim(), "mv1", *ref, f.w.Fences, preFlip{})
+ if err != nil {
+ f.t.Fatal(err)
}
- if o = f.worker(nil); o.Claim.Op.Intent["target_box"] != "dst" {
- t.Fatalf("the converted move names its same-region target: %+v", o.Claim.Op.Intent)
+ if o.Claim.Op.Journal["fence.src"] == "" || o.Claim.Op.Journal["fence.src.observed_at"] != "" || slices.Contains(o.Claim.Lost, "src") || !slices.Contains(o.Claim.Op.Participants, "far") {
+ f.t.Fatalf("not the converted shape this test is about: %+v lost %v", o.Claim.Op, o.Claim.Lost)
}
- f.worker(ErrNotYet) // inside the 30-second hold
- if len(f.leases) != 0 {
- t.Fatalf("a lease was taken inside the hold: %v", f.leases)
+ return o
+}
+
+// Task 1's fix: a move converted to relocating binds fence.<source> with
+// no observation time, and JournalPin refuses until FenceSource has
+// consumed it. The worker reads the converted source as not yet lost, so
+// its first CAS is FenceSource; the pin then waits FailoverPinDelay past
+// the fence's observed_at, under the location lease. Task 5 fix round 1:
+// the move's own target leaves the barrier when the same-region target is
+// named, so one that never acknowledged (or died) holds nothing up.
+func TestWorkerFencesAConvertedMovesSourceBeforeItPins(t *testing.T) {
+ for _, farAcks := range []bool{true, false} {
+ t.Run(fmt.Sprintf("the move target acknowledged %v", farAcks), func(t *testing.T) {
+ f := newFixture(t)
+ convertedMove(f, farAcks)
+ o := f.worker(nil)
+ if o.Claim.Op.Journal["fence.src.observed_at"] == "" || !slices.Contains(o.Claim.Lost, "src") || o.Claim.Op.Phase != hostclaim.PhasePinning || o.Claim.Op.Journal["pin"] != "" {
+ t.Fatalf("the worker's first CAS on a converted move is FenceSource: %+v lost %v", o.Claim.Op, o.Claim.Lost)
+ }
+ if o = f.worker(nil); o.Claim.Op.Intent["target_box"] != "dst" || !slices.Equal(o.Claim.Op.Participants, []string{"dst"}) {
+ t.Fatalf("the converted move names its same-region target alone: %+v", o.Claim.Op)
+ }
+ f.worker(ErrNotYet) // inside the 30-second hold
+ if len(f.leases) != 0 {
+ t.Fatalf("a lease was taken inside the hold: %v", f.leases)
+ }
+ f.now = f.now.Add(20 * time.Second)
+ if o = f.worker(nil); o.Claim.Op.Journal["pin"] == "" {
+ t.Fatal("not pinned at observation plus thirty seconds")
+ }
+ if want := []string{"acquire", "pin", "release"}; !slices.Equal(f.leases, want) {
+ t.Fatalf("the pin was not selected and journaled under the location lease: %v", f.leases)
+ }
+ f.worker(ErrNotYet) // the named target has not acknowledged
+ f.ack("dst")
+ if o = f.worker(nil); o.Claim.Op.Phase != hostclaim.PhaseInstalling || o.Claim.Op.Exec != "dst" {
+ t.Fatalf("installing: %+v", o.Claim.Op)
+ }
+ })
}
- f.now = f.now.Add(20 * time.Second)
- if o = f.worker(nil); o.Claim.Op.Journal["pin"] == "" {
- t.Fatal("not pinned at observation plus thirty seconds")
+}
+
+// Task 5 fix round 1, absence is never evidence: a converted move's
+// fence.<source> is bound in the journal, so a fence object that is no
+// longer there is an error, never "not fenced". The worker refuses the
+// pass: it names no target and never calls the pinner.
+func TestWorkerRefusesAConvertedMoveWhoseBoundFenceIsMissing(t *testing.T) {
+ f := newFixture(t)
+ convertedMove(f, false)
+ if err := f.s.Delete(context.Background(), hostfence.FenceKey(enrollment("src"))); err != nil {
+ t.Fatal(err)
}
- if want := []string{"acquire", "pin", "release"}; !slices.Equal(f.leases, want) {
- t.Fatalf("the pin was not selected and journaled under the location lease: %v", f.leases)
+ before := f.claim().ETag
+ for range 3 {
+ f.now = f.now.Add(time.Minute)
+ if _, err := f.w.Step(context.Background(), host); err == nil || errors.Is(err, ErrNotYet) {
+ t.Fatalf("a missing bound fence read as no fence: %v", err)
+ }
}
- f.worker(ErrNotYet) // the named target has not acknowledged
- f.ack("dst")
- if o = f.worker(nil); o.Claim.Op.Phase != hostclaim.PhaseInstalling || o.Claim.Op.Exec != "dst" {
- t.Fatalf("installing: %+v", o.Claim.Op)
+ if o := f.claim(); o.ETag != before || o.Claim.Op.Intent["target_box"] != "" || len(f.leases) != 0 {
+ t.Fatalf("the worker acted on a missing fence: %+v leases %v", o.Claim.Op, f.leases)
}
}