amadan
zdd-readiness: a wake's book snapshot at its plan's end, retry clears only on an un-retry, stale hold-downs leave a marker (Task 4 fix round 1)
Finding 1: a planned wake's snapshot is taken in Plan's deferred function, after decideWake's book reads and the replan, so a retry landing during Materialize or the policy load leaves the retried build's failure standing; the plan time (At) stays at the plan's start. Finding 2: RetryHost notes a host clear only when it un-retries the host, so a second retry during the first retry's canary no longer makes the canary's failure stale and strands the host (deviation 3 reverted). Finding 3: a stale failure that held its host down sets the host's marker with the execution's Exec and start, in done() and buildFailed alike (via Record); the simulation now drops a pruned build's clears and notes a host clear only when the code's record still lists the host as unretried. Finding 4: markLocked keeps the later At whatever the build.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8 files changed,
+207
−65
internal/rollout/bookseq_test.go+97 −25internal/rollout/failures.go+22 −20internal/rollout/fake_test.go+9 −0internal/rollout/holdran_test.go+8 −0internal/rollout/manager.go+7 −2internal/rollout/scheduler.go+5 −2internal/rollout/sim_test.go+46 −10internal/rollout/wakeplan.go+13 −6
diff --git a/internal/rollout/bookseq_test.go b/internal/rollout/bookseq_test.go| index dfe3bb732..3451d82c5 100644 |
| --- a/internal/rollout/bookseq_test.go |
| +++ b/internal/rollout/bookseq_test.go |
| @@ -3,6 +3,7 @@ package rollout |
| import ( |
| "context" |
| "slices" |
| + "sync" |
| "testing" |
| "time" |
| @@ -65,9 +66,16 @@ func TestAnotherExecutionOfTheMarkedBuildRenewsTheMarker(t *testing.T) { |
| // 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 { |
| + late, _ := b.HoldRanOn("h") |
| + if !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) |
| } |
| + // The same for another build's older execution (Task 4 fix round 1): |
| + // the marker names the newest report's build, never an older stamp. |
| + _ = b.MarkHoldRan("h", HoldRan{Account: "bab", App: "blog", Build: "v3", Exec: "a9", At: t0}) |
| + if other, _ := b.HoldRanOn("h"); other.Build != "v3" || !other.At.Equal(second) || other.Seq <= late.Seq { |
| + t.Fatalf("marker %+v after a late report of another build's earlier run: want v3's, stamped %v, newer sequence", other, second) |
| + } |
| } |
| // Retry ordering: a failure of an attempt that began before the host's retry |
| @@ -107,11 +115,11 @@ func TestAPreRetryFailureNeverRecreatesAClearedRecord(t *testing.T) { |
| } |
| } |
| -// 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) { |
| +// A pre-retry hold-down is a hold build that ran against the instance's |
| +// database, like a requested marker: the retry keeps the hold-down off, and |
| +// the instance keeps the marker (no automatic fallback onto it, spec §6.4; |
| +// Task 4 fix round 1), with the execution's identity and start. |
| +func TestAPreRetryHoldDownLeavesTheMarker(t *testing.T) { |
| b := &FailureBook{} |
| before := b.Snapshot() |
| b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s", Seq: before}) |
| @@ -122,30 +130,94 @@ func TestAPreRetryHoldDownLeavesWhatTheRetryLeaves(t *testing.T) { |
| 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) |
| + if !ok || got.Build != "v2" || got.Exec != "a7" || !got.At.Equal(started) { |
| + t.Fatalf("marker %+v, %v: want v2's from execution a7, 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") |
| +// Task 4 fix round 1, the reviewer's stranding sequence: a retry lets b's |
| +// canary through the halt; a second retry arrives while that canary runs; |
| +// the canary fails. Its failure began after the first retry, which is the |
| +// one it spends: it is recorded and b is held again, and a later retry runs |
| +// another attempt. A second retry of a host still retried notes no clear, |
| +// or the canary's failure would be stale, b would stay retried behind the |
| +// halt with its allowance spent, and no retry could free it. |
| +func TestASecondRetryDuringTheCanaryNeverStrandsTheHost(t *testing.T) { |
| + h := newHarness(t, "a", "b") |
| + h.stage("v2") |
| + h.failing.Store(true) |
| + h.act.outcome = func(a activator.Attempt) activator.SwapOutcome { |
| + if a.Build == "v2" && h.failing.Load() { |
| + return activator.SwapFailed |
| + } |
| + return activator.SwapDone |
| + } |
| + h.install("v2", "a", "b") |
| + h.pass() |
| + waitFor(t, "a failure record", func() bool { _, ok := h.book.Lookup("bab", "blog", "v2"); return ok }) |
| + gate := h.act.hold("b") |
| + if !h.m.RetryHost("b") { |
| + t.Fatal("the first retry cleared nothing") |
| + } |
| + waitFor(t, "b's retry canary", func() bool { |
| + _, started, _ := h.act.snapshot() |
| + return slices.Contains(started, "b:v2") |
| + }) |
| + h.m.RetryHost("b") // the second retry, while the canary runs |
| + close(gate) |
| + waitFor(t, "b's canary to fail", func() bool { return !h.m.attemptRunning("b") && h.book.Blocks("bab", "blog", "v2", "b", false) }) |
| + |
| + h.failing.Store(false) |
| + if !h.m.RetryHost("b") { |
| + t.Fatal("a retry after the canary's failure cleared nothing: b is stranded") |
| + } |
| + waitFor(t, "b on v2", func() bool { return h.servingAll("v2", "b") }) |
| + _, started, _ := h.act.snapshot() |
| + n := 0 |
| + for _, st := range started { |
| + if st == "b:v2" { |
| + n++ |
| + } |
| + } |
| + if n != 2 { |
| + t.Fatalf("attempts %v: want b's canary and the attempt after the third retry", started) |
| + } |
| +} |
| + |
| +// Task 4 fix round 1: a planned wake's snapshot is taken at the plan's end. |
| +// A retry that lands during the planner's slow reads (here the policy load) |
| +// is what let the plan boot the retried build, so the build's failure in |
| +// that wake is a real one: recorded, and in hold mode a hold-down. |
| +func TestARetryDuringThePlanLeavesTheWakesFailureStanding(t *testing.T) { |
| + ctx := context.Background() |
| + h := newHarness(t, "w") |
| + h.swap = appconfig.SwapHold |
| + h.stage("v2") |
| + h.install("v2", "w") |
| + h.sleepHost("w") |
| + h.pass() |
| + h.book.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "s", PromotedAt: t0, At: t0}) |
| + h.wakeHost("w") |
| + h.advance(time.Minute) // the policy cache is stale: the plan loads it |
| + var once sync.Once |
| + h.mu.Lock() |
| + h.policyHook = func() { |
| + once.Do(func() { |
| + if !h.m.RetryHost("w") { |
| + t.Error("the retry during the plan cleared nothing") |
| + } |
| + }) |
| + } |
| + h.mu.Unlock() |
| + p := h.m.Plan(ctx, h.rows.get("w")) |
| + if !p.FirstWake || p.Boot.Version != "v2" || p.Mode != activator.ModeHold { |
| + t.Fatalf("plan %+v: want a hold first wake of the retried v2", p) |
| } |
| - 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) |
| + h.m.OnGen(activator.GenEvent{Kind: activator.GenFailed, Route: h.rows.get("w"), Failed: p.Boot, Mode: p.Mode, |
| + FirstWake: true, Reason: "health check failed", Token: p.Token, ConfigRev: p.ConfigRev}) |
| + if !h.book.Blocks("bab", "blog", "v2", "w", false) || !h.book.HeldDown("bab", "blog", "v2", "w") { |
| + t.Fatal("the failure of a wake the retry let through was taken as pre-retry") |
| } |
| } |
diff --git a/internal/rollout/failures.go b/internal/rollout/failures.go| index f54eb73b2..4aef326ae 100644 |
| --- a/internal/rollout/failures.go |
| +++ b/internal/rollout/failures.go |
| @@ -78,7 +78,7 @@ const ( |
| // 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). |
| + // it carries, or one in place of its hold-down, is still set (Record). |
| PreRetry |
| ) |
| @@ -322,14 +322,18 @@ func (b *FailureBook) clearedLocked(acct, app, build, host string) { |
| // 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. |
| +// 3). What the execution did to the instance's database still stands: a |
| +// requested hold-ran marker is set, and so is one in place of the hold-down |
| +// r asked for (a hold build that ran there and went down, Task 4 fix round |
| +// 1), with the execution's identity and start (HoldRanExec, HoldRanAt), so |
| +// nothing falls back onto the instance automatically (spec §6.4). A stale |
| +// hold run that went down is restricted as one that was superseded is. |
| func (b *FailureBook) Record(r FailureRecord) (RecordResult, error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| 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}) |
| + mark := r.MarkHoldRan || (stale && slices.Contains(r.HeldDown, r.Host)) |
| + marked := mark && 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 { |
| @@ -378,10 +382,10 @@ func (b *FailureBook) Record(r FailureRecord) (RecordResult, error) { |
| // 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. |
| +// it clears nothing (ClearHoldRanBefore). It keeps the later of the two |
| +// stamps, whatever the builds (Task 4 fix round 1): a late report of an |
| +// older execution never moves At back, so a rollback signed between the two |
| +// runs never looks newer than the marker. The caller holds b.mu and saves. |
| func (b *FailureBook) markLocked(host string, h HoldRan) bool { |
| cur, ok := b.held[host] |
| if ok && cur.same(h) && h.Exec != "" && cur.Exec == h.Exec { |
| @@ -393,7 +397,7 @@ 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) { |
| + if ok && cur.At.After(h.At) { |
| h.At = cur.At |
| } |
| h.Seq = b.bumpLocked() |
| @@ -510,23 +514,21 @@ 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. |
| +// A retry that clears the host also notes a host clear at the book's next |
| +// sequence value (ruling 191): a failure of an execution of host that began |
| +// before it is then stale (Record), and takes nothing back. A retry of a |
| +// host already retried notes nothing (Task 4 fix round 1): the canary the |
| +// first retry let through may be running, and its failure must spend the |
| +// retry, or the host would be left neither held nor allowed through the |
| +// halt, with no later retry able to free 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 { |
| + if i < 0 || slices.Contains(b.recs[i].Retried, host) { |
| 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] |
| cur.Retried = append(slices.Clone(cur.Retried), host) |
| if j := slices.Index(cur.HeldDown, host); j >= 0 { |
diff --git a/internal/rollout/fake_test.go b/internal/rollout/fake_test.go| index 16f56f2ae..2e5918e17 100644 |
| --- a/internal/rollout/fake_test.go |
| +++ b/internal/rollout/fake_test.go |
| @@ -322,6 +322,9 @@ type harness struct { |
| // demandHook, when set, runs at the start of every demand read, with no |
| // lock held: a test's window into the planner's slow store read. |
| demandHook func(ver string) |
| + // policyHook, when set, runs at the start of every policy load, with no |
| + // lock held: a test's window into the planner's policy read. |
| + policyHook func() |
| } |
| func newHarness(t *testing.T, hosts ...string) *harness { |
| @@ -418,6 +421,12 @@ func (h *harness) demand(_ context.Context, _, _, ver, _ string) (string, string |
| } |
| func (h *harness) policy(context.Context, string, string) (*appconfig.Manifest, error) { |
| + h.mu.Lock() |
| + hook := h.policyHook |
| + h.mu.Unlock() |
| + if hook != nil { |
| + hook() |
| + } |
| h.mu.Lock() |
| defer h.mu.Unlock() |
| return &appconfig.Manifest{Schema: 1, App: "blog", Swap: h.swap, SwapConcurrency: h.concurrency}, nil |
diff --git a/internal/rollout/holdran_test.go b/internal/rollout/holdran_test.go| index 2ac2d71d9..fba7cfd68 100644 |
| --- a/internal/rollout/holdran_test.go |
| +++ b/internal/rollout/holdran_test.go |
| @@ -711,8 +711,16 @@ func TestTheHoldRanMarkerStampAndItsRollbackClear(t *testing.T) { |
| if got, _ := b.HoldRanOn("a"); got.Build != "v3" || !got.At.Equal(now) { |
| t.Fatalf("marker %+v: want v3's, stamped at %v", got, now) |
| } |
| + // A report of an execution that started before the standing marker's |
| + // (Task 4 fix round 1) names its build but keeps the later stamp: At |
| + // never moves back. |
| started := now.Add(-30 * time.Second) |
| b.MarkHoldRan("a", HoldRan{Account: "bab", App: "blog", Build: "v4", At: started}) |
| + if got, _ := b.HoldRanOn("a"); got.Build != "v4" || !got.At.Equal(now) { |
| + t.Fatalf("marker %+v: want v4's, keeping the later stamp %v", got, now) |
| + } |
| + b.ClearHoldRan("a") |
| + b.MarkHoldRan("a", HoldRan{Account: "bab", App: "blog", Build: "v4", At: started}) |
| if got, _ := b.HoldRanOn("a"); got.Build != "v4" || !got.At.Equal(started) { |
| t.Fatalf("marker %+v: want v4's, stamped at its execution's start %v, not the book's clock", got, started) |
| } |
diff --git a/internal/rollout/manager.go b/internal/rollout/manager.go| index ea942bf14..78efcedc3 100644 |
| --- a/internal/rollout/manager.go |
| +++ b/internal/rollout/manager.go |
| @@ -692,7 +692,8 @@ func wakeExecID(tok uint64) string { |
| } |
| // 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. |
| +// plan time (the plan's start) and the failure book's snapshot taken at the |
| +// plan's end, after the planner's book reads (Plan). |
| type wakeStart struct { |
| token uint64 |
| at time.Time |
| @@ -1250,7 +1251,8 @@ func (m *Manager) configFailure(host, rev, reason string) (time.Duration, bool) |
| // 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). |
| +// (PreRetry, ruling 191); a hold-down it asked for is a hold-ran marker |
| +// instead (FailureBook.Record), whose rollback read is then due. |
| func (m *Manager) cohortFailed(s *scheduler, rt registry.Route, rec FailureRecord) { |
| s.mu.Lock() |
| res, superseded, err := failCohort(m.d.Book, s.q, rec) |
| @@ -1260,6 +1262,9 @@ func (m *Manager) cohortFailed(s *scheduler, rt registry.Route, rec FailureRecor |
| } |
| 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) |
| + if slices.Contains(rec.HeldDown, rec.Host) { |
| + m.pointerReadDue(s.key) |
| + } |
| s.poke() |
| return |
| } |
diff --git a/internal/rollout/scheduler.go b/internal/rollout/scheduler.go| index 4f7dbe4ed..de5b7ea8b 100644 |
| --- a/internal/rollout/scheduler.go |
| +++ b/internal/rollout/scheduler.go |
| @@ -255,7 +255,8 @@ func (s *scheduler) done(d dispatch, res activator.SwapResult, mode activator.Sw |
| // 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. 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 |
| + // (PreRetry, ruling 191): the book sets its marker, if any, or one |
| + // in place of its hold-down (r.HoldRanExec, r.HoldRanAt), and |
| // nothing else. |
| r := *rec |
| r.MarkHoldRan = mark |
| @@ -289,7 +290,9 @@ func (s *scheduler) done(d dispatch, res activator.SwapResult, mode activator.Sw |
| serving = h.serving |
| } |
| s.mu.Unlock() |
| - if mark { |
| + if mark || (stale && heldDown) { |
| + // A marker was set: the requested one, or one the book set in place |
| + // of a stale hold-down (Task 4 fix round 1). |
| m.pointerReadDue(s.key) |
| } |
| defer s.poke() |
diff --git a/internal/rollout/sim_test.go b/internal/rollout/sim_test.go| index fa65fd46b..d3d0e1bbc 100644 |
| --- a/internal/rollout/sim_test.go |
| +++ b/internal/rollout/sim_test.go |
| @@ -6,6 +6,7 @@ import ( |
| "math/rand" |
| "os" |
| "runtime/debug" |
| + "slices" |
| "sort" |
| "strconv" |
| "strings" |
| @@ -433,12 +434,16 @@ func (s *sim) expectHold(build string) 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.) |
| +// stale (ruling 191): it stands for nothing the retry cleared, and a |
| +// hold-down it would have made leaves the hold-ran restriction instead (the |
| +// hold build ran against the database; Task 4 fix round 1). A restriction a |
| +// run leaves without a hold-down 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)) |
| + if heldDown { |
| + s.holdRanOn(s.byName[host], build, start) |
| + } |
| return |
| } |
| if stamp.IsZero() { |
| @@ -726,6 +731,27 @@ func (s *sim) pass() { |
| s.logf("ground truth: a pass cleared %s's record on a re-signed pointer", b) |
| } |
| } |
| + // The Pass's prune drops the retry clears of builds no row installs and |
| + // no planned first wake still carries (the hosts waking on a first-wake |
| + // plan: Manager.planned, once a pass has let go of the unowned ones). A |
| + // failure arriving after is fresh again (Task 4 fix round 1). |
| + for _, h := range s.hosts { |
| + if h.phase == simWaking && h.plan.FirstWake { |
| + installed[h.plan.Boot.Version] = true |
| + } |
| + } |
| + for _, b := range sortedKeys(s.clearedAt) { |
| + if !installed[b] { |
| + delete(s.clearedAt, b) |
| + } |
| + } |
| + for _, host := range sortedKeys(s.retryAt) { |
| + for _, b := range sortedKeys(s.retryAt[host]) { |
| + if !installed[b] { |
| + delete(s.retryAt[host], b) |
| + } |
| + } |
| + } |
| for _, h := range s.hosts { |
| if _, still := s.book.HoldRanOn(h.name); !marked[h.name] || still { |
| continue |
| @@ -862,11 +888,12 @@ 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. 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. |
| +// stamp is the latest start of the runs since it was last lifted, whatever |
| +// their builds (Task 4 fix round 1), 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 || start.After(s.holdRanAt[h.name]) { |
| + if s.holdRan[h.name] == "" || start.After(s.holdRanAt[h.name]) { |
| s.holdRanAt[h.name] = start |
| } |
| s.holdRan[h.name] = build |
| @@ -1190,15 +1217,24 @@ func (s *sim) hostRetry() { |
| if !s.stands(b) { |
| return |
| } |
| + // The retry orders later failures only when it un-retries the host (Task |
| + // 4 fix round 1): a record of b stands and does not already exempt h. |
| + // Whether the code's record still lists h as retried is read from the |
| + // book, like the clears pass() sees: the model's exempt outlives a record |
| + // that was cleared and written again. |
| + rec, recorded := s.book.Lookup("bab", "blog", b) |
| + clears := recorded && !slices.Contains(rec.Retried, h.name) |
| ok := s.m.RetryHost(h.name) |
| if s.exempt[h.name] == nil { |
| 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{} |
| + if clears { |
| + if s.retryAt[h.name] == nil { |
| + s.retryAt[h.name] = map[string]time.Time{} |
| + } |
| + s.retryAt[h.name][b] = s.now |
| } |
| - 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/wakeplan.go b/internal/rollout/wakeplan.go| index b40a8d505..401064810 100644 |
| --- a/internal/rollout/wakeplan.go |
| +++ b/internal/rollout/wakeplan.go |
| @@ -205,14 +205,18 @@ 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, snap, plan) |
| + // The book's snapshot is taken here, at the plan's end, after every |
| + // book read the planner made (decideWake, the replan): the wake's |
| + // failure is stale only after a retry the plan could not have seen |
| + // (ruling 191, Task 4 fix round 1). A retry landing during the slow |
| + // reads (Materialize, the policy load) let this plan boot the build, |
| + // so the build's failure in it is a real one. Its good write clears |
| + // only markers set before here (Astra R3 N6): the exec follows the |
| + // plan, and the plan's check re-reads the marker there. |
| + m.noteWakeStart(rt.Host, tok, planAt, m.d.Book.Snapshot(), plan) |
| }() |
| if m.attemptRunning(rt.Host) { |
| // The instance's attempt has not reported yet (spec §6.4, §8.1): a |
| @@ -501,7 +505,10 @@ func (m *Manager) buildFailed(rt registry.Route, ev activator.GenEvent, started |
| // 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(), Seq: started.snap} |
| + PromotedAt: ev.FailedUnder, At: m.d.Now(), |
| + // The wake's start for a stale failure's marker: Record sets one in |
| + // place of a hold-down when the failure is pre-retry (ruling 191). |
| + Seq: started.snap, HoldRanAt: started.at, HoldRanExec: wakeExecID(ev.Token)} |
| 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 |