amadan
zdd-readiness: a persisted book sequence orders marker clears and pre-retry outcomes (N6, retry ordering)
The failure book keeps a persisted sequence. Every hold-ran marker takes the next value (HoldRan.Seq), and every retry clear does too, in memory: carlos instances retry per host, and a promote --retry pointer clear app-wide. An execution takes a snapshot when it starts (an attempt in scheduler.execute beside Started, with an ExecID from a Manager-wide counter; a planned wake in Plan, kept per token beside its plan time). A good write clears only a marker set before its execution began (ClearHoldRanBefore; Bootstrap passes zero and clears nothing), so a delayed older publication no longer erases a newer marker (Astra R3 N6). Markers are per execution (HoldRan.Exec, plan review finding 4): another run of the same build renews the sequence. Record returns a disposition; a failure whose execution began before a retry since is PreRetry: no record change, no hold-down, no halt and no supersession (plan review finding 3, ruling 191), while a requested marker is still set. The simulation models the same ordering; mutate.sh's M3 follows the renamed line.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 files changed,
+861
−133
internal/rollout/admit_test.go+1 −1internal/rollout/bookseq_test.go+376 −0internal/rollout/failures.go+219 −24internal/rollout/failures_test.go+4 −4internal/rollout/holdran_test.go+9 −6internal/rollout/manager.go+95 −49internal/rollout/queue.go+26 −4internal/rollout/scheduler.go+27 −9internal/rollout/sim_test.go+77 −18internal/rollout/testdata/rolloutsim/mutate.sh+1 −1internal/rollout/wakeplan.go+23 −14internal/rollout/wakeplan_test.go+3 −3
diff --git a/internal/rollout/admit_test.go b/internal/rollout/admit_test.go| index dd2eb5b01..afe72fd4f 100644 |
| --- a/internal/rollout/admit_test.go |
| +++ b/internal/rollout/admit_test.go |
| @@ -116,7 +116,7 @@ type orderBook struct { |
| atRecord uint64 |
| } |
| -func (o *orderBook) Record(r FailureRecord) (bool, error) { |
| +func (o *orderBook) Record(r FailureRecord) (RecordResult, error) { |
| o.atRecord = o.q.newest(o.host) |
| return o.FailureBook.Record(r) |
| } |
diff --git a/internal/rollout/bookseq_test.go b/internal/rollout/bookseq_test.go| new file mode 100644 |
| index 000000000..dfe3bb732 |
| --- /dev/null |
| +++ b/internal/rollout/bookseq_test.go |
| @@ -0,0 +1,376 @@ |
| +package rollout |
| + |
| +import ( |
| + "context" |
| + "slices" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/activator" |
| + "github.com/carlosframework/platform/internal/appconfig" |
| +) |
| + |
| +// N6, Astra's sequence: A's good write was decided before hold B ran on the |
| +// host; B's marker is newer, and A's write must not clear it. |
| +func TestAnOlderGoodWriteNeverClearsANewerMarker(t *testing.T) { |
| + b := &FailureBook{} |
| + snapA := b.Snapshot() |
| + if err := b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v2"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if cleared, _ := b.ClearHoldRanBefore("h", snapA); cleared { |
| + t.Fatal("a write decided before the marker cleared it") |
| + } |
| + if cleared, _ := b.ClearHoldRanBefore("h", b.Snapshot()); !cleared { |
| + t.Fatal("a write decided after the marker must clear it") |
| + } |
| +} |
| + |
| +func TestAZeroSnapshotClearsNothing(t *testing.T) { |
| + b := &FailureBook{} |
| + _ = b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v2"}) |
| + if cleared, _ := b.ClearHoldRanBefore("h", 0); cleared { |
| + t.Fatal("no snapshot must clear nothing") |
| + } |
| +} |
| + |
| +// Plan review finding 4: a marker belongs to one execution, not to its |
| +// build. B's marker (execution a1) stands before C's snapshot; B then runs |
| +// again (a2) while C's good write is delayed. The second execution renews |
| +// the marker's sequence, so C's write clears nothing. A repeated report of |
| +// one execution changes nothing. |
| +func TestAnotherExecutionOfTheMarkedBuildRenewsTheMarker(t *testing.T) { |
| + now := t0 |
| + b := &FailureBook{now: func() time.Time { return now }} |
| + if err := b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v2", Exec: "a1", At: t0}); err != nil { |
| + t.Fatal(err) |
| + } |
| + snapC := b.Snapshot() |
| + second := t0.Add(time.Minute) |
| + if err := b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v2", Exec: "a2", At: second}); err != nil { |
| + t.Fatal(err) |
| + } |
| + got, _ := b.HoldRanOn("h") |
| + if got.Exec != "a2" || got.Seq < snapC || !got.At.Equal(second) { |
| + t.Fatalf("marker %+v after a second execution of v2: want a2's, sequence at least %d, stamped %v", got, snapC, second) |
| + } |
| + if cleared, _ := b.ClearHoldRanBefore("h", snapC); cleared { |
| + t.Fatal("C's good write cleared the marker of an execution that began after C's snapshot") |
| + } |
| + // The same execution reported again keeps its sequence and its stamp. |
| + _ = b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v2", Exec: "a2", At: second.Add(time.Minute)}) |
| + if again, _ := b.HoldRanOn("h"); again != got { |
| + t.Fatalf("marker %+v after a repeated report of a2, want it unchanged at %+v", again, got) |
| + } |
| + // A late report of an earlier execution of the same build never moves |
| + // the stamp back: a rollback signed between the two runs predates a2. |
| + _ = b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v2", Exec: "a0", At: t0.Add(-time.Minute)}) |
| + if late, _ := b.HoldRanOn("h"); !late.At.Equal(second) || late.Seq <= got.Seq { |
| + t.Fatalf("marker %+v after a late report of an earlier run: want the stamp kept at %v and a newer sequence", late, second) |
| + } |
| +} |
| + |
| +// Retry ordering: a failure of an attempt that began before the host's retry |
| +// never takes the retry back. |
| +func TestAPreRetryFailureLeavesTheRetryStanding(t *testing.T) { |
| + b := &FailureBook{} |
| + before := b.Snapshot() |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s", Seq: before}) |
| + if ok, _ := b.RetryHost("bab", "blog", "v2", "h"); !ok { |
| + t.Fatal("retry") |
| + } |
| + if res, _ := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Seq: before, HeldDown: []string{"h"}}); res != PreRetry { |
| + t.Fatalf("Record = %v, want PreRetry", res) |
| + } |
| + if b.Blocks("bab", "blog", "v2", "h", false) || b.HeldDown("bab", "blog", "v2", "h") { |
| + t.Fatal("a pre-retry failure undid the retry") |
| + } |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Seq: b.Snapshot()}) |
| + if !b.Blocks("bab", "blog", "v2", "h", false) { |
| + t.Fatal("a failure after the retry must stand") |
| + } |
| +} |
| + |
| +func TestAPreRetryFailureNeverRecreatesAClearedRecord(t *testing.T) { |
| + b := &FailureBook{} |
| + before := b.Snapshot() |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s", Seq: before}) |
| + if ok, _ := b.ClearOnPointer("bab", "blog", "v2", time.Now().Add(time.Hour)); !ok { |
| + t.Fatal("clear") |
| + } |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "t", Seq: before, MarkHoldRan: true, Mode: "hold"}) |
| + if _, held := b.Lookup("bab", "blog", "v2"); held { |
| + t.Fatal("a pre-retry failure recreated the record") |
| + } |
| + if _, marked := b.HoldRanOn("t"); !marked { |
| + t.Fatal("the marker is a fact about t's database and must still be set") |
| + } |
| +} |
| + |
| +// A pre-retry hold-down leaves what the failure followed by the retry would |
| +// have left: the retry clears the hold-down, so nothing holds the host down |
| +// and no marker stands in its place. A requested marker is set, with the |
| +// execution's identity and start. |
| +func TestAPreRetryHoldDownLeavesWhatTheRetryLeaves(t *testing.T) { |
| + b := &FailureBook{} |
| + before := b.Snapshot() |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s", Seq: before}) |
| + b.RetryHost("bab", "blog", "v2", "h") |
| + started := t0.Add(-time.Minute) |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Seq: before, Mode: "hold", HeldDown: []string{"h"}, |
| + HoldRanAt: started, HoldRanExec: "a7"}) |
| + if b.HeldDown("bab", "blog", "v2", "h") { |
| + t.Fatal("a pre-retry failure held the retried host down") |
| + } |
| + if got, ok := b.HoldRanOn("h"); ok { |
| + t.Fatalf("marker %+v: a hold-down that was never made left one", got) |
| + } |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Seq: before, Mode: "hold", |
| + MarkHoldRan: true, HoldRanAt: started, HoldRanExec: "a8"}) |
| + got, ok := b.HoldRanOn("h") |
| + if !ok || got.Build != "v2" || got.Exec != "a8" || !got.At.Equal(started) { |
| + t.Fatalf("marker %+v, %v: want v2's from execution a8, stamped %v", got, ok, started) |
| + } |
| +} |
| + |
| +// Every `carlos instances retry` of a host on a standing record orders the |
| +// failures after it, a second retry included: an execution that began |
| +// between the two is before the newest one. |
| +func TestASecondRetryOrdersTheFailuresBeforeIt(t *testing.T) { |
| + b := &FailureBook{} |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s"}) |
| + b.RetryHost("bab", "blog", "v2", "h") |
| + between := b.Snapshot() |
| + if ok, _ := b.RetryHost("bab", "blog", "v2", "h"); ok { |
| + t.Fatal("a second retry of a retried host reported a change") |
| + } |
| + if res, _ := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Seq: between}); res != PreRetry { |
| + t.Fatalf("Record = %v, want PreRetry for an execution before the second retry", res) |
| + } |
| +} |
| + |
| +// What Record reports, which failCohort acts on (plan review finding 3). |
| +func TestRecordReportsWhatItDid(t *testing.T) { |
| + b := &FailureBook{} |
| + if res, _ := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "a"}); res != Recorded { |
| + t.Fatalf("first failure: %v, want Recorded", res) |
| + } |
| + if res, _ := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "b", HeldDown: []string{"b"}}); res != Merged { |
| + t.Fatalf("a new hold-down: %v, want Merged", res) |
| + } |
| + if res, _ := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "b", HeldDown: []string{"b"}}); res != Unchanged { |
| + t.Fatalf("the same failure again: %v, want Unchanged", res) |
| + } |
| + if res, _ := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "c", Seq: b.Snapshot()}); res == PreRetry { |
| + t.Fatal("a failure with no retry before it was taken as pre-retry") |
| + } |
| +} |
| + |
| +// A retry clear goes with its build's record when no route installs the |
| +// build: Prune drops both. |
| +func TestPruneDropsTheRetryClearsOfBuildsNoRouteInstalls(t *testing.T) { |
| + b := &FailureBook{} |
| + before := b.Snapshot() |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s"}) |
| + b.RetryHost("bab", "blog", "v2", "h") |
| + if err := b.Prune(func(string, string, string) bool { return false }); err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(b.cleared) != 0 { |
| + t.Fatalf("clears %v survived the prune of their build", b.cleared) |
| + } |
| + if res, _ := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Seq: before}); res != Recorded { |
| + t.Fatalf("Record = %v after the prune, want Recorded", res) |
| + } |
| +} |
| + |
| +func TestTheSequenceSurvivesAReopen(t *testing.T) { |
| + dd := t.TempDir() |
| + b, _ := OpenFailureBook(dd) |
| + _ = b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v2"}) |
| + b2, err := OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if cleared, _ := b2.ClearHoldRanBefore("h", b2.Snapshot()); !cleared { |
| + t.Fatal("after a reopen a fresh snapshot must exceed the old marker") |
| + } |
| +} |
| + |
| +// N6 through the Manager: h's first wake of v1 is planned (its snapshot |
| +// taken) and is observed; meanwhile a scheduled hold attempt onto v2 runs |
| +// on h and is superseded, which marks h. v1's settle then arrives and |
| +// writes v1 as h's good build: the write was decided before v2 ran, so v2's |
| +// marker stands. |
| +func TestASettleDecidedBeforeAHoldRanKeepsItsMarker(t *testing.T) { |
| + ctx := context.Background() |
| + h := newHarness(t, "h") |
| + r := h.rows.get("h") |
| + r.GoodVersion, r.GoodDigest = "", "" // v1 has not passed here yet |
| + h.rows.put(r) |
| + h.swap = appconfig.SwapHold |
| + h.stage("v2") |
| + h.act.mode = activator.ModeHold |
| + h.act.executed = func(a activator.Attempt, _ activator.SwapOutcome) bool { return a.Build == "v2" } |
| + |
| + p := h.m.Plan(ctx, h.rows.get("h")) |
| + if !p.FirstWake || p.Boot.Version != "v1" { |
| + t.Fatalf("plan %+v: want a first wake of v1", p) |
| + } |
| + g, _ := h.act.Current("h") |
| + h.m.OnGen(activator.GenEvent{Kind: activator.GenHealthy, Route: h.rows.get("h"), Gen: g, Mode: p.Mode, FirstWake: true, Token: p.Token}) |
| + |
| + gate := h.act.hold("h") |
| + h.install("v2", "h") |
| + h.pass() |
| + waitFor(t, "h's hold attempt onto v2", func() bool { |
| + _, started, _ := h.act.snapshot() |
| + return slices.Contains(started, "h:v2") |
| + }) |
| + h.install("v3", "h") // v3 is not staged: its attempt is refused, unrun |
| + h.pass() |
| + close(gate) |
| + waitFor(t, "v2's marker", func() bool { m, ok := h.book.HoldRanOn("h"); return ok && m.Build == "v2" }) |
| + |
| + h.m.OnGen(activator.GenEvent{Kind: activator.GenSettled, Route: h.rows.get("h"), Gen: g, Mode: p.Mode, FirstWake: true, Token: p.Token}) |
| + if r := h.rows.get("h"); r.GoodVersion != "v1" { |
| + t.Fatalf("good build %q after v1's settle, want v1", r.GoodVersion) |
| + } |
| + if m, ok := h.book.HoldRanOn("h"); !ok || m.Build != "v2" { |
| + t.Fatalf("marker %+v, %v after v1's settle: want v2's, set after v1's wake was planned", m, ok) |
| + } |
| +} |
| + |
| +// Plan review finding 3: an outcome that began before a retry is stale, and |
| +// none of the failure transition runs for it. After `carlos instances retry` |
| +// of the failing host, or a `carlos promote --retry` pointer that cleared |
| +// the record, a delayed failure of an attempt or of a planned wake that |
| +// began before it leaves the cohort unhalted, supersedes no running attempt |
| +// (y's), and leaves the retried host free: its queued adoption is admitted, |
| +// or its next wake plans the retried build. |
| +func TestAPreRetryOutcomeRunsNoFailureTransition(t *testing.T) { |
| + for _, retry := range []string{"host", "pointer"} { |
| + for _, kind := range []string{"attempt", "wake"} { |
| + t.Run(retry+" retry, "+kind, func(t *testing.T) { |
| + preRetryOutcome(t, retry, kind) |
| + }) |
| + } |
| + } |
| +} |
| + |
| +func preRetryOutcome(t *testing.T, retry, kind string) { |
| + ctx := context.Background() |
| + k := appKey{"bab", "blog"} |
| + h := newHarness(t, "h", "y", "w") |
| + h.concurrency = 2 |
| + h.swap = appconfig.SwapHold |
| + h.stage("v2") |
| + h.install("v2", "h", "y", "w") |
| + s := h.m.scheduler(k) |
| + s.mu.Lock() |
| + // The cohort's canary passed on another host already, so h and y run |
| + // side by side. |
| + *s.q.cohortFor(cohortKey("v2", classAdopt)) = cohort{canary: "x", settled: true} |
| + s.mu.Unlock() |
| + gateH, gateY := h.act.hold("h"), h.act.hold("y") |
| + h.sleepHost("w") |
| + h.pass() |
| + waitFor(t, "h's and y's attempts onto v2", func() bool { |
| + _, started, _ := h.act.snapshot() |
| + return slices.Contains(started, "h:v2") && slices.Contains(started, "y:v2") |
| + }) |
| + host := "h" |
| + var plan activator.WakePlan |
| + if kind == "wake" { |
| + host = "w" |
| + h.wakeHost("w") |
| + plan = h.m.Plan(ctx, h.rows.get("w")) |
| + if !plan.FirstWake || plan.Boot.Version != "v2" { |
| + t.Fatalf("w's plan %+v: want a first wake of v2", plan) |
| + } |
| + } |
| + |
| + // A sibling's failure of v2, written straight into the book: no halt |
| + // stands, so a halt after this point is the stale outcome's. |
| + h.book.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s", PromotedAt: t0, At: t0}) |
| + switch retry { |
| + case "host": |
| + if !h.m.RetryHost(host) { |
| + t.Fatalf("RetryHost(%s) cleared nothing", host) |
| + } |
| + case "pointer": |
| + h.advance(time.Minute) |
| + h.pointer("stable", "v2", h.clock()) |
| + h.pass() |
| + if _, held := h.book.Lookup("bab", "blog", "v2"); held { |
| + t.Fatal("the retry pointer left v2's record standing") |
| + } |
| + } |
| + s.mu.Lock() |
| + revY := s.q.newest("y") |
| + s.mu.Unlock() |
| + |
| + // The outcome that began before the retry arrives now. |
| + if kind == "attempt" { |
| + h.act.mu.Lock() |
| + h.act.outcome = func(a activator.Attempt) activator.SwapOutcome { |
| + if a.Host == "h" && a.Cause != activator.CauseRetry { |
| + return activator.SwapFailed |
| + } |
| + return activator.SwapDone |
| + } |
| + h.act.mu.Unlock() |
| + gateH2 := h.act.hold("h") // h's next attempt parks in Swap |
| + defer close(gateH2) |
| + close(gateH) |
| + waitFor(t, "h's attempt after the stale failure", func() bool { |
| + calls, started, _ := h.act.snapshot() |
| + n := 0 |
| + for _, st := range started { |
| + if st == "h:v2" { |
| + n++ |
| + } |
| + } |
| + return len(calls) >= 1 && n == 2 |
| + }) |
| + } else { |
| + defer close(gateH) |
| + h.m.OnGen(activator.GenEvent{Kind: activator.GenFailed, Route: h.rows.get("w"), Failed: plan.Boot, Mode: plan.Mode, |
| + FirstWake: true, Reason: "health check failed", Token: plan.Token, ConfigRev: plan.ConfigRev}) |
| + } |
| + defer close(gateY) |
| + |
| + s.mu.Lock() |
| + halted := s.q.cohortFor(cohortKey("v2", classAdopt)).halted |
| + y := s.q.hosts["y"] |
| + yRunning, yRev, yNewest := y.running != nil, uint64(0), y.buildRev |
| + if y.running != nil { |
| + yRev = y.running.SchedRev |
| + } |
| + s.mu.Unlock() |
| + if halted { |
| + t.Fatal("a stale outcome halted the cohort") |
| + } |
| + if !yRunning || yRev != revY || yNewest != revY { |
| + t.Fatalf("y's attempt: running %v, rev %d, newest %d; want it running unsuperseded at %d", yRunning, yRev, yNewest, revY) |
| + } |
| + if h.book.Blocks("bab", "blog", "v2", host, false) || h.book.HeldDown("bab", "blog", "v2", host) { |
| + t.Fatalf("the stale outcome took %s's retry back", host) |
| + } |
| + if retry == "pointer" { |
| + if _, held := h.book.Lookup("bab", "blog", "v2"); held { |
| + t.Fatal("the stale outcome recreated the cleared record") |
| + } |
| + } |
| + if kind == "attempt" { |
| + calls, _, _ := h.act.snapshot() |
| + if c := calls[0].a; c.Host != "h" || calls[0].outcome != activator.SwapFailed { |
| + t.Fatalf("first finished call %+v: want h's stale failure", calls[0]) |
| + } |
| + return |
| + } |
| + h.wakeHost("w") |
| + if p := h.m.Plan(ctx, h.rows.get("w")); p.Refuse != "" || p.Boot.Version != "v2" || !p.FirstWake { |
| + t.Fatalf("w's next plan %+v: want the retried v2", p) |
| + } |
| +} |
diff --git a/internal/rollout/failures.go b/internal/rollout/failures.go| index 7725c2a1a..f54eb73b2 100644 |
| --- a/internal/rollout/failures.go |
| +++ b/internal/rollout/failures.go |
| @@ -50,6 +50,50 @@ type FailureRecord struct { |
| // HoldRanAt is the marker's At for MarkHoldRan: when the attempt that |
| // ran the build started (its dispatch). An instruction too, never stored. |
| HoldRanAt time.Time `json:"-"` |
| + // HoldRanExec is the marker's Exec: the identity of the execution that |
| + // failed (HoldRan.Exec). An instruction, never stored. |
| + HoldRanExec string `json:"-"` |
| + // Seq is the book's snapshot (FailureBook.Snapshot) taken when the |
| + // failing execution started; zero when it is not known. Record compares |
| + // it with the retries since (Astra ruling 191, retry ordering): a failure |
| + // of an execution that began before a retry never takes the retry back. |
| + // An instruction, never stored. |
| + Seq uint64 `json:"-"` |
| +} |
| + |
| +// RecordResult is what Record did with a failure. failCohort runs the rest |
| +// of the cohort-failure transition (the halt and the supersession) for every |
| +// result but PreRetry (plan review finding 3). |
| +type RecordResult int |
| + |
| +const ( |
| + // Recorded: the build had no record; this failure is its first. |
| + Recorded RecordResult = iota |
| + // Merged: a record stood, and this failure changed it (its host's retry |
| + // spent, a hold-down added, a newer PromotedAt). |
| + Merged |
| + // Unchanged: a record stood, and this failure added nothing to it. |
| + Unchanged |
| + // PreRetry: the failing execution began before a retry that has since |
| + // cleared the build for its host or for the app (`carlos instances |
| + // retry`, `carlos promote --retry`). It is stale: nothing of the record |
| + // changes, and no halt or supersession may follow it. A hold-ran marker |
| + // it carries is still set (Record). |
| + PreRetry |
| +) |
| + |
| +func (r RecordResult) String() string { |
| + switch r { |
| + case Recorded: |
| + return "recorded" |
| + case Merged: |
| + return "merged" |
| + case Unchanged: |
| + return "unchanged" |
| + case PreRetry: |
| + return "pre-retry" |
| + } |
| + return fmt.Sprintf("RecordResult(%d)", int(r)) |
| } |
| // HoldRan is an instance's "hold ran" marker (final review wave 2, N2): a |
| @@ -61,13 +105,26 @@ type FailureRecord struct { |
| // (decideWake, planCheck), and a worker's replacement carries no fallback |
| // onto it (executeWorker, Astra R3 N5). It is a restriction on the |
| // instance, not a failure of the build: it blocks no sibling and halts no |
| -// cohort, and it lifts no failure record either (Astra R3 N7). Its own |
| -// good-build write (a pass), `carlos instances retry`, or a signed rollback |
| -// pointer newer than the marker (applyPointers, ruling 205) clears it. |
| +// cohort, and it lifts no failure record either (Astra R3 N7). A good-build |
| +// write decided after the marker was set (a pass whose execution began after |
| +// it, Astra R3 N6), `carlos instances retry`, or a signed rollback pointer |
| +// newer than the marker (applyPointers, ruling 205) clears it. |
| type HoldRan struct { |
| Account string `json:"account"` |
| App string `json:"app"` |
| Build string `json:"build"` |
| + // Exec is the identity of the execution that set the marker: "a<n>" for |
| + // a scheduled attempt (dispatch.ExecID), "w<token>" for a planned wake. |
| + // A marker belongs to one execution, not to its build (plan review |
| + // finding 4): another run of the same build renews the marker's Seq, so |
| + // a good write decided between the two runs clears nothing. Empty when |
| + // the caller could not tell, which counts as a new execution every time. |
| + Exec string `json:"exec,omitempty"` |
| + // Seq is the book's sequence when the marker was set (markLocked), so a |
| + // good write clears only markers set before its execution began |
| + // (ClearHoldRanBefore, Astra R3 N6). Zero in a file an earlier agent |
| + // wrote, which any snapshot is newer than. |
| + Seq uint64 `json:"seq,omitempty"` |
| // At is when the execution that set the marker started: an attempt's |
| // dispatch, a planned wake's plan (Task 3 fix round 1). Only a rollback |
| // pointer signed after it lifts the marker (ruling 205, Astra R3): a |
| @@ -80,8 +137,7 @@ type HoldRan struct { |
| At time.Time `json:"at,omitempty"` |
| } |
| -// same reports whether h and o mark the same build of the same app. At is |
| -// left out: marking the build again changes nothing worth a write. |
| +// same reports whether h and o mark the same build of the same app. |
| func (h HoldRan) same(o HoldRan) bool { |
| return h.Account == o.Account && h.App == o.App && h.Build == o.Build |
| } |
| @@ -93,6 +149,9 @@ type failureFile struct { |
| // agent that predates it ignores the key (encoding/json skips unknown |
| // fields), and a file without it has no markers. |
| HoldRan map[string]HoldRan `json:"hold_ran,omitempty"` |
| + // Seq is the book's sequence (FailureBook.seq), additive like HoldRan, |
| + // so a snapshot after a restart is newer than every marker before it. |
| + Seq uint64 `json:"seq,omitempty"` |
| } |
| // FailureBook is the box's failure records. An empty path keeps them in |
| @@ -114,6 +173,22 @@ type FailureBook struct { |
| // now stamps a marker whose caller gave no At (markLocked). Nil means |
| // time.Now; tests and the simulation set their own clock. |
| now func() time.Time |
| + // seq orders the book's events that later outcomes are compared with |
| + // (Astra R3 N6, ruling 191): every marker set and every retry clear takes |
| + // the next value, and an execution takes a Snapshot (seq+1) when it |
| + // starts. An outcome is older than an event exactly when its snapshot is |
| + // not greater than the event's value. It is saved with the markers, so |
| + // snapshots after a restart are newer than every marker on disk; a value |
| + // taken by a retry clear and never saved is harmless, because clears are |
| + // not saved either. |
| + seq uint64 |
| + // cleared holds, in memory only, the sequence of the latest retry clear |
| + // per (account, app, build, host): `carlos instances retry` per host |
| + // (RetryHost), `carlos promote --retry` app-wide with host "" (a |
| + // ClearOnPointer that cleared a record). Executions in flight do not |
| + // survive an agent restart, so neither need these. Prune drops the |
| + // entries of builds no route installs, with their records. |
| + cleared map[[4]string]uint64 |
| } |
| func (b *FailureBook) clock() time.Time { |
| @@ -151,6 +226,13 @@ func OpenFailureBook(dataDir string) (*FailureBook, error) { |
| } |
| b.recs = f.Records |
| b.held = f.HoldRan |
| + b.seq = f.Seq |
| + for _, h := range b.held { |
| + // A marker's sequence is saved with the book's, so this changes |
| + // nothing in a file this book wrote; it keeps a hand-edited one from |
| + // handing out a snapshot a standing marker is newer than. |
| + b.seq = max(b.seq, h.Seq) |
| + } |
| return b, nil |
| } |
| @@ -160,7 +242,7 @@ func (b *FailureBook) saveLocked() error { |
| if b.path == "" { |
| return nil |
| } |
| - raw, err := json.MarshalIndent(failureFile{Schema: 1, Records: b.recs, HoldRan: b.held}, "", " ") |
| + raw, err := json.MarshalIndent(failureFile{Schema: 1, Records: b.recs, HoldRan: b.held, Seq: b.seq}, "", " ") |
| if err == nil { |
| err = release.WriteFileAtomic(b.path, raw, 0o644) |
| } |
| @@ -183,6 +265,49 @@ func (b *FailureBook) find(acct, app, build string) int { |
| }) |
| } |
| +// Snapshot is the book's sequence plus one, taken when an execution starts |
| +// (an attempt in scheduler.execute, a planned wake in Plan): every marker |
| +// and retry clear already in the book is older than it, and every one set |
| +// later is not (Astra R3 N6, ruling 191). Zero is never a snapshot: it |
| +// means "not known", which clears no marker and makes no outcome stale. |
| +func (b *FailureBook) Snapshot() uint64 { |
| + b.mu.Lock() |
| + defer b.mu.Unlock() |
| + return b.seq + 1 |
| +} |
| + |
| +// bumpLocked takes the book's next sequence value. The caller holds b.mu. |
| +func (b *FailureBook) bumpLocked() uint64 { |
| + b.seq++ |
| + return b.seq |
| +} |
| + |
| +// clearKey is a retry clear's key in FailureBook.cleared; host "" is the |
| +// app-wide clear of `carlos promote --retry`. |
| +func clearKey(acct, app, build, host string) [4]string { |
| + return [4]string{acct, app, build, host} |
| +} |
| + |
| +// preRetryLocked reports whether r's execution began before a retry that |
| +// cleared its build since, for its host or app-wide (ruling 191): its |
| +// snapshot is not newer than the clear. The caller holds b.mu. |
| +func (b *FailureBook) preRetryLocked(r FailureRecord) bool { |
| + if r.Seq == 0 { |
| + return false |
| + } |
| + return r.Seq <= b.cleared[clearKey(r.Account, r.App, r.Build, r.Host)] || |
| + r.Seq <= b.cleared[clearKey(r.Account, r.App, r.Build, "")] |
| +} |
| + |
| +// clearedLocked notes a retry clear of build for host ("" app-wide) at the |
| +// book's next sequence value. The caller holds b.mu. |
| +func (b *FailureBook) clearedLocked(acct, app, build, host string) { |
| + if b.cleared == nil { |
| + b.cleared = map[[4]string]uint64{} |
| + } |
| + b.cleared[clearKey(acct, app, build, host)] = b.bumpLocked() |
| +} |
| + |
| // Record writes r unless a record for its build already stands, in which case |
| // the first failure stands and only r's host is merged in: it leaves Retried |
| // (its retry was spent), joins HeldDown if r holds it down, and the record |
| @@ -192,19 +317,34 @@ func (b *FailureBook) find(acct, app, build string) int { |
| // the error is for the journal, and the next call on the book (this one |
| // again, if nothing new, or Prune) retries the save. r.MarkHoldRan sets r's |
| // host's hold-ran marker in the same save. |
| -func (b *FailureBook) Record(r FailureRecord) (bool, error) { |
| +// |
| +// A failure of an execution that began before a retry of its build (r.Seq, |
| +// ruling 191) is stale: the record is neither created nor changed, the |
| +// host's retry is not spent and no hold-down is added, and Record reports |
| +// PreRetry so the caller runs none of the transition (plan review finding |
| +// 3). That is what the failure followed by the retry would have left: the |
| +// retry clears the host's hold-down. A requested hold-ran marker is still |
| +// set, because it is a fact about the instance's database, not a failure. |
| +func (b *FailureBook) Record(r FailureRecord) (RecordResult, error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| - marked := r.MarkHoldRan && b.markLocked(r.Host, HoldRan{Account: r.Account, App: r.App, Build: r.Build, At: r.HoldRanAt}) |
| - r.MarkHoldRan, r.HoldRanAt = false, time.Time{} |
| + stale := b.preRetryLocked(r) |
| + marked := r.MarkHoldRan && b.markLocked(r.Host, HoldRan{Account: r.Account, App: r.App, Build: r.Build, Exec: r.HoldRanExec, At: r.HoldRanAt}) |
| + r.MarkHoldRan, r.HoldRanAt, r.HoldRanExec, r.Seq = false, time.Time{}, "", 0 |
| + if stale { |
| + if marked { |
| + return PreRetry, b.saveLocked() |
| + } |
| + return PreRetry, b.retryLocked() |
| + } |
| i := b.find(r.Account, r.App, r.Build) |
| if i < 0 { |
| r.Retried = nil |
| b.recs = append(b.recs, cloneRec(r)) |
| - return true, b.saveLocked() |
| + return Recorded, b.saveLocked() |
| } |
| cur := &b.recs[i] |
| - changed := marked |
| + changed := false |
| if j := slices.Index(cur.Retried, r.Host); j >= 0 { |
| cur.Retried = slices.Delete(slices.Clone(cur.Retried), j, j+1) |
| changed = true |
| @@ -219,22 +359,32 @@ func (b *FailureBook) Record(r FailureRecord) (bool, error) { |
| changed = true |
| } |
| } |
| - if !changed { |
| - return false, b.retryLocked() |
| + switch { |
| + case changed: |
| + return Merged, b.saveLocked() |
| + case marked: |
| + return Unchanged, b.saveLocked() |
| } |
| - return false, b.saveLocked() |
| + return Unchanged, b.retryLocked() |
| } |
| // markLocked sets host's hold-ran marker to h and reports whether that |
| // changed anything. h.At is the execution's start, from the caller. A zero |
| // one (a caller that could not know it) is stamped with the book's clock |
| // now: later than any real start, so only a pointer signed later still |
| -// lifts the marker, the safe direction. A marker already standing for the |
| -// same build is left as it is, its first stamp included, so marking it |
| -// again writes nothing (Task 4 gives each execution its own identity). The |
| -// caller holds b.mu and saves. |
| +// lifts the marker, the safe direction. |
| +// |
| +// A marker is an execution's (plan review finding 4). A repeated report of |
| +// the execution that set the standing marker (same build, same non-empty |
| +// Exec) changes nothing and writes nothing. Any other execution gets the |
| +// book's next sequence value, so a good write whose execution began before |
| +// it clears nothing (ClearHoldRanBefore); for the same build it keeps the |
| +// later of the two stamps, so a late report of an earlier run never makes a |
| +// rollback signed between the runs look newer than the marker. The caller |
| +// holds b.mu and saves. |
| func (b *FailureBook) markLocked(host string, h HoldRan) bool { |
| - if cur, ok := b.held[host]; ok && cur.same(h) { |
| + cur, ok := b.held[host] |
| + if ok && cur.same(h) && h.Exec != "" && cur.Exec == h.Exec { |
| return false |
| } |
| if b.held == nil { |
| @@ -243,6 +393,10 @@ func (b *FailureBook) markLocked(host string, h HoldRan) bool { |
| if h.At.IsZero() { |
| h.At = b.clock() |
| } |
| + if ok && cur.same(h) && cur.At.After(h.At) { |
| + h.At = cur.At |
| + } |
| + h.Seq = b.bumpLocked() |
| b.held[host] = h |
| return true |
| } |
| @@ -269,9 +423,9 @@ func (b *FailureBook) HoldRanOn(host string) (HoldRan, bool) { |
| return h, ok |
| } |
| -// ClearHoldRan drops host's hold-ran marker: the instance recorded a new |
| -// good build, or `carlos instances retry` named it. It reports whether a |
| -// marker stood. |
| +// ClearHoldRan drops host's hold-ran marker, whatever it is: `carlos |
| +// instances retry` named the host, the explicit recovery. It reports whether |
| +// a marker stood. |
| func (b *FailureBook) ClearHoldRan(host string) (bool, error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| @@ -282,6 +436,24 @@ func (b *FailureBook) ClearHoldRan(host string) (bool, error) { |
| return true, b.saveLocked() |
| } |
| +// ClearHoldRanBefore drops host's hold-ran marker for a good-build write |
| +// whose execution took snapshot snap (Astra R3 N6): only a marker set before |
| +// that execution began, which the instance has since passed over. A marker |
| +// set later is a hold build that ran after the write was decided (the write |
| +// was delayed, say, between its acknowledgement check and here), and it |
| +// stands. A zero snapshot (Bootstrap, a wake whose start is not known) |
| +// clears nothing. The check and the clear are one hold of the lock. |
| +func (b *FailureBook) ClearHoldRanBefore(host string, snap uint64) (bool, error) { |
| + b.mu.Lock() |
| + defer b.mu.Unlock() |
| + cur, ok := b.held[host] |
| + if !ok || snap == 0 || cur.Seq >= snap { |
| + return false, b.retryLocked() |
| + } |
| + delete(b.held, host) |
| + return true, b.saveLocked() |
| +} |
| + |
| // ClearHoldRanOnPointer is an explicit rollback as this box sees it (ruling |
| // 205, Astra R3): host's route installs its good build, and the signed |
| // pointer of its channel names that build with promotedAt. The caller has |
| @@ -337,11 +509,22 @@ func (b *FailureBook) HeldDown(acct, app, build, host string) bool { |
| // RetryHost is `carlos instances retry <host>`: it clears the record for that |
| // host only (spec §8.4). How the request reaches the box is plan 6's. |
| +// |
| +// Every retry of a standing record also notes a host clear at the book's |
| +// next sequence value (ruling 191), a retry of an already retried host |
| +// included: a failure of an execution of host that began before the newest |
| +// retry is then stale (Record), and takes nothing back. Only the clear is |
| +// new for a host already retried, and it is kept in memory, so nothing is |
| +// written for it. |
| func (b *FailureBook) RetryHost(acct, app, build, host string) (bool, error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| i := b.find(acct, app, build) |
| - if i < 0 || slices.Contains(b.recs[i].Retried, host) { |
| + if i < 0 { |
| + return false, b.retryLocked() |
| + } |
| + b.clearedLocked(acct, app, build, host) |
| + if slices.Contains(b.recs[i].Retried, host) { |
| return false, b.retryLocked() |
| } |
| cur := &b.recs[i] |
| @@ -355,7 +538,10 @@ func (b *FailureBook) RetryHost(acct, app, build, host string) (bool, error) { |
| // ClearOnPointer is `carlos promote --retry` as this box sees it: a signed |
| // channel pointer naming build whose PromotedAt is newer than the record's |
| // clears the record (spec rev 14 §8.4). The pointer's signature is verified |
| -// by the caller's read; nobody can forge a fresh one. |
| +// by the caller's read; nobody can forge a fresh one. A clear is noted |
| +// app-wide at the book's next sequence value (ruling 191), so a failure of |
| +// any instance's execution that began before it is stale (Record) and never |
| +// writes the record back. |
| func (b *FailureBook) ClearOnPointer(acct, app, build string, promotedAt time.Time) (bool, error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| @@ -364,6 +550,7 @@ func (b *FailureBook) ClearOnPointer(acct, app, build string, promotedAt time.Ti |
| return false, b.retryLocked() |
| } |
| b.recs = slices.Delete(b.recs, i, i+1) |
| + b.clearedLocked(acct, app, build, "") |
| return true, b.saveLocked() |
| } |
| @@ -385,9 +572,17 @@ func (b *FailureBook) ForApp(acct, app string) []FailureRecord { |
| // (spec §8.4), and only for those. installed is called under the book's lock |
| // and must take no lock of its own (nor call back into the book). The |
| // scheduler runs Prune every sweep, so it also retries a save that failed. |
| +// The retry clears of such builds go too (they are in memory only): a stale |
| +// failure of one arriving after is a new record, which the next Prune drops |
| +// again. |
| func (b *FailureBook) Prune(installed func(acct, app, build string) bool) error { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| + for k := range b.cleared { |
| + if !installed(k[0], k[1], k[2]) { |
| + delete(b.cleared, k) |
| + } |
| + } |
| kept := b.recs[:0] |
| for _, r := range b.recs { |
| if installed(r.Account, r.App, r.Build) { |
diff --git a/internal/rollout/failures_test.go b/internal/rollout/failures_test.go| index 77c84ce95..93351996a 100644 |
| --- a/internal/rollout/failures_test.go |
| +++ b/internal/rollout/failures_test.go |
| @@ -21,7 +21,7 @@ func rec(build, host string) FailureRecord { |
| func TestFailureRecordBlocksOnlyInstancesThatHaveNotServedTheBuild(t *testing.T) { |
| b := &FailureBook{} |
| added, err := b.Record(rec("v2", "a.example.com")) |
| - if err != nil || !added { |
| + if err != nil || added != Recorded { |
| t.Fatalf("Record = %v, %v", added, err) |
| } |
| if !b.Blocks("bab", "blog", "v2", "b.example.com", false) { |
| @@ -36,7 +36,7 @@ func TestFailureRecordBlocksOnlyInstancesThatHaveNotServedTheBuild(t *testing.T) |
| if b.Blocks("bab", "other", "v2", "b.example.com", false) { |
| t.Fatal("the scope is (app, build, box), not the build name alone") |
| } |
| - if added, _ := b.Record(rec("v2", "b.example.com")); added { |
| + if added, _ := b.Record(rec("v2", "b.example.com")); added == Recorded { |
| t.Fatal("the first failure stands; a second is not a second record") |
| } |
| } |
| @@ -222,8 +222,8 @@ func TestFailedSaveIsRetriedByTheNextRecord(t *testing.T) { |
| t.Fatal("the first Record must report the failed save") |
| } |
| heal() |
| - if added, err := b.Record(rec("v2", "a.example.com")); err != nil || added { |
| - t.Fatalf("the repeat Record = %v, %v; want false, nil", added, err) |
| + if added, err := b.Record(rec("v2", "a.example.com")); err != nil || added != Unchanged { |
| + t.Fatalf("the repeat Record = %v, %v; want unchanged, nil", added, err) |
| } |
| again, err := OpenFailureBook(dir) |
| if err != nil { |
diff --git a/internal/rollout/holdran_test.go b/internal/rollout/holdran_test.go| index 6eeb741c2..2ac2d71d9 100644 |
| --- a/internal/rollout/holdran_test.go |
| +++ b/internal/rollout/holdran_test.go |
| @@ -683,17 +683,20 @@ func TestAWakesHoldRanMarkerIsStampedWithItsPlanTime(t *testing.T) { |
| // Ruling 205's book half: a marker keeps the At its caller gives (the |
| // execution's start), and one given none is stamped by the book's clock; a |
| -// re-mark of the same build keeps the first stamp, and writes nothing. The |
| -// rollback clear lifts only (acct, app)'s marker, and only for a pointer |
| -// newer than it. |
| +// repeated report of the same execution keeps the first stamp, and writes |
| +// nothing (another execution renews it: bookseq_test.go). The rollback |
| +// clear lifts only (acct, app)'s marker, and only for a pointer newer than |
| +// it. |
| func TestTheHoldRanMarkerStampAndItsRollbackClear(t *testing.T) { |
| now := t0 |
| b := &FailureBook{now: func() time.Time { return now }} |
| - b.MarkHoldRan("a", markV2) |
| + one := markV2 |
| + one.Exec = "a1" |
| + b.MarkHoldRan("a", one) |
| now = now.Add(time.Minute) |
| - b.MarkHoldRan("a", markV2) |
| + b.MarkHoldRan("a", one) |
| if got, _ := b.HoldRanOn("a"); !got.At.Equal(t0) { |
| - t.Fatalf("marker At %v after a re-mark of the same build, want %v", got.At, t0) |
| + t.Fatalf("marker At %v after a repeated report of its execution, want %v", got.At, t0) |
| } |
| if ok, _ := b.ClearHoldRanOnPointer("a", "bab", "blog", t0); ok { |
| t.Fatal("a pointer as old as the marker cleared it") |
diff --git a/internal/rollout/manager.go b/internal/rollout/manager.go| index a1218b66d..ea942bf14 100644 |
| --- a/internal/rollout/manager.go |
| +++ b/internal/rollout/manager.go |
| @@ -6,6 +6,7 @@ import ( |
| "log" |
| "os" |
| "slices" |
| + "strconv" |
| "strings" |
| "sync" |
| "time" |
| @@ -141,15 +142,19 @@ type Manager struct { |
| // wake's, and acts on nothing that belongs to the newer one. |
| wakeTok map[string]uint64 |
| tokenCount uint64 |
| - // wakeAt holds, per host, the plan time of each planned first wake that |
| - // may still end with its build run (wakeStart, by token): the start of |
| - // that wake's execution, which a hold-ran marker the wake leaves carries |
| - // as its At (ruling 205, Task 3 fix round 1). Kept per token, not only |
| - // the newest plan's, because the marker is not token-scoped: a late |
| - // event of an older wake still marks the instance, with its own start. |
| - // An entry goes with its wake's last event (any kind but healthy), with |
| - // the host (forget), or when the host has wakeStartsKept newer ones. |
| + // wakeAt holds, per host, the start of each planned wake that may still |
| + // report (wakeStart, by token): its plan time, which a hold-ran marker |
| + // the wake leaves carries as its At (ruling 205, Task 3 fix round 1), and |
| + // the failure book's snapshot taken with it, which orders the wake's |
| + // outcome against the book (Astra R3 N6, ruling 191). Kept per token, |
| + // not only the newest plan's, because neither a marker nor a failure is |
| + // token-scoped: a late event of an older wake still acts, as of its own |
| + // start. An entry goes with its wake's last event, with the host |
| + // (forget), or when the host has wakeStartsKept newer ones. |
| wakeAt map[string][]wakeStart |
| + // execCount numbers the scheduled attempts' executions (nextExec, |
| + // dispatch.ExecID). |
| + execCount uint64 |
| // goodMu serialises every serving- and good-build write with its |
| // acknowledgement check (served, publishServing, writeGood), so an older |
| @@ -582,7 +587,11 @@ func (m *Manager) retryBuild(k appKey, build, host string) { |
| // the scheduler's lock (scheduler.mu before FailureBook.mu), the book first. |
| // Admission cannot run between the two, so the adoption onto the cleared |
| // build is the attempt the flag lets through, as a canary, and a restart or |
| -// config attempt cannot start in between and spend the flag. |
| +// config attempt cannot start in between and spend the flag. The book also |
| +// notes the retry's place in its sequence there (ruling 191), in memory: an |
| +// outcome of an execution that began before it is stale, and takes the |
| +// retry back no more than it halts the cohort (failCohort). No write is |
| +// added under the scheduler's lock: the book saves as it did before. |
| // |
| // It also clears host's hold-ran marker (final review wave 2, N2), whatever |
| // build the marker names and whether or not a record held the host: the |
| @@ -665,53 +674,75 @@ func (m *Manager) newWakeToken(host string) (uint64, time.Time) { |
| return m.tokenCount, now |
| } |
| -// wakeStart is a planned first wake's plan time, by its token |
| -// (Manager.wakeAt). |
| +// nextExec numbers a scheduled attempt's execution (dispatch.ExecID). |
| +func (m *Manager) nextExec() uint64 { |
| + m.mu.Lock() |
| + defer m.mu.Unlock() |
| + m.execCount++ |
| + return m.execCount |
| +} |
| + |
| +// wakeExecID is a planned wake's execution identity (HoldRan.Exec): "w" |
| +// and its token. Zero, an event of no known plan, has none. |
| +func wakeExecID(tok uint64) string { |
| + if tok == 0 { |
| + return "" |
| + } |
| + return "w" + strconv.FormatUint(tok, 10) |
| +} |
| + |
| +// wakeStart is a planned wake's start, by its token (Manager.wakeAt): its |
| +// plan time and the failure book's snapshot taken in the same plan. |
| type wakeStart struct { |
| token uint64 |
| at time.Time |
| + snap uint64 |
| } |
| // wakeStartsKept bounds Manager.wakeAt per host. A wake that ends with no |
| -// event (refused at its lease) leaves its entry until this many newer plans |
| -// push it out; a live wake is never that far behind its host's newest plan, |
| -// because the activator waits for a planned wake's outcome before it plans |
| -// again (outcomeFenceWait). |
| +// event (refused at its lease, a failed ordinary wake) leaves its entry |
| +// until this many newer plans push it out; a live wake is never that far |
| +// behind its host's newest plan, because the activator waits for a planned |
| +// wake's outcome before it plans again (outcomeFenceWait). |
| const wakeStartsKept = 4 |
| -// noteWakeStart keeps the plan time of a planned first wake (tok), the only |
| -// kind whose end can leave a hold-ran marker (OnGen). Any mode is kept: the |
| -// activator reports the mode the wake ran in, which the plan does not fix. |
| -func (m *Manager) noteWakeStart(host string, tok uint64, at time.Time, plan activator.WakePlan) { |
| - if tok == 0 || !plan.FirstWake || plan.Refuse != "" || plan.Boot.StagedPath == "" { |
| +// noteWakeStart keeps the start of a planned wake that boots a build (tok): |
| +// a first wake's end can leave a hold-ran marker or a failure (OnGen), and |
| +// any wake's healthy event writes the good build, which clears only markers |
| +// older than its snapshot (Astra R3 N6). Any mode is kept: the activator |
| +// reports the mode the wake ran in, which the plan does not fix. |
| +func (m *Manager) noteWakeStart(host string, tok uint64, at time.Time, snap uint64, plan activator.WakePlan) { |
| + if tok == 0 || plan.Refuse != "" || plan.Boot.StagedPath == "" { |
| return |
| } |
| m.mu.Lock() |
| defer m.mu.Unlock() |
| - starts := append(m.wakeAt[host], wakeStart{token: tok, at: at}) |
| + starts := append(m.wakeAt[host], wakeStart{token: tok, at: at, snap: snap}) |
| if len(starts) > wakeStartsKept { |
| starts = slices.Delete(starts, 0, len(starts)-wakeStartsKept) |
| } |
| m.wakeAt[host] = starts |
| } |
| -// wakeStartedAt is when the wake whose token is tok was planned, and drops |
| -// its entry when ended (the wake's last event). Zero means the start is not |
| -// known (a token pushed out by newer plans, or a host forgotten): the book |
| -// then stamps the marker with its own clock, later than the real start, so |
| -// only a pointer signed later still lifts it (the safe direction). |
| -func (m *Manager) wakeStartedAt(host string, tok uint64, ended bool) time.Time { |
| +// wakeStarted is the start of the wake whose token is tok, and drops its |
| +// entry when ended (the wake's last event). A zero result means the start |
| +// is not known (a token pushed out by newer plans, or a host forgotten): the |
| +// book then stamps a marker with its own clock, later than the real start, |
| +// so only a pointer signed later still lifts it; the good write clears no |
| +// marker; and a failure is never taken as stale. Each is the safe |
| +// direction. |
| +func (m *Manager) wakeStarted(host string, tok uint64, ended bool) wakeStart { |
| m.mu.Lock() |
| defer m.mu.Unlock() |
| if tok == 0 { |
| - return time.Time{} |
| + return wakeStart{} |
| } |
| starts := m.wakeAt[host] |
| i := slices.IndexFunc(starts, func(w wakeStart) bool { return w.token == tok }) |
| if i < 0 { |
| - return time.Time{} |
| + return wakeStart{} |
| } |
| - at := starts[i].at |
| + w := starts[i] |
| if ended { |
| if starts = slices.Delete(starts, i, i+1); len(starts) == 0 { |
| delete(m.wakeAt, host) |
| @@ -719,7 +750,7 @@ func (m *Manager) wakeStartedAt(host string, tok uint64, ended bool) time.Time { |
| m.wakeAt[host] = starts |
| } |
| } |
| - return at |
| + return w |
| } |
| // notePlanned records what host's newest plan boots: an entry for a first |
| @@ -865,13 +896,15 @@ func (m *Manager) liftClearedHalts() { |
| // Bootstrap is spec §8.5's first start: every awake instance records the |
| // build it serves as good. Nothing is inferred from Version or PrevVersion. |
| +// No execution of this agent started the generation, so the write has no |
| +// book snapshot and clears no hold-ran marker (Astra R3 N6). |
| func (m *Manager) Bootstrap(routes []registry.Route) { |
| for _, rt := range routes { |
| if !Managed(rt) || rt.GoodVersion != "" { |
| continue |
| } |
| if g, ok := m.d.Act.Current(rt.Host); ok && g.Build != "" { |
| - m.writeGood(rt.Host, g) |
| + m.writeGood(rt.Host, g, 0) |
| } |
| } |
| } |
| @@ -1024,8 +1057,9 @@ func (m *Manager) stagedDemand(ctx context.Context, rt registry.Route, build, pa |
| // served records a generation that became current after its checks: the |
| // serving build, and the good build (spec §8.5). Both only while g is the |
| -// generation the activator serves (acknowledged). |
| -func (m *Manager) served(host string, g activator.Gen) { |
| +// generation the activator serves (acknowledged). snap is the book's |
| +// snapshot from the start of the execution that produced g (writeGoodLocked). |
| +func (m *Manager) served(host string, g activator.Gen, snap uint64) { |
| if g.Build == "" { |
| return |
| } |
| @@ -1036,7 +1070,7 @@ func (m *Manager) served(host string, g activator.Gen) { |
| return |
| } |
| m.setServingLocked(host, g) |
| - m.writeGoodLocked(host, g) |
| + m.writeGoodLocked(host, g, snap) |
| } |
| // setServingLocked writes an instance's serving build. A worker's is the |
| @@ -1082,15 +1116,15 @@ func (m *Manager) acknowledged(host string, g activator.Gen) bool { |
| return cur.StagedPath == g.StagedPath && cur.Digest == g.Digest |
| } |
| -// writeGood is writeGoodLocked for a caller outside served: a settled first |
| -// wake, a healthy planned wake. Same terms: g must be acknowledged. |
| -func (m *Manager) writeGood(host string, g activator.Gen) { |
| +// writeGood is writeGoodLocked for a caller outside served: Bootstrap. |
| +// Same terms: g must be acknowledged. |
| +func (m *Manager) writeGood(host string, g activator.Gen, snap uint64) { |
| m.goodMu.Lock() |
| defer m.goodMu.Unlock() |
| if !m.acknowledged(host, g) { |
| return |
| } |
| - m.writeGoodLocked(host, g) |
| + m.writeGoodLocked(host, g, snap) |
| } |
| // writeGoodLocked replaces host's good build with g's identity, |
| @@ -1101,8 +1135,12 @@ func (m *Manager) writeGood(host string, g activator.Gen) { |
| // |
| // A good build recorded (written, or found already written) clears the |
| // host's hold-ran marker (final review wave 2, N2): the instance passed on |
| -// g, so g is what a later wake may fall back to. |
| -func (m *Manager) writeGoodLocked(host string, g activator.Gen) { |
| +// g, so g is what a later wake may fall back to. Only a marker set before |
| +// g's execution began (snap, Astra R3 N6): the write can be delayed after |
| +// its acknowledgement check, and a hold build that ran on the host |
| +// meanwhile (an attempt done() records without goodMu) left a marker the |
| +// instance has not passed over. |
| +func (m *Manager) writeGoodLocked(host string, g activator.Gen, snap uint64) { |
| if g.Build == "" || g.Digest == "" { |
| return |
| } |
| @@ -1113,12 +1151,12 @@ func (m *Manager) writeGoodLocked(host string, g activator.Gen) { |
| return |
| } |
| if cur.Good() == next { |
| - m.goodRecorded(host, next.Version) |
| + m.goodRecorded(host, next.Version, snap) |
| return |
| } |
| err = m.d.Routes.SetGoodBuild(host, cur.Good(), next) |
| if err == nil { |
| - m.goodRecorded(host, next.Version) |
| + m.goodRecorded(host, next.Version, snap) |
| return |
| } |
| if !errors.Is(err, registry.ErrGoodBuildChanged) { |
| @@ -1130,9 +1168,10 @@ func (m *Manager) writeGoodLocked(host string, g activator.Gen) { |
| } |
| // goodRecorded follows a good-build write that landed (writeGoodLocked): |
| -// the hold-ran marker goes, and the queue learns the build. |
| -func (m *Manager) goodRecorded(host, ver string) { |
| - if _, err := m.d.Book.ClearHoldRan(host); err != nil { |
| +// a hold-ran marker older than the write's execution (snap) goes, and the |
| +// queue learns the build. |
| +func (m *Manager) goodRecorded(host, ver string, snap uint64) { |
| + if _, err := m.d.Book.ClearHoldRanBefore(host, snap); err != nil { |
| m.d.Logf("rollout: %s: clear hold-ran marker after good build %s: %v (cleared in memory)", host, ver, err) |
| } |
| m.observeGood(host, ver) |
| @@ -1209,18 +1248,25 @@ func (m *Manager) configFailure(host, rev, reason string) (time.Duration, bool) |
| return configBackoff(t.fails), true |
| } |
| -// cohortFailed runs the cohort-failure transition for s's app. |
| +// cohortFailed runs the cohort-failure transition for s's app. A failure of |
| +// an execution that began before a retry of its build runs none of it |
| +// (PreRetry, ruling 191). |
| func (m *Manager) cohortFailed(s *scheduler, rt registry.Route, rec FailureRecord) { |
| s.mu.Lock() |
| - added, superseded, err := failCohort(m.d.Book, s.q, rec) |
| + res, superseded, err := failCohort(m.d.Book, s.q, rec) |
| s.mu.Unlock() |
| if err != nil { |
| m.d.Logf("rollout: write failure record %s/%s %s: %v (held in memory)", rec.Account, rec.App, rec.Build, err) |
| } |
| + if res == PreRetry { |
| + m.d.Logf("rollout: %s/%s build %s failed on %s, but in an execution that began before a retry of the build: the retry stands", rec.Account, rec.App, rec.Build, rec.Host) |
| + s.poke() |
| + return |
| + } |
| if len(superseded) > 0 { |
| m.d.Logf("rollout: %s/%s build %s failed: superseding attempts past spawn on %v", rec.Account, rec.App, rec.Build, superseded) |
| } |
| - if added { |
| + if res == Recorded { |
| m.d.Logf("rollout: %s/%s build %s failed on %s: %s", rec.Account, rec.App, rec.Build, rec.Host, rec.Reason) |
| m.emit(rt, EvCohortHalted, "warn", textCohortHalted(rec.Build, m.d.Box, reasonWord(rec.Reason))) |
| } |
diff --git a/internal/rollout/queue.go b/internal/rollout/queue.go| index cfd535b2c..6c0c7e1aa 100644 |
| --- a/internal/rollout/queue.go |
| +++ b/internal/rollout/queue.go |
| @@ -51,6 +51,17 @@ type dispatch struct { |
| // ReplaceWorker): its execution's start, the At of a hold-ran marker it |
| // leaves (ruling 205, Task 3 fix round 1). Zero until then. |
| Started time.Time |
| + // BookSeq is the failure book's snapshot (FailureBook.Snapshot), taken |
| + // with Started: the order of this execution's start against the book's |
| + // markers and retry clears. Its good write clears only a marker set |
| + // before it (Astra R3 N6), and its failure is stale after a retry since |
| + // (ruling 191). Zero until then. |
| + BookSeq uint64 |
| + // ExecID is this execution's identity, "a<n>" from a Manager-wide counter |
| + // (Manager.nextExec), set with Started. A hold-ran marker it leaves |
| + // carries it (HoldRan.Exec). SchedRev cannot serve: an abandoned |
| + // attempt is derived again with the same revision. |
| + ExecID string |
| } |
| // hostSlot is one instance's desired state. The attempt to run is derived |
| @@ -485,7 +496,7 @@ func (q *appQueue) nextDue(now time.Time) time.Time { |
| // recorder is the failure book's write half, an interface so a test can |
| // watch the transition's order. |
| type recorder interface { |
| - Record(FailureRecord) (bool, error) |
| + Record(FailureRecord) (RecordResult, error) |
| } |
| // failCohort is the cohort-failure transition of spec §8.1, in its order: |
| @@ -498,8 +509,19 @@ type recorder interface { |
| // holds the scheduler's lock for q, so the record, the halt and the freed |
| // slot (finish) are one critical section. A failed file write still returns |
| // the record held in memory, so the transition never stops halfway. |
| -func failCohort(book recorder, q *appQueue, rec FailureRecord) (added bool, superseded []string, err error) { |
| - added, err = book.Record(rec) // rolloutsim:M3 |
| +// |
| +// A failure the book reports PreRetry began before a retry that has since |
| +// cleared its build (ruling 191): it is stale, and none of the transition |
| +// runs (plan review finding 3). Halting would stop the cohort the retry |
| +// reopened and supersede attempts the retry let start, though the record |
| +// that would justify it was never written. Every other result halts, an |
| +// Unchanged one included: the record it found still holds the build, and a |
| +// halt lives in memory beside it (an agent restart loses it). |
| +func failCohort(book recorder, q *appQueue, rec FailureRecord) (res RecordResult, superseded []string, err error) { |
| + res, err = book.Record(rec) // rolloutsim:M3 |
| + if res == PreRetry { |
| + return res, nil, err |
| + } |
| superseded = q.halt(rec.Build, rec.Reason) |
| - return added, superseded, err |
| + return res, superseded, err |
| } |
diff --git a/internal/rollout/scheduler.go b/internal/rollout/scheduler.go| index c2016ca50..4f7dbe4ed 100644 |
| --- a/internal/rollout/scheduler.go |
| +++ b/internal/rollout/scheduler.go |
| @@ -2,6 +2,7 @@ package rollout |
| import ( |
| "context" |
| + "strconv" |
| "sync" |
| "time" |
| @@ -135,8 +136,14 @@ func (s *scheduler) execute(ctx context.Context, d dispatch) { |
| } |
| // The execution's start, after every read and before the activator runs |
| // anything: a hold-ran marker this attempt leaves carries it (ruling 205), |
| - // so a rollback signed while it runs is newer than the marker. |
| + // so a rollback signed while it runs is newer than the marker. The book's |
| + // snapshot is taken at the same point, so both describe one start: the |
| + // attempt's good write clears only markers set before it (Astra R3 N6), |
| + // and its failure is stale after a retry since (ruling 191). ExecID names |
| + // this execution for a marker it leaves (plan review finding 4). |
| d.Started = m.d.Now() |
| + d.BookSeq = m.d.Book.Snapshot() |
| + d.ExecID = "a" + strconv.FormatUint(m.nextExec(), 10) |
| if rt.IsSidecar() { |
| // Workers and native sidecar members keep stop-then-start on the |
| // same entry (spec §7.9). done() runs the cohort-failure transition |
| @@ -178,7 +185,8 @@ func (s *scheduler) done(d dispatch, res activator.SwapResult, mode activator.Sw |
| var rec *FailureRecord |
| if failed { |
| r := FailureRecord{Account: s.key.acct, App: s.key.app, Build: d.ID.Build, Host: d.ID.Host, |
| - Cause: string(d.ID.Cause), Mode: string(mode), Reason: res.Reason, PromotedAt: d.FailedUnder, At: m.d.Now()} |
| + Cause: string(d.ID.Cause), Mode: string(mode), Reason: res.Reason, PromotedAt: d.FailedUnder, At: m.d.Now(), |
| + Seq: d.BookSeq, HoldRanAt: d.Started, HoldRanExec: d.ExecID} |
| if r.PromotedAt.IsZero() { |
| // No pointer named the build when the attempt started: the |
| // failure's own time, so only a pointer signed after it clears |
| @@ -218,14 +226,15 @@ func (s *scheduler) done(d dispatch, res activator.SwapResult, mode activator.Sw |
| // still holds its host's slot no newer attempt can be admitted and |
| // complete first, so this write can never land on top of a newer |
| // one. writeGood also requires the generation to be the one the |
| - // activator serves (acknowledged). |
| - m.served(d.ID.Host, res.Gen) |
| + // activator serves (acknowledged), and clears only a hold-ran marker |
| + // set before this attempt started (d.BookSeq, Astra R3 N6). |
| + m.served(d.ID.Host, res.Gen, d.BookSeq) |
| } |
| now := m.d.Now() |
| s.mu.Lock() |
| transition := s.q.finish(d, res.Outcome) |
| var ( |
| - added bool |
| + recorded RecordResult |
| superseded []string |
| recErr error |
| markErr error |
| @@ -241,15 +250,21 @@ func (s *scheduler) done(d dispatch, res activator.SwapResult, mode activator.Sw |
| heldDown := transition && rec != nil && len(rec.HeldDown) > 0 |
| mark := mode == activator.ModeHold && d.Class == classAdopt && res.Executed && |
| res.Outcome != activator.SwapDone && !heldDown |
| + stale := false |
| if transition && rec != nil { |
| // The whole cohort-failure transition (spec §8.1 steps 1 and 2): the |
| - // record (in memory at once, on disk as it can) and the halt. |
| + // record (in memory at once, on disk as it can) and the halt. None of |
| + // it runs for an attempt that began before a retry of its build |
| + // (PreRetry, ruling 191): the book sets its marker, if any, and |
| + // nothing else. |
| r := *rec |
| - r.MarkHoldRan, r.HoldRanAt = mark, d.Started |
| - added, superseded, recErr = failCohort(m.d.Book, s.q, r) |
| + r.MarkHoldRan = mark |
| + recorded, superseded, recErr = failCohort(m.d.Book, s.q, r) |
| + stale = recorded == PreRetry |
| } else if mark { |
| - markErr = m.d.Book.MarkHoldRan(d.ID.Host, HoldRan{Account: s.key.acct, App: s.key.app, Build: d.ID.Build, At: d.Started}) |
| + markErr = m.d.Book.MarkHoldRan(d.ID.Host, HoldRan{Account: s.key.acct, App: s.key.app, Build: d.ID.Build, Exec: d.ExecID, At: d.Started}) |
| } |
| + added := transition && rec != nil && recorded == Recorded |
| if res.Outcome == activator.SwapAbandoned && !res.Executed { |
| // It is proven that nothing of the build ran (a refusal before its |
| // exec, a staging refusal, an unreadable row): a retried host's |
| @@ -282,6 +297,9 @@ func (s *scheduler) done(d dispatch, res activator.SwapResult, mode activator.Sw |
| if recErr != nil { |
| m.d.Logf("rollout: write failure record %s/%s %s: %v (held in memory)", rec.Account, rec.App, rec.Build, recErr) |
| } |
| + if stale { |
| + m.d.Logf("rollout: %s: %s attempt onto %s failed, but it began before a retry of the build: the retry stands", d.ID.Host, d.ID.Cause, d.ID.Build) |
| + } |
| if markErr != nil { |
| m.d.Logf("rollout: %s: write hold-ran marker for %s: %v (held in memory)", d.ID.Host, d.ID.Build, markErr) |
| } |
diff --git a/internal/rollout/sim_test.go b/internal/rollout/sim_test.go| index 5d76141f5..fa65fd46b 100644 |
| --- a/internal/rollout/sim_test.go |
| +++ b/internal/rollout/sim_test.go |
| @@ -63,6 +63,7 @@ import ( |
| // first wake ended with no verdict after its exec), no wake of that |
| // instance boots its good build in place of another installed build, |
| // as a recovery or a fallback, until the instance records a good build |
| +// from an execution that began after the hold build ran (Astra R3 N6) |
| // or a host retry names it (spec §6.4). A Pass may also lift the |
| // restriction, but only on rollback evidence (ruling 205): the |
| // instance's installed build is its good build, and its channel's |
| @@ -277,6 +278,18 @@ type sim struct { |
| // pointer must be newer than. |
| holdRan map[string]string |
| holdRanAt map[string]time.Time |
| + // holdRanSet is when the restriction was last set (the step), which a |
| + // good write must come after the start of: a good build whose execution |
| + // began before a hold build ran on the host never lifts the restriction |
| + // (Astra R3 N6). |
| + holdRanSet map[string]time.Time |
| + // Retry ordering (ruling 191). retryAt is each host's latest `carlos |
| + // instances retry` of a build, clearedAt the latest Pass that cleared a |
| + // build's record on a `carlos promote --retry` pointer. A failure of an |
| + // execution that started before either is stale: it changes nothing a |
| + // retry cleared and holds nothing down. |
| + retryAt map[string]map[string]time.Time |
| + clearedAt map[string]time.Time |
| swaps map[string]*simSwap |
| events chan simEvent |
| @@ -290,7 +303,8 @@ func newSim(seed int64, opt string, staged map[string]bool) *sim { |
| pointers: map[string]release.ChannelPointer{}, builds: map[string]*simBuild{}, |
| lastStamp: map[string]time.Time{}, lastSigned: map[string]time.Time{}, failedEver: map[string]bool{}, |
| exempt: map[string]map[string]bool{}, heldDown: map[string]map[string]bool{}, holdRan: map[string]string{}, |
| - holdRanAt: map[string]time.Time{}, swaps: map[string]*simSwap{}, events: make(chan simEvent)} |
| + holdRanAt: map[string]time.Time{}, holdRanSet: map[string]time.Time{}, retryAt: map[string]map[string]time.Time{}, |
| + clearedAt: map[string]time.Time{}, swaps: map[string]*simSwap{}, events: make(chan simEvent)} |
| s.book.now = func() time.Time { return s.now } |
| s.hold = s.r.Intn(4) == 0 |
| s.concurrency = 1 + s.r.Intn(3) |
| @@ -416,8 +430,17 @@ func (s *sim) expectHold(build string) bool { |
| return s.hold || s.builds[build].holdDemand |
| } |
| -// failure is the ground truth of a build that ran and failed on host. |
| -func (s *sim) failure(build, host string, stamp time.Time, heldDown bool) { |
| +// failure is the ground truth of a build that ran and failed on host, in the |
| +// execution that started at start. One that started before a retry of the |
| +// build since (the host's, or a Pass's clear on a re-signed pointer) is |
| +// stale (ruling 191): it stands for nothing, as if the retry had come after |
| +// it, and holds nothing down. (A hold-ran restriction the run leaves is the |
| +// caller's, as for any run.) |
| +func (s *sim) failure(build, host string, stamp time.Time, heldDown bool, start time.Time) { |
| + if !start.IsZero() && (start.Before(s.retryAt[host][build]) || start.Before(s.clearedAt[build])) { |
| + s.logf("ground truth: %s failed on %s in an execution started at %s, before a retry of it: stale", build, host, start.Sub(t0)) |
| + return |
| + } |
| if stamp.IsZero() { |
| stamp = s.now |
| } |
| @@ -529,7 +552,7 @@ func (s *sim) finishSwap(sw *simSwap) activator.SwapResult { |
| } |
| s.logf("swap %s %s %s (%s): %s", a.Host, a.Cause, a.Build, a.Mode, res.Outcome) |
| if fresh { |
| - s.failure(a.Build, a.Host, sw.stamp, res.Outcome == activator.SwapDown) |
| + s.failure(a.Build, a.Host, sw.stamp, res.Outcome == activator.SwapDown, sw.started) |
| } |
| if res.Outcome == activator.SwapFailed && a.Mode == activator.ModeHold && !sw.serves { |
| // A hold adoption that ran, failed and was not held down. |
| @@ -547,7 +570,7 @@ func (s *sim) finishSwap(sw *simSwap) activator.SwapResult { |
| } |
| g := s.newGen(release.StagedIdentity{Version: a.Build, StagedPath: a.StagedPath, Digest: a.Digest, Demand: a.Demand}) |
| h.phase, h.gen = simAwake, g |
| - s.goodAt(h, a.Build) |
| + s.goodAt(h, a.Build, sw.started) |
| h.served[a.Build] = true |
| res.Outcome, res.Gen = activator.SwapDone, g |
| s.logf("swap %s %s %s (%s): done", a.Host, a.Cause, a.Build, a.Mode) |
| @@ -670,6 +693,12 @@ func (s *sim) pick(ok func(h *simHost) bool) *simHost { |
| // Pass may clear is one a signed rollback lifts (applyPointers, ruling 205), |
| // so each marker it cleared is checked against the model's evidence, then |
| // lifted in the model too. |
| +// |
| +// A record the Pass cleared while a row still installs its build was cleared |
| +// by a re-signed pointer (a prune drops only builds no row installs): that |
| +// is the retry ordering's app-wide clear (ruling 191), noted as clearedAt. |
| +// (Read from the book like the markers; the model's stands rule already |
| +// says whether it may go.) |
| func (s *sim) pass() { |
| marked := map[string]bool{} |
| for _, h := range s.hosts { |
| @@ -677,8 +706,26 @@ func (s *sim) pass() { |
| marked[h.name] = true |
| } |
| } |
| + recorded := map[string]bool{} |
| + for _, r := range s.book.List() { |
| + recorded[r.Build] = true |
| + } |
| rs, _ := s.rows.List() |
| s.m.Pass(rs) |
| + installed := map[string]bool{} |
| + for _, r := range rs { |
| + installed[r.Version] = true |
| + } |
| + still := map[string]bool{} |
| + for _, r := range s.book.List() { |
| + still[r.Build] = true |
| + } |
| + for _, b := range sortedKeys(recorded) { |
| + if !still[b] && installed[b] { |
| + s.clearedAt[b] = s.now |
| + s.logf("ground truth: a pass cleared %s's record on a re-signed pointer", b) |
| + } |
| + } |
| for _, h := range s.hosts { |
| if _, still := s.book.HoldRanOn(h.name); !marked[h.name] || still { |
| continue |
| @@ -814,21 +861,29 @@ func (s *sim) heldDownOn(host, build string) bool { |
| // holdRanOn is the ground truth of a hold build that ran on h without |
| // passing or being held down there (N2), in the execution that started at |
| -// start. The stamp is the first run's: running the same build again under |
| -// its own restriction leaves the book's marker, and its stamp, as they were. |
| +// start. Each execution renews the restriction (plan review finding 4): its |
| +// stamp is the latest start of the build's runs, so a rollback signed |
| +// between two runs lifts nothing, and it is set again now, so a good write |
| +// decided before now lifts nothing either. |
| func (s *sim) holdRanOn(h *simHost, build string, start time.Time) { |
| - if s.holdRan[h.name] != build { |
| + if s.holdRan[h.name] != build || start.After(s.holdRanAt[h.name]) { |
| s.holdRanAt[h.name] = start |
| } |
| s.holdRan[h.name] = build |
| + s.holdRanSet[h.name] = s.now |
| s.logf("ground truth: hold build %s ran on %s without a verdict", build, h.name) |
| } |
| // goodAt is h recording build as its good build (the code's good-build |
| -// write), which lifts any hold-ran restriction. |
| -func (s *sim) goodAt(h *simHost, build string) { |
| +// write) for the execution that started at start. It lifts a hold-ran |
| +// restriction set before that start, and only such a one (Astra R3 N6): a |
| +// hold build that ran on the host after the passing execution began is not |
| +// one the instance has passed over. |
| +func (s *sim) goodAt(h *simHost, build string, start time.Time) { |
| h.good = build |
| - delete(s.holdRan, h.name) |
| + if s.holdRanSet[h.name].Before(start) { |
| + delete(s.holdRan, h.name) |
| + } |
| } |
| // checkI5 is I5 at the exec of build on h, for a wake planned against rt. |
| @@ -964,7 +1019,7 @@ func (s *sim) wakeExec() { |
| s.checkI5(h, rt, good.Version) |
| h.phase, h.gen = simAwake, s.newGen(good) |
| h.served[good.Version] = true |
| - s.goodAt(h, good.Version) // served: the good build is written again |
| + s.goodAt(h, good.Version, h.planAt) // served: the good build is written again |
| s.logf("wake %s: %s untried, booted the good build %s", h.name, plan.Boot.Version, good.Version) |
| s.m.OnGen(activator.GenEvent{Kind: activator.GenHealthy, Route: rt, Gen: h.gen, Mode: plan.Mode, At: s.now, Token: plan.Token, ConfigRev: plan.ConfigRev}) |
| return |
| @@ -978,7 +1033,7 @@ func (s *sim) wakeExec() { |
| } |
| if !plan.FirstWake { |
| h.phase, h.gen = simAwake, s.newGen(boot) |
| - s.goodAt(h, boot.Version) |
| + s.goodAt(h, boot.Version, h.planAt) |
| h.served[boot.Version] = true |
| s.logf("wake %s: booted %s", h.name, boot.Version) |
| s.m.OnGen(activator.GenEvent{Kind: activator.GenHealthy, Route: rt, Gen: h.gen, Mode: plan.Mode, At: s.now, Token: plan.Token, ConfigRev: plan.ConfigRev}) |
| @@ -1014,7 +1069,7 @@ func (s *sim) wakeExec() { |
| case canFall && bootable(plan, plan.Fallback.Version) != nil: |
| // B failed, and its fallback's own gate refused the good build at |
| // its exec: B's failure is recorded, nothing fell back. |
| - s.failure(boot.Version, h.name, h.stamp, false) |
| + s.failure(boot.Version, h.name, h.stamp, false, h.planAt) |
| h.phase, h.gen = simAsleep, activator.Gen{} |
| s.logf("wake %s: first wake of %s failed, its fallback %s refused at its exec", h.name, boot.Version, plan.Fallback.Version) |
| s.m.OnGen(activator.GenEvent{Kind: activator.GenFailed, Route: rt, Failed: boot, Mode: plan.Mode, |
| @@ -1024,14 +1079,14 @@ func (s *sim) wakeExec() { |
| } |
| case canFall: |
| s.checkI5(h, rt, plan.Fallback.Version) |
| - s.failure(boot.Version, h.name, h.stamp, false) |
| + s.failure(boot.Version, h.name, h.stamp, false, h.planAt) |
| h.phase, h.gen = simAwake, s.newGen(*plan.Fallback) |
| s.logf("wake %s: first wake of %s failed, fell back to %s", h.name, boot.Version, plan.Fallback.Version) |
| s.m.OnGen(activator.GenEvent{Kind: activator.GenFellBack, Route: rt, Gen: h.gen, Failed: boot, Mode: plan.Mode, |
| FirstWake: true, Reason: "health check failed", At: s.now, FailedUnder: plan.FailedUnder, Token: plan.Token, ConfigRev: plan.ConfigRev}) |
| default: |
| hold := plan.Mode == activator.ModeHold |
| - s.failure(boot.Version, h.name, h.stamp, hold) |
| + s.failure(boot.Version, h.name, h.stamp, hold, h.planAt) |
| h.phase, h.gen = simAsleep, activator.Gen{} |
| s.logf("wake %s: first wake of %s failed, nothing fell back (mode %s)", h.name, boot.Version, plan.Mode) |
| s.m.OnGen(activator.GenEvent{Kind: activator.GenFailed, Route: rt, Failed: boot, Mode: plan.Mode, |
| @@ -1050,7 +1105,7 @@ func (s *sim) observeEnd() { |
| } |
| rt, plan := h.planRoute, h.plan |
| if h.failLater { |
| - s.failure(plan.Boot.Version, h.name, h.stamp, plan.Mode == activator.ModeHold) |
| + s.failure(plan.Boot.Version, h.name, h.stamp, plan.Mode == activator.ModeHold, h.planAt) |
| h.phase, h.gen = simAsleep, activator.Gen{} |
| s.logf("wake %s: %s exited inside its observation", h.name, plan.Boot.Version) |
| s.m.OnGen(activator.GenEvent{Kind: activator.GenFailed, Route: rt, Failed: plan.Boot, Mode: plan.Mode, FirstWake: true, |
| @@ -1058,7 +1113,7 @@ func (s *sim) observeEnd() { |
| return |
| } |
| h.phase = simAwake |
| - s.goodAt(h, h.gen.Build) |
| + s.goodAt(h, h.gen.Build, h.planAt) |
| h.served[h.gen.Build] = true |
| s.logf("wake %s: %s settled", h.name, h.gen.Build) |
| s.m.OnGen(activator.GenEvent{Kind: activator.GenSettled, Route: rt, Gen: h.gen, Mode: plan.Mode, FirstWake: true, At: s.now, Token: plan.Token, ConfigRev: plan.ConfigRev}) |
| @@ -1140,6 +1195,10 @@ func (s *sim) hostRetry() { |
| s.exempt[h.name] = map[string]bool{} |
| } |
| s.exempt[h.name][b] = true |
| + if s.retryAt[h.name] == nil { |
| + s.retryAt[h.name] = map[string]time.Time{} |
| + } |
| + s.retryAt[h.name][b] = s.now |
| delete(s.heldDown[h.name], b) |
| delete(s.holdRan, h.name) |
| s.logf("instances retry %s onto %s: %v", h.name, b, ok) |
diff --git a/internal/rollout/testdata/rolloutsim/mutate.sh b/internal/rollout/testdata/rolloutsim/mutate.sh| index ac7b0a3cb..65b232cf7 100755 |
| --- a/internal/rollout/testdata/rolloutsim/mutate.sh |
| +++ b/internal/rollout/testdata/rolloutsim/mutate.sh |
| @@ -57,7 +57,7 @@ ONLY=" $* " |
| # rolloutsim:M10. The substitution is the last field, so it may contain "|". |
| MUTANTS='M1|queue.go|I2|s/if q\.running >= q\.concurrency \{/if false \&\& q.running >= q.concurrency {/ |
| M2|queue.go|I3|s/class := classRecover/class := classAdopt/ |
| -M3|queue.go|I1|s/added, err = book\.Record\(rec\)/added, err = true, nil/ |
| +M3|queue.go|I1|s/res, err = book\.Record\(rec\)/res, err = Recorded, nil/ |
| M4|queue.go|I4|s/h\.buildRev = q\.nextRev\(\)/_ = q.nextRev()/ |
| M5|wakeplan.go|I1|s/if hasGood \&\& in\.Book\.Blocks\(/if false \&\& in.Book.Blocks(/ |
| M6|wakeplan.go|I1|s/plan\.Fallback = \&good/plan.Fallback = nil/' |
diff --git a/internal/rollout/wakeplan.go b/internal/rollout/wakeplan.go| index 91f055cca..b40a8d505 100644 |
| --- a/internal/rollout/wakeplan.go |
| +++ b/internal/rollout/wakeplan.go |
| @@ -205,10 +205,14 @@ func (m *Manager) Plan(ctx context.Context, rt registry.Route) (plan activator.W |
| return activator.WakePlan{} |
| } |
| tok, planAt := m.newWakeToken(rt.Host) |
| + // The book's snapshot, with the plan time: the start of the wake's |
| + // execution. Its good write clears only markers set before it (Astra R3 |
| + // N6), and its failure is stale after a retry since (ruling 191). |
| + snap := m.d.Book.Snapshot() |
| defer func() { |
| plan.Token = tok |
| m.notePlanned(rt, plan, tok) |
| - m.noteWakeStart(rt.Host, tok, planAt, plan) |
| + m.noteWakeStart(rt.Host, tok, planAt, snap, plan) |
| }() |
| if m.attemptRunning(rt.Host) { |
| // The instance's attempt has not reported yet (spec §6.4, §8.1): a |
| @@ -389,10 +393,13 @@ func (m *Manager) attemptRunning(host string) bool { |
| // activator serves (acknowledged). A token of zero matches nothing. |
| func (m *Manager) OnGen(ev activator.GenEvent) { |
| rt := ev.Route |
| - // The wake's plan time, the start of its execution: a hold-ran marker it |
| - // leaves carries it (ruling 205). Every kind but healthy is the wake's |
| - // last event, which lets the entry go. |
| - started := m.wakeStartedAt(rt.Host, ev.Token, ev.Kind != activator.GenHealthy) |
| + // The wake's start, its plan: a hold-ran marker it leaves carries the |
| + // plan time (ruling 205), and its good write and its failure are ordered |
| + // against the book by the plan's snapshot (Astra R3 N6, ruling 191). |
| + // Every kind but a first wake's healthy is the wake's last event, which |
| + // lets the entry go: an ordinary wake (an untried first wake's good |
| + // build included) reports healthy once and nothing after. |
| + started := m.wakeStarted(rt.Host, ev.Token, ev.Kind != activator.GenHealthy || !ev.FirstWake) |
| m.mu.Lock() |
| newest := ev.Token != 0 && m.wakeTok[rt.Host] == ev.Token |
| // The wake has an outcome: its build is no longer only planned, so its |
| @@ -423,23 +430,23 @@ func (m *Manager) OnGen(ev activator.GenEvent) { |
| // untried under Task 9's verdict floor: an ordinary wake of the good |
| // build, recorded as served, with nothing against the build it skipped. |
| if !ev.FirstWake { |
| - m.served(rt.Host, ev.Gen) |
| + m.served(rt.Host, ev.Gen, started.snap) |
| } |
| // A first wake is published when it settles: until then the |
| // activator hides it (Current), so it is not yet acknowledged. |
| m.wakeConfigActivated(rt.Host, ev.Token, ev.ConfigRev) |
| case activator.GenSettled: |
| - m.served(rt.Host, ev.Gen) |
| + m.served(rt.Host, ev.Gen, started.snap) |
| case activator.GenFellBack: |
| // The failure hold first (spec §8.1): nothing that reads the |
| // serving build or the active config may see the fallback before the |
| // failed build is held. |
| - m.buildFailed(rt, ev) |
| + m.buildFailed(rt, ev, started) |
| m.publishServing(rt.Host, ev.Gen) |
| m.wakeConfigActivated(rt.Host, ev.Token, ev.ConfigRev) |
| m.emit(rt, EvFallbackServed, "warn", textFallback(ev.Failed.Version, ev.Gen.Build)) |
| case activator.GenFailed: |
| - m.buildFailed(rt, ev) |
| + m.buildFailed(rt, ev, started) |
| case activator.GenCancelled: |
| // An intentional stop ended the first wake's observation, or the |
| // wake's lifecycle ended it after its build ran (spec §6.4): no |
| @@ -451,10 +458,10 @@ func (m *Manager) OnGen(ev activator.GenEvent) { |
| if ev.Executed && ev.FirstWake && ev.Mode == activator.ModeHold && ev.Gen.Build != "" { |
| // At is the wake's plan time, its execution's start (ruling 205): |
| // a rollback signed while the wake ran is newer than the marker. |
| - if started.IsZero() { |
| + if started.at.IsZero() { |
| m.d.Logf("rollout: %s: the start of the wake that ran %s is not known; its hold-ran marker is stamped now", rt.Host, ev.Gen.Build) |
| } |
| - mark := HoldRan{Account: release.RouteAccount(rt), App: rt.App, Build: ev.Gen.Build, At: started} |
| + mark := HoldRan{Account: release.RouteAccount(rt), App: rt.App, Build: ev.Gen.Build, Exec: wakeExecID(ev.Token), At: started.at} |
| if err := m.d.Book.MarkHoldRan(rt.Host, mark); err != nil { |
| m.d.Logf("rollout: %s: write hold-ran marker for %s: %v (held in memory)", rt.Host, mark.Build, err) |
| } |
| @@ -486,13 +493,15 @@ func (m *Manager) wakeConfigActivated(host string, tok uint64, rev string) { |
| } |
| // buildFailed is a failed first wake's cohort-failure transition (spec |
| -// §8.1, §8.2 step 4). |
| -func (m *Manager) buildFailed(rt registry.Route, ev activator.GenEvent) { |
| +// §8.1, §8.2 step 4). started is the wake's start (wakeStarted): a wake |
| +// planned before a retry of its build fails stale (ruling 191), and runs |
| +// none of the transition. |
| +func (m *Manager) buildFailed(rt registry.Route, ev activator.GenEvent, started wakeStart) { |
| // No blocking read before the record (spec §8.1): the failing pointer's |
| // PromotedAt was read when the wake was planned (WakePlan.FailedUnder). |
| rec := FailureRecord{Account: release.RouteAccount(rt), App: rt.App, Build: ev.Failed.Version, Host: rt.Host, |
| Cause: string(activator.CauseRelease), Mode: string(ev.Mode), Reason: ev.Reason, |
| - PromotedAt: ev.FailedUnder, At: m.d.Now()} |
| + PromotedAt: ev.FailedUnder, At: m.d.Now(), Seq: started.snap} |
| if rec.PromotedAt.IsZero() { |
| // No pointer named the build when the wake was planned: the |
| // failure's own time, so only a pointer signed after it clears the |
diff --git a/internal/rollout/wakeplan_test.go b/internal/rollout/wakeplan_test.go| index dcb0a8eab..eb65428b9 100644 |
| --- a/internal/rollout/wakeplan_test.go |
| +++ b/internal/rollout/wakeplan_test.go |
| @@ -211,7 +211,7 @@ func TestTheSchedulerLeavesAWorkersServingVersionAlone(t *testing.T) { |
| h.act.serve("a", "v2") |
| g, _ := h.act.Current("a") |
| g.Worker = true |
| - h.m.served("a", g) |
| + h.m.served("a", g, 0) |
| if r := h.rows.get("a"); r.ServingVersion != "" || r.GoodVersion != "v2" { |
| t.Fatalf("row = %+v: want the good build recorded and the serving build left to the activator", r) |
| } |
| @@ -228,8 +228,8 @@ func TestAStaleSuccessNeverOverwritesANewerBuild(t *testing.T) { |
| older, _ := h.act.Current("a") |
| h.act.serve("a", "v3") // a newer generation took over |
| newer, _ := h.act.Current("a") |
| - h.m.served("a", newer) |
| - h.m.served("a", older) // the older one's completion arrives late |
| + h.m.served("a", newer, 0) |
| + h.m.served("a", older, 0) // the older one's completion arrives late |
| h.m.OnGen(activator.GenEvent{Kind: activator.GenSettled, Route: h.rows.get("a"), Gen: older, FirstWake: true}) |
| if r := h.rows.get("a"); r.GoodVersion != "v3" || r.ServingVersion != "v3" { |
| t.Fatalf("row = %+v: an older generation overwrote the newer good and serving build", r) |