carlos / platform Public

console: admission needs lifetimes on; the prepare's order and the cancel of a returning move

Fix round 1, Important 2: selfServeAdmits is the switch on a console that
has turned lifetimes on, so a prepare before the cutover is MV-O198 (self-
serve moves are not enabled on this console) rather than a settling 409
that never clears; the settling sentence stays the migration's own
transient. The gate and pass tests read admission with lifetimes on.

Minors: the source's facts judge freshness and the lifetime first (has not
reported recently), then OverCap, then Known and Truncated; the record's
checks (class, run, move) come before the journal's read, which now has
its own helper (selfServeOpenRow) for the prepare and the estimate; a
cancel of a row reverting without a cancellation of its own is the
confirmed 409, not no move in flight.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude 2 days ago eaa0bd64e8c5e97885a1c50f4b4444a8476dcc16 parent 9d50876

4 files changed, +71 −49

internal/console/selfserve_door.go +51 −41
internal/console/selfserve_gate.go +4 −2
internal/console/selfserve_gate_test.go +14 −4
internal/console/selfserve_pass_test.go +2 −2

diff --git a/internal/console/selfserve_door.go b/internal/console/selfserve_door.go

index 0bc22d690..0f0257aa9 100644

--- a/internal/console/selfserve_door.go

+++ b/internal/console/selfserve_door.go

@@ -95,10 +95,11 @@ func selfServeInFlight(rec *provision.Record, open *SelfServeMove) *offlineRefus

return &offlineRefusal{status: http.StatusConflict, msg: fmt.Sprintf("a move to %s is already %s", open.To, selfServeRowState(open))}

}

-// selfServeBusy is the prepare's checks of the record and the journal, in

-// order: the record's class (recordEligibility, main's sentences), an open

-// run, a move the record carries, an open row.

