amadan

carlos / platform Public

host: the open is proved, its stamp inside the hold lock (Plan 3 Task 7 fix round 1, finding 3)

A replay is only a row already ready and unheld; a release that changed
nothing is re-read and accepted only when the row is ready and unheld;
anything else refuses. The release and the opened_at stamp run inside
the hold lock, in that order. The census row moves from committedTarget
to landingRun.open.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 073e1fcfa5c9c11b5110e255b99bd0ddbf2acb77 parent 55892ff
4 files changed, +72 −18
  • internal/host/movesland_commit_test.go +25 −0
  • internal/host/movesland_test.go +5 −0
  • internal/host/moveslandcommit.go +40 −16
  • internal/provision/plan9_b8_census_test.go +2 −2
diff --git a/internal/host/movesland_commit_test.go b/internal/host/movesland_commit_test.go
index c9f3d74f5..fc2c2d2e0 100644
--- a/internal/host/movesland_commit_test.go
+++ b/internal/host/movesland_commit_test.go
@@ -181,3 +181,28 @@ func TestCommitScopesAreThePlan5Helpers(t *testing.T) {
t.Fatalf("the commit's parks are not the helpers' %+v and %+v: %+v", wantSource, wantStaging, c.Obligations)
}
}
+
+// Task 7 fix round 1, finding 3: the open is proved, never assumed. An
+// unheld row that is not ready is no replay of the open, and a release that
+// matched nothing is refused: opened_at is not stamped in either case.
+func TestCommittedOpenIsProved(t *testing.T) {
+ h, _ := probedLanding(t)
+ h.route.HoldOp = ""
+ h.land(t)
+ if m := h.read(t, p1Host).Move; m.State != provision.MoveCommitted || m.OpenedAt != nil {
+ t.Fatalf("an unheld row that is not ready was taken as open: %+v", m)
+ }
+
+ h2, _ := probedLanding(t)
+ h2.releaseNoop = true
+ h2.land(t)
+ if m := h2.read(t, p1Host).Move; m.State != provision.MoveCommitted || m.OpenedAt != nil || h2.route.Status == "ready" {
+ t.Fatalf("a release that matched nothing was taken as the open: %+v %+v", m, h2.route)
+ }
+ // The release happened and the row is ready: the next pass stamps it.
+ h2.releaseNoop = false
+ h2.land(t)
+ if m := h2.read(t, p1Host).Move; m.OpenedAt == nil || h2.route.Status != "ready" {
+ t.Fatalf("the open after the refusal: %+v %+v", m, h2.route)
+ }
+}
diff --git a/internal/host/movesland_test.go b/internal/host/movesland_test.go
index 50fdde0a4..f89dd3be6 100644
--- a/internal/host/movesland_test.go
+++ b/internal/host/movesland_test.go
@@ -86,6 +86,8 @@ type landingHarness struct {
// holdProbed stops every pass at probed, before the commit (Task 6's
// tests, which end there).
holdProbed bool
+ // releaseNoop makes ReleaseHeld match nothing and change nothing.
+ releaseNoop bool
}
// fakeParkLeases refuses a second acquisition of a held location.
@@ -367,6 +369,9 @@ func (h *landingHarness) landingCfg(t *testing.T) moveCfg {
}
c.ReleaseHeld = func(host, op string) (bool, error) {
h.openLog = append(h.openLog, "release:"+h.openState(t))
+ if h.releaseNoop {
+ return false, nil
+ }
if host != p1Host || !registry.IsHold(h.route.Status) || h.route.HoldOp != op {
return false, nil
}
diff --git a/internal/host/moveslandcommit.go b/internal/host/moveslandcommit.go
index 5c995d670..04f5bb65b 100644
--- a/internal/host/moveslandcommit.go
+++ b/internal/host/moveslandcommit.go
@@ -97,13 +97,11 @@ func (w *movesWorker) committedTarget(ctx context.Context, s release.Store, rec
w.notice(rec.Host+"#open", "moves: %s: the open of %s waits: this box cannot release the hold", rec.Host, m.ID)
return nil
}
- if err := l.open(ctx); err != nil {
+ stamped, err := l.open(ctx)
+ if err != nil {
return err
}
- if m.OpenedAt == nil {
- if _, err := provision.SetRecordMove(ctx, s, rec.Account, rec.App, rec.Host, provision.MoveOpened(l.g, cfg.Now())); err != nil {
- return fmt.Errorf("the record's opened_at: %w", err)
- }
+ if stamped {
log.Printf("moves: %s: landing %s is open", rec.Host, m.ID)
nudgeSerial(ctx, s)
}
@@ -163,13 +161,16 @@ func (l *landingRun) commitScopes(c *hostclaim.Claim) (source, staging hostclaim
return moves.SourceLocationScope(*c.Holder, *m.FlippedAt), moves.StagingScope(l.rec.Host, l.g.ID, c.Holder.Incarnation, seal.Bound()), nil
}
-// open is the conditional open: under the host's hold lock, with the record
-// and the claim re-read and both committed, the probe's entry is marked open
-// and then the hold is released. A row already open is a replay.
-func (l *landingRun) open(ctx context.Context) error {
+// open is the conditional open, proved (fix round 1, finding 3): under the
+// host's hold lock, with the record and the claim re-read and both
+// committed, the probe's entry is marked open, the hold released, and then
+// opened_at stamped, in that order. A replay is only a row already ready and
+// unheld; a release that changed nothing is re-read and accepted only when
+// the row is ready and unheld; anything else refuses and stamps nothing.
+// stamped reports that this call wrote opened_at.
+func (l *landingRun) open(ctx context.Context) (stamped bool, err error) {
cfg := l.cfg()
host, id := l.rec.Host, l.g.ID
- var err error
provision.WithHostHoldLock(host, func() {
var fresh *provision.Record
if fresh, err = provision.GetRecord(ctx, l.s, l.rec.Account, l.rec.App, host); err != nil {
@@ -195,19 +196,42 @@ func (l *landingRun) open(ctx context.Context) error {
case rt == nil:
err = fmt.Errorf("move %s: this box has no row to open", id)
return
+ case rt.HoldOp == "" && rt.Status == "ready":
+ // open already: a replay
case rt.HoldOp == "":
- return // open already
+ err = fmt.Errorf("move %s: the row is unheld but %q, not open", id, rt.Status)
+ return
case rt.HoldOp != id:
err = fmt.Errorf("move %s: the row is held %s by %q", id, rt.Status, rt.HoldOp)
return
+ default:
+ if cfg.MarkProbeOpen != nil && !cfg.MarkProbeOpen(host, id) {
+ log.Printf("moves: %s: no live probe entry for %s; the next request wakes it under the commit", host, id)
+ }
+ var released bool
+ if released, err = cfg.ReleaseHeld(host, id); err != nil {
+ return
+ }
+ if !released {
+ if rt, err = cfg.Route(host); err != nil {
+ return
+ }
+ if rt == nil || rt.Status != "ready" || rt.HoldOp != "" {
+ err = fmt.Errorf("move %s: the release matched nothing and the row is not open", id)
+ return
+ }
+ }
}
- if cfg.MarkProbeOpen != nil && !cfg.MarkProbeOpen(host, id) {
- log.Printf("moves: %s: no live probe entry for %s; the next request wakes it under the commit", host, id)
+ if fresh.Move.OpenedAt == nil {
+ if _, err = provision.SetRecordMove(ctx, l.s, l.rec.Account, l.rec.App, host, provision.MoveOpened(l.g, cfg.Now())); err != nil {
+ err = fmt.Errorf("the record's opened_at: %w", err)
+ return
+ }
+ stamped = true
}
- _, err = cfg.ReleaseHeld(host, id)
})
if err != nil {
- return fmt.Errorf("open the row: %w", err)
+ return false, fmt.Errorf("open the row: %w", err)
}
- return nil
+ return stamped, nil
}
diff --git a/internal/provision/plan9_b8_census_test.go b/internal/provision/plan9_b8_census_test.go
index 3ab49b530..f97435cd4 100644
--- a/internal/provision/plan9_b8_census_test.go
+++ b/internal/provision/plan9_b8_census_test.go
@@ -503,8 +503,8 @@ var plan9B8CensusInventory = []censusEntry{
{"internal/host/movesinstall.go", "recoveryInstallSource", "record-writer", "key:SetRecordMove", "b4bb0798a032c1599b9063a165656ab76c00b6ea349cb17b5f6a95769432bd5f"},
// Commits the landing through SetRecordMove (MoveCommit), its refusal the rollback mark.
{"internal/host/moveslandcommit.go", "landingRun.probed", "record-writer", "key:SetRecordMove", "6f961790d8fe169af68e2f70790c400d37c2388a4f62c26921f22b8b47c859de"},
- // Stamps opened_at through SetRecordMove (MoveOpened) after the target's open.
- {"internal/host/moveslandcommit.go", "movesWorker.committedTarget", "record-writer", "key:SetRecordMove", "f112b24fdc86aebcefe90f959a0d72a82aace3e0631e60ad8261ab76f7760eb8"},
+ // Stamps opened_at through SetRecordMove (MoveOpened) inside the hold lock, after the proved release.
+ {"internal/host/moveslandcommit.go", "landingRun.open", "record-writer", "key:SetRecordMove", "a61665f2f91222b44a54f9550a9500b5b60ed415bf80485db8ae286a9059c7ab"},
// Closes the rollback record through SetRecordMove; claim release and local journal parking use separate storage.
{"internal/host/movesrecover.go", "recoverRun.finalize", "record-writer", "key:SetRecordMove", "fff2e28c78db68bccb16da6ab70ca877b51243b0aa56b9cb2c513c6f5660f7f7"},
// Advances the source recovery phase through SetRecordMove and its sequenced writer.