carlos / platform Public

hostbox: reconcile asks the fence observer only when the row decision depends on it, the source in releasing, so a relocation target with no local record mints its row through Install

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude agent, operated by paul@keymail.dev b3381d24dcb20782e5e284cb7269f67fddb9a187 parent ef75033
2 files changed, +99 −12
  • internal/hostbox/reconcile.go +25 −11
  • internal/hostbox/reconcile_relocation_test.go +74 −1
diff --git a/internal/hostbox/reconcile.go b/internal/hostbox/reconcile.go
index 409cee474..2b308aa01 100644
--- a/internal/hostbox/reconcile.go
+++ b/internal/hostbox/reconcile.go
@@ -32,7 +32,8 @@ type Reconciler struct {
Adopt func(context.Context, RecordSelection) (hostclaim.Observed, error)
// FenceClosed reports whether this box's admission fence for the host
// is closed on the claim given (region HA §5.3: the source in
- // releasing is ready until then). It is called under the host lock
+ // releasing is ready until then). It is asked only where the row
+ // decision depends on it (rowDecision). It is called under the host lock
// and must not take it (hostguard.Guard.AdmissionFenced). nil: never
// closed, today's behaviour. An error keeps rows and mints none (C6).
FenceClosed func(context.Context, hostclaim.Observed) (bool, error)
@@ -60,13 +61,25 @@ func (r Reconciler) fenceClosed(ctx context.Context, obs hostclaim.Observed) (bo
return r.FenceClosed(ctx, obs)
}
-// rowDecision is §5.3's decision for this box on a fresh read.
-func (r Reconciler) rowDecision(ctx context.Context, obs hostclaim.Observed) (RowDecision, error) {
- closed, err := r.fenceClosed(ctx, obs)
+// rowDecision is §5.3's decision for this box on a fresh read. The fence
+// observer is asked only when the decision depends on its answer, which
+// is the source in releasing alone: every other relocating box, a target
+// before it has a local record included, decides without it. closed is
+// the observer's answer, false when it was not asked.
+func (r Reconciler) rowDecision(ctx context.Context, obs hostclaim.Observed) (d RowDecision, closed bool, err error) {
+ open := RelocationRow(obs.Claim, r.Box, true, false)
+ shut := RelocationRow(obs.Claim, r.Box, true, true)
+ if open == shut {
+ return open, false, nil
+ }
+ closed, err = r.fenceClosed(ctx, obs)
if err != nil {
- return RowDecision{}, err
+ return RowDecision{}, false, err
}
- return RelocationRow(obs.Claim, r.Box, true, closed), nil
+ if closed {
+ return shut, true, nil
+ }
+ return open, false, nil
}
type activationReader struct {
@@ -100,7 +113,7 @@ func (r Reconciler) check(ctx context.Context, obs hostclaim.Observed, selected
return err
}
} else {
- d, err := r.rowDecision(ctx, obs)
+ d, _, err := r.rowDecision(ctx, obs)
if err != nil {
return err
}
@@ -151,12 +164,13 @@ func (r Reconciler) Install(ctx context.Context, selected RecordSelection, write
return nil
}
if r.relocates(obs.Claim) {
- // One answer from the observer, for the decision and the path.
- closed, err := r.fenceClosed(ctx, obs)
+ // One answer from the observer, for the decision and the path,
+ // asked only when the decision depends on it.
+ d, closed, err := r.rowDecision(ctx, obs)
if err != nil {
return err
}
- if d := RelocationRow(obs.Claim, r.Box, true, closed); d.Case == RowHeld && (obs.Claim.Op.Exec != r.Box || closed) {
+ if d.Case == RowHeld && (obs.Claim.Op.Exec != r.Box || closed) {
// No installation of ours to activate, or one whose admission
// fence is closed (the source in releasing, still op.exec,
// which With refuses): a kept row's hold only, under the
@@ -215,7 +229,7 @@ func (r Reconciler) holdOnly(ctx context.Context, selected RecordSelection, d Ro
if fresh.Absent || fresh.Claim == nil || fresh.Claim.Holder == nil || fresh.Claim.Holder.Incarnation != selected.Incarnation || !r.relocates(fresh.Claim) {
return ErrChanged
}
- again, err := r.rowDecision(ctx, fresh)
+ again, _, err := r.rowDecision(ctx, fresh)
if err != nil {
return err
}
diff --git a/internal/hostbox/reconcile_relocation_test.go b/internal/hostbox/reconcile_relocation_test.go
index 3c3bd1965..7c8e36d6c 100644
--- a/internal/hostbox/reconcile_relocation_test.go
+++ b/internal/hostbox/reconcile_relocation_test.go
@@ -225,7 +225,11 @@ func TestReconcileWithAFenceObserverLeavesMovesToMain(t *testing.T) {
}
// Install asks the fence observer once for its own decision: the hold-only
-// path acts on the answer the decision was made from.
+// path acts on the answer the decision was made from. The source in
+// releasing is the one row case that depends on the observer, so it is
+// asked here three times (check, Install, holdOnly), as before the final
+// review's fix; a box whose case does not depend on it is never asked
+// (TestReconcileRelocationTargetWithNoLocalRecordMintsItsRow).
func TestReconcileInstallAsksTheFenceObserverOnce(t *testing.T) {
r, selected, holds, _ := relocationReconciler(t)
relocate(t, r, selected.Host, hostclaim.PhaseReleasing)
@@ -252,6 +256,75 @@ func TestReconcileInstallAsksTheFenceObserverOnce(t *testing.T) {
}
}
+// targetReconciler is relocationReconciler moved onto the target "dst":
+// its own guard directory, with no local record for the host, and the
+// real fence observer over it, which errors on a box with no record.
+func targetReconciler(t *testing.T, r Reconciler) (Reconciler, hostguard.Guard, *int) {
+ t.Helper()
+ dir := t.TempDir()
+ locks := filepath.Join(dir, "locks")
+ for _, p := range []string{locks, filepath.Join(dir, "hostguard")} {
+ if err := os.Mkdir(p, 0770); err != nil {
+ t.Fatal(err)
+ }
+ }
+ guard := hostguard.Guard{DataDir: dir, LockDir: locks, Claims: hostguard.StoreReader{Store: r.Claims}}
+ target := WorkSource{Guard: guard, Box: "dst", Region: "ie", Now: r.Now,
+ Policy: Policy{Held: func(context.Context, string) (string, error) { return "", nil }, Relocation: RelocationPolicy("dst")}.Check,
+ Local: func(context.Context, *hostclaim.Claim, Selection) error { return nil }}
+ r.Box, r.Guard, r.Writes.Selector = "dst", guard, target
+ r.Local = park.Local{Guard: guard, Claims: r.Claims, DataDir: dir, Box: "dst"}
+ calls := new(int)
+ r.FenceClosed = func(_ context.Context, o hostclaim.Observed) (bool, error) {
+ *calls++
+ captured, err := hostguard.Capture(dir, o.Host)
+ if err != nil {
+ return false, err
+ }
+ return guard.AdmissionFenced(o.Host, o.Claim, captured)
+ }
+ return r, guard, calls
+}
+
+// Final review Important 2: with region HA on, a relocation target that
+// has no local guard record yet mints its row through Install. Its row
+// case (held, as op.exec and a participant) does not depend on the fence,
+// so the observer, which errors without a record, is never asked; the
+// guarded path activates the installation and runs the write.
+func TestReconcileRelocationTargetWithNoLocalRecordMintsItsRow(t *testing.T) {
+ r, selected, holds, _ := relocationReconciler(t)
+ r, guard, calls := targetReconciler(t, r)
+ ctx := context.Background()
+ obs, err := hostclaim.Read(ctx, r.Claims, selected.Host)
+ if err != nil {
+ t.Fatal(err)
+ }
+ obs, err = hostclaim.Update(ctx, r.Claims, obs, func(c *hostclaim.Claim) error {
+ c.Op = hostclaim.Op{Kind: hostclaim.KindRelocating, ID: "rel-1", Phase: hostclaim.PhaseInstalling, Exec: "dst", Participants: []string{"box", "dst"},
+ Intent: map[string]string{"region": "ie", "source_box": "box", "target_box": "dst"}, Journal: map[string]string{"exec_ack.box": "{}", "exec_ack.dst": "{}"}}
+ return nil
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ // The production observer cannot answer here: no record.
+ if _, err := r.FenceClosed(ctx, obs); err == nil {
+ t.Fatal("the observer answered for a box with no local record")
+ }
+ *calls = 0
+ writes := 0
+ if err := r.Install(ctx, selected, func() error { writes++; return nil }); err != nil {
+ t.Fatalf("the target's first install: %v", err)
+ }
+ if writes != 1 || len(*holds) != 0 || *calls != 0 {
+ t.Fatalf("writes %d holds %v observer calls %d", writes, *holds, *calls)
+ }
+ captured, err := hostguard.Capture(guard.DataDir, selected.Host)
+ if err != nil || captured.Stamp != obs.Claim.Holder.Stamp {
+ t.Fatalf("the target's installation was not activated: %+v %v", captured, err)
+ }
+}
+
// snapshotDir is every regular file under dir, by relative path.
func snapshotDir(t *testing.T, dir string) map[string]string {
t.Helper()