amadan

carlos / platform Public

host: a claims-on box releases the offline open and revert through today's ReleaseHold

Task 7 fix round 1, item 1. offlineRelease dispatches: with no judge
(claims on) the open and the revert release through moveCfg.ReleaseHold,
the row's pair untouched, so a row at an incarnation the record has since
left is released rather than wedged; with a judge, releaseObserved and
ReleaseHoldSeq with the observed pair as before. TestOfflineMoveE2E gains
the claims-on open and revert over rows at incarnation 1 under a record
rewritten to 2.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude edde2800f33d3933fd14ebf3d17aed4772c30bf3 parent 13a3671
4 files changed, +119 −23
  • internal/host/movesoffline.go +36 −11
  • internal/host/movesoffline_e2e_test.go +72 −3
  • internal/host/movesoffline_land.go +8 −9
  • internal/host/movesoffline_land_test.go +3 −0
diff --git a/internal/host/movesoffline.go b/internal/host/movesoffline.go
index aff34ded4..3052233d5 100644
--- a/internal/host/movesoffline.go
+++ b/internal/host/movesoffline.go
@@ -315,9 +315,37 @@ func offlinePair(rec provision.Record) registry.Pair {
return p
}
-// releaseObserved is an offline release under reconcileMu and the host's
-// hold lock (the caller holds both): the record is observed afresh through
-// the box's judge, and the row is released for the move only while that
+// errNoOfflineRelease is an offline release on a box with no registry to
+// release through.
+var errNoOfflineRelease = errors.New("offline move: no registry to release the hold through")
+
+// offlineRelease is the offline open's and revert's release, under
+// reconcileMu and the host's hold lock (the caller holds both). With no
+// judge (claims on) it is today's release exactly: moveCfg.ReleaseHold under
+// the move's op, the row's pair untouched, so a row at an incarnation the
+// record has since left is released rather than wedged. With a judge it is
+// releaseObserved. true means the release ran (the caller re-reads the row);
+// false with no error means this pass skips it.
+func (w *movesWorker) offlineRelease(ctx context.Context, s release.Store, rec provision.Record, want string) (bool, error) {
+ cfg := w.cfg
+ if cfg.Judge == nil {
+ if cfg.ReleaseHold == nil {
+ return false, errNoOfflineRelease
+ }
+ if _, err := cfg.ReleaseHold(rec.Host, rec.Move.ID); err != nil {
+ return false, err
+ }
+ return true, nil
+ }
+ if cfg.ReleaseHoldSeq == nil {
+ return false, errNoOfflineRelease
+ }
+ return w.releaseObserved(ctx, s, rec, want)
+}
+
+// releaseObserved is an offline release under a judge, under reconcileMu and
+// the host's hold lock (the caller holds both): the record is observed
+// afresh through the box's judge, and the row is released for the move only while that
// record still carries this move in state want, with the observed record's
// pair (the source's revert: the abort's; the target's open: open's). A
// record that moved on, or that is absent, stale or retired, is not
@@ -1138,19 +1166,16 @@ func (w *movesWorker) offlineSourceGone(ctx context.Context, s release.Store, re
// has finished is never released. A pass that starts after the release
// lists the record at offline-reverting, which holds nothing.
func (w *movesWorker) offlineSourceRevert(ctx context.Context, s release.Store, rec provision.Record) error {
- cfg := w.cfg
m, host := rec.Move, rec.Host
- if cfg.ReleaseHoldSeq == nil {
- return errors.New("offline move: no registry to release the hold through")
- }
- // The release names the abort's pair, from the record observed under
- // the locks (releaseObserved): a record no longer reverting this move
- // releases nothing this pass.
+ // Under a judge the release names the abort's pair, from the record
+ // observed under the locks: a record no longer reverting this move
+ // releases nothing this pass. Claims on, today's release
+ // (offlineRelease).
var released bool
var relErr error
reconcileMu.Lock()
provision.WithHostHoldLock(host, func() {
- released, relErr = w.releaseObserved(ctx, s, rec, provision.OfflineReverting)
+ released, relErr = w.offlineRelease(ctx, s, rec, provision.OfflineReverting)
})
reconcileMu.Unlock()
if relErr != nil {
diff --git a/internal/host/movesoffline_e2e_test.go b/internal/host/movesoffline_e2e_test.go
index 193eca8b3..6e5ae03e0 100644
--- a/internal/host/movesoffline_e2e_test.go
+++ b/internal/host/movesoffline_e2e_test.go
@@ -315,9 +315,12 @@ func newE2EWorld(t *testing.T, opts landOpts) *e2eWorld {
if err := provision.PutManifest(ctx, rs, wd.acct, "app", provision.Manifest{Schema: 1, App: "app", Domain: "example.com"}); err != nil {
t.Fatal(err)
}
- if opts.lifetime {
+ switch {
+ case opts.lifetime:
wd.createLifetime(offHost, 1)
- } else {
+ case opts.incarnation > 0:
+ wd.createAt(offHost, opts.incarnation)
+ default:
wd.create(offHost)
}
@@ -1086,6 +1089,10 @@ func TestOfflineMoveE2E(t *testing.T) {
t.Run("done, through a lifetime", func(t *testing.T) { offlineMoveDone(t, landOpts{lifetime: true}) })
t.Run("aborted from landed, through a lifetime", offlineMoveAbortedFromLanded)
t.Run("a release skips a record that moved on", offlineReleaseSkipsAMovedRecord)
+ // Task 7 fix round 1: a claims-on box (no judge) releases through
+ // today's ReleaseHold, whatever incarnation its held rows carry.
+ t.Run("claims on, open over a row of an older incarnation", func(t *testing.T) { offlineClaimsOnOlderRow(t, false) })
+ t.Run("claims on, revert over a row of an older incarnation", func(t *testing.T) { offlineClaimsOnOlderRow(t, true) })
// Harness Run 9, finding 7: the doors driven with nothing between them
// but what production runs (each door's serial bump, the boxes' own
// nudges and the offline re-assert loop), first back to back, then with
@@ -1566,8 +1573,30 @@ func (wd *offWorld) createLifetime(host string, inc uint64) {
provision.HostIncarnation{Account: wd.acct, App: "app", Incarnation: inc, State: provision.SlotLive, Intent: "create-e2e"}, ""); err != nil {
wd.t.Fatal(err)
}
+ wd.createAt(host, inc)
+}
+
+// createAt creates host's record carrying incarnation inc, with no slot.
+func (wd *offWorld) createAt(host string, inc uint64) {
+ wd.t.Helper()
rec := provision.Record{Schema: 1, Host: host, Slug: strings.SplitN(host, ".", 2)[0], Account: wd.acct, App: "app", Region: "", Channel: "stable", CreatedAt: wd.now, Incarnation: inc}
- if err := provision.CreateRecord(ctx, wd.s, rec); err != nil {
+ if err := provision.CreateRecord(context.Background(), wd.s, rec); err != nil {
+ wd.t.Fatal(err)
+ }
+}
+
+// reincarnate rewrites the record at incarnation inc, the move and every
+// other field kept, through the record writer: the record a claims-on box
+// reads after its hostname was recreated under rows it already holds.
+func (wd *offWorld) reincarnate(inc uint64) {
+ wd.t.Helper()
+ ctx := context.Background()
+ rec, etag, err := provision.GetRecordETag(ctx, wd.s, wd.acct, "app", offHost)
+ if err != nil {
+ wd.t.Fatal(err)
+ }
+ rec.Incarnation = inc
+ if err := provision.UpdateRecord(ctx, wd.s, *rec, etag); err != nil {
wd.t.Fatal(err)
}
}
@@ -1649,3 +1678,43 @@ func offlineReleaseSkipsAMovedRecord(t *testing.T) {
t.Fatalf("fixture: state %s", st)
}
}
+
+// requireRowAt: b's row is held by op at incarnation inc ("" is ready).
+func (w *e2eWorld) requireRowAt(b *offBox, inc uint64, op, when string) registry.Route {
+ w.t.Helper()
+ rt := b.row(offHost)
+ if rt == nil || rt.Incarnation != inc || rt.HoldOp != op || (op != "" && !registry.IsHold(rt.Status)) || (op == "" && rt.Status != "ready") {
+ w.t.Fatalf("%s: %s's row %+v, want %q at incarnation %d", when, b.name, rt, op, inc)
+ }
+ return *rt
+}
+
+// A claims-on box reads no slot and holds through SetHoldIf, so its rows
+// can carry an incarnation the record has since left (a hostname recreated
+// under rows the box still holds). The open and the revert release through
+// today's ReleaseHold, so such a row is released, never wedged.
+func offlineClaimsOnOlderRow(t *testing.T, revert bool) {
+ w := newE2EWorld(t, landOpts{incarnation: 1})
+ if w.ie.w.cfg.Judge != nil || w.uk.w.cfg.Judge != nil {
+ t.Fatal("fixture: a box has a judge")
+ }
+ w.begin()
+ w.requireRowAt(w.ie, 1, w.id, "after start")
+ w.seal()
+ w.flipAndLand()
+ w.requireRowAt(w.uk, 1, w.id, "after the landing")
+ w.wd.reincarnate(2)
+ if !revert {
+ w.accept("open", "", provision.OfflineOpen)
+ w.until("the open", nextIs("finish"))
+ w.requireRowAt(w.uk, 1, "", "after open")
+ return
+ }
+ w.accept("abort", "", provision.OfflineReverting)
+ w.until("the revert", nextIs("finish"))
+ w.requireRowAt(w.ie, 1, "", "after the revert")
+ if rt := w.uk.row(offHost); rt != nil {
+ t.Fatalf("uk-box has a row after the revert: %+v", rt)
+ }
+}
+
diff --git a/internal/host/movesoffline_land.go b/internal/host/movesoffline_land.go
index 6eaf1f6ab..3dc24e6e7 100644
--- a/internal/host/movesoffline_land.go
+++ b/internal/host/movesoffline_land.go
@@ -123,8 +123,9 @@ func (w *movesWorker) landCrash(point string) error {
// still held by the move, every landing item is ensured and verified against
// the seal once more, and the verified report is written; only when every
// item is OK is the hold released (under reconcileMu and the host's hold
-// lock, as the source's revert releases its own), with open's lifetime pair
-// from the record observed there (releaseObserved). The row then reads ready:
+// lock, as the source's revert releases its own): under a judge with open's
+// lifetime pair from the record observed there, claims on through today's
+// ReleaseHold (offlineRelease). The row then reads ready:
// ordinary adoption stages the channel's build, and the first request
// promotes moves/<host>/. Then the move's archive prefix is deleted from this
// box's bucket and the box reports opened. A pass after the release finds the
@@ -157,17 +158,15 @@ func (w *movesWorker) offlineTargetOpen(ctx context.Context, s release.Store, re
return fmt.Errorf("offline move %s: %s: not opening: %s not OK on this box", m.ID, host, strings.Join(bad, ", "))
}
items = r.Items
- if cfg.ReleaseHoldSeq == nil {
- return errors.New("offline move: no registry to release the hold through")
- }
- // The release names open's pair, from the record observed under the
- // locks (releaseObserved): a record no longer open for this move
- // releases nothing this pass.
+ // Under a judge the release names open's pair, from the record
+ // observed under the locks: a record no longer open for this move
+ // releases nothing this pass. Claims on, today's release
+ // (offlineRelease).
var released bool
var relErr error
reconcileMu.Lock()
provision.WithHostHoldLock(host, func() {
- released, relErr = w.releaseObserved(ctx, s, rec, provision.OfflineOpen)
+ released, relErr = w.offlineRelease(ctx, s, rec, provision.OfflineOpen)
})
reconcileMu.Unlock()
if relErr != nil {
diff --git a/internal/host/movesoffline_land_test.go b/internal/host/movesoffline_land_test.go
index a8dbc8507..5953ea24a 100644
--- a/internal/host/movesoffline_land_test.go
+++ b/internal/host/movesoffline_land_test.go
@@ -90,6 +90,9 @@ type landOpts struct {
// host's incarnation slot at 1, live, then the record at incarnation 1
// (the create protocol's order, self-serve moves plan 1 Task 3).
lifetime bool
+ // incarnation creates the record at that incarnation with no slot, as
+ // a claims-on box (no judge) sees one; only without judged or lifetime.
+ incarnation uint64
}
// landWorld is offHost at home (ie-a, data-ie) moving to uk (uk-box,