-func selfServeBusy(rec *provision.Record, open *SelfServeMove) *offlineRefusal {

+// selfServeBusy is the prepare's checks of the record, in order: the

+// record's class (recordEligibility, main's sentences), an open run, a move

+// the record carries. The open row is checked after it, from the journal

+// (selfServeOpenRow).

+func selfServeBusy(rec *provision.Record) *offlineRefusal {

if ref := recordEligibility(rec, mainMoveCopy()); ref != nil {

return ref

}

@@ -106,12 +107,29 @@ func selfServeBusy(rec *provision.Record, open *SelfServeMove) *offlineRefusal {

// provision.MoveRunOpenError's sentence, the run family's 409.

return &offlineRefusal{status: http.StatusConflict, msg: fmt.Sprintf("a run is open on this instance: %s. wait for it to finish or cancel it, then move the instance", rec.ActiveRun.ID)}

}

- if rec.Move != nil || open != nil {

- return selfServeInFlight(rec, open)

+ if rec.Move != nil {

+ return selfServeInFlight(rec, nil)

}

return nil

}

+// selfServeOpenRow reads the instance's open row after selfServeBusy: a

+// read that fails is the 502, an open row is the in-flight 409. It answers

+// w itself and reports false once it has.

+func (s *Server) selfServeOpenRow(w http.ResponseWriter, acct, app, host string) bool {

+ open, _, err := s.store.SelfServeMoveForHost(acct, app, host)

+ if err != nil {

+ log.Printf("console: self-serve move %s/%s/%s: the rows: %v", acct, app, host, err)

+ jsonError(w, http.StatusBadGateway, msgSelfServeReadMove)

+ return false

+ }

+ if open != nil {

+ writeRefusal(w, selfServeInFlight(nil, open))

+ return false

+ }

+ return true

// selfServePrepared is what the prepare's checks found: the participants

// and the stage target, and the source's facts for the host.

type selfServePrepared struct {

@@ -158,33 +176,30 @@ func containsString(list []string, v string) bool {

// selfServeSourceFacts judges the source box's facts for host from its

// status report, which pinning has already found current. In order: no

-// facts for the host (the walk did not run) cannot be measured; over the

-// cap is final, even on a walk that was cut; a walk that did not finish

-// cannot be measured; facts of another lifetime, or collected more than ten

-// minutes ago or more than a minute ahead, are a box that has not reported

-// for this host recently; a walk cut short cannot be measured.

+// facts for the host (the walk did not run) cannot be measured; facts of

+// another lifetime, or collected more than ten minutes ago or more than a

+// minute ahead, are a box that has not reported for this host recently;

+// over the cap is final, even on a walk that was cut; a walk that did not

+// finish, or was cut short, cannot be measured.

func selfServeSourceFacts(m *status.BoxMetrics, host string, inc uint64, now time.Time) (status.HostFacts, *offlineRefusal) {

var f status.HostFacts

ok := false

if m != nil {

f, ok = m.Hosts[host]

}

- switch {

- case !ok:

- return f, &offlineRefusal{status: http.StatusConflict, msg: msgSelfServeUnmeasured}

- case f.OverCap:

- return f, &offlineRefusal{status: http.StatusConflict, msg: msgSelfServeTooLarge}

- case !f.Known:

+ if !ok {

return f, &offlineRefusal{status: http.StatusConflict, msg: msgSelfServeUnmeasured}

}

fresh := func(at int64) bool {

age := now.Sub(time.Unix(at, 0))

return at > 0 && age <= selfServeFactsFresh && age >= -selfServeFactsSkew

}

- if f.Incarnation != inc || !fresh(f.CollectedAt) || !fresh(f.ReplicaCollectedAt) {

+ switch {

+ case f.Incarnation != inc || !fresh(f.CollectedAt) || !fresh(f.ReplicaCollectedAt):

return f, &offlineRefusal{status: http.StatusConflict, msg: msgSelfServeSourceStale}

- }

- if f.Truncated {

+ case f.OverCap:

+ return f, &offlineRefusal{status: http.StatusConflict, msg: msgSelfServeTooLarge}

+ case !f.Known, f.Truncated:

return f, &offlineRefusal{status: http.StatusConflict, msg: msgSelfServeUnmeasured}

}

return f, nil

@@ -243,18 +258,11 @@ func (s *Server) selfServeMoveRequest(w http.ResponseWriter, r *http.Request) {

return

}

rec := obs.Rec

- if ref := recordEligibility(rec, mainMoveCopy()); ref != nil {

+ if ref := selfServeBusy(rec); ref != nil {

writeRefusal(w, ref)

return

}

- open, _, err := s.store.SelfServeMoveForHost(acct.Sqid, app, host)

- if err != nil {

- log.Printf("console: self-serve move %s/%s/%s: the rows: %v", acct.Sqid, app, host, err)

- jsonError(w, http.StatusBadGateway, msgSelfServeReadMove)

- return

- }

- if ref := selfServeBusy(rec, open); ref != nil {

- writeRefusal(w, ref)

+ if !s.selfServeOpenRow(w, acct.Sqid, app, host) {

return

}

var req struct {

@@ -281,12 +289,14 @@ func (s *Server) selfServeMoveRequest(w http.ResponseWriter, r *http.Request) {

}

// The lifetime: the row is bound to the record's incarnation, so a

// legacy record is migrated first, under the app and hostname locks and

- // bound to the observation the checks ran on. A record admitted before

- // the cutover has no lifetime to migrate into: it is settling until the

- // console turns lifetimes on.

+ // bound to the observation the checks ran on. Admission needs lifetimes

+ // on (selfServeAdmits), and the cutover drains every unit admitted before

+ // it, so this unit is one admitted on; one that is not has no lifetime to

+ // migrate into and is refused as admission is. The settling 409 below is

+ // the migration's own transient (a conflict, another operation pending).

if rec.Incarnation == 0 {

if !lifetimesUnit(ctx) {

- jsonError(w, http.StatusConflict, msgLifetimeSettling)

+ jsonError(w, http.StatusConflict, msgSelfServeNotEnabled)

return

}

selfServeDoorPause("migrate")

@@ -307,7 +317,7 @@ func (s *Server) selfServeMoveRequest(w http.ResponseWriter, r *http.Request) {

return

}

rec = mrec

- if ref := selfServeBusy(rec, nil); ref != nil {

+ if ref := selfServeBusy(rec); ref != nil {

writeRefusal(w, ref)

return

}

@@ -459,7 +469,10 @@ func (s *Server) selfServeMoveAbort(w http.ResponseWriter, r *http.Request) {

}

switch {

case !row.AbortRequestedAt.IsZero():

- case !row.CutRequestedAt.IsZero() || selfServeCutPhase(row.Phase):

+ case !row.CutRequestedAt.IsZero() || selfServeCutPhase(row.Phase) || row.Phase == ssReverting:

+ // A move reverting with no cancellation of its own is being

+ // returned by the job (an expiry or a failed fact): no cancel

+ // can change that.

jsonError(w, http.StatusConflict, msgSelfServeConfirmed)

return

default:

@@ -517,14 +530,11 @@ func (s *Server) selfServeAnswer(w http.ResponseWriter, r *http.Request, acct *A

// prepare's checks for a move not yet requested, through the lane, then the

// estimate. It writes and migrates nothing.

func (s *Server) selfServeEstimate(w http.ResponseWriter, r *http.Request, acct *Account, app, host string, rec *provision.Record) {

- open, _, err := s.store.SelfServeMoveForHost(acct.Sqid, app, host)

- if err != nil {

- log.Printf("console: self-serve move %s/%s/%s: the rows: %v", acct.Sqid, app, host, err)

- jsonError(w, http.StatusBadGateway, msgSelfServeReadMove)

+ if ref := selfServeBusy(rec); ref != nil {

+ writeRefusal(w, ref)

return

}

- if ref := selfServeBusy(rec, open); ref != nil {

- writeRefusal(w, ref)

+ if !s.selfServeOpenRow(w, acct.Sqid, app, host) {

return

}

prep, ref := s.selfServePrepareChecks(r.Context(), acct.Sqid, app, host, rec, r.URL.Query().Get("region"))

diff --git a/internal/console/selfserve_gate.go b/internal/console/selfserve_gate.go

index ea714b159..dc7d714e3 100644

--- a/internal/console/selfserve_gate.go

+++ b/internal/console/selfserve_gate.go

@@ -34,9 +34,11 @@ func (s *Server) selfServeMode() bool {

}

// selfServeAdmits reports whether a new self-serve move may be admitted (a

-// new prepare): the switch on. Recovery mode alone admits nothing.

+// new prepare): the switch on, on a console that has turned lifetimes on (a

+// self-serve row is bound to the record's incarnation, which a record has

+// only from the cutover on). Recovery mode alone admits nothing.

func (s *Server) selfServeAdmits() bool {

- return s.cfg.SelfServeSwitchOn()

+ return s.cfg.SelfServeSwitchOn() && s.lifetimesOn()

}

// moveCapable reports whether p may ask for a self-serve move of acct's app:

diff --git a/internal/console/selfserve_gate_test.go b/internal/console/selfserve_gate_test.go

index b5326d29b..adffa1a29 100644

--- a/internal/console/selfserve_gate_test.go

+++ b/internal/console/selfserve_gate_test.go

@@ -431,14 +431,16 @@ func TestMoveCapableGateAtTheRoutes(t *testing.T) {

if got := s.selfServeMode(); got != c.want {

t.Errorf("selfServeMode(%q, claims %v, phase 1 %v) = %v, want %v", c.sw, c.hostClaim, c.phase, got, c.want)

}

- // Admission is the switch alone.

- if got := s.selfServeAdmits(); got != c.want {

- t.Errorf("selfServeAdmits(%q, claims %v, phase 1 %v) = %v, want %v", c.sw, c.hostClaim, c.phase, got, c.want)

+ // Admission needs lifetimes on as well: a console with no

+ // cutover gate admits nothing (the switch with lifetimes on is

+ // checked below, once the scene's console has cut over).

+ if got := s.selfServeAdmits(); got {

+ t.Errorf("selfServeAdmits(%q, claims %v, phase 1 %v) = true with lifetimes off", c.sw, c.hostClaim, c.phase)

}

// Recovery mode is self-serve mode whatever the switch, and

// admits nothing new.

s.selfServeRecovery.Store(true)

- if !s.selfServeMode() || s.selfServeAdmits() != c.want {

+ if !s.selfServeMode() || s.selfServeAdmits() {

t.Errorf("recovery mode (%q, claims %v, phase 1 %v): mode %v admits %v, want true %v",

c.sw, c.hostClaim, c.phase, s.selfServeMode(), s.selfServeAdmits(), c.want)

}

@@ -488,6 +490,14 @@ func TestMoveCapableGateAtTheRoutes(t *testing.T) {

if !fx.srv.lifetimesOn() {

t.Fatal("lifetimes are not on")

}

+ // With lifetimes on, admission is the switch.

+ for _, sw := range []string{"offline", ""} {

+ fx.srv.cfg.SelfServeMoves = sw

+ if got := fx.srv.selfServeAdmits(); got != (sw == "offline") {

+ t.Errorf("selfServeAdmits with lifetimes on and the switch %q = %v", sw, got)

+ }

+ fx.srv.cfg.SelfServeMoves = "offline"

// From here every bucket call and every statement is counted.

counted, call := sc.counting(t)

diff --git a/internal/console/selfserve_pass_test.go b/internal/console/selfserve_pass_test.go

index 214ed41bb..3f6649939 100644

--- a/internal/console/selfserve_pass_test.go

+++ b/internal/console/selfserve_pass_test.go

@@ -375,8 +375,8 @@ func TestPassStartsAndRetriesOnReadFailure(t *testing.T) {

t.Run("the switch on runs the pass with an empty journal", func(t *testing.T) {

fx, rel := ssPassFixture(t)

fx.srv.cfg.SelfServeMoves = "offline"

- if !fx.srv.selfServeMode() || !fx.srv.selfServeAdmits() {

- t.Fatal("the switch on is not self-serve mode with admission")

+ if !fx.srv.selfServeMode() || fx.srv.selfServeAdmits() != fx.srv.lifetimesOn() {

+ t.Fatal("the switch on is not self-serve mode, admitting once lifetimes are on")

}

fx.srv.selfServeIteration(ctx)

if _, ok := ssLeaseBody(t, rel); !ok {