amadan
zdd-5 task 10: keep a failure record while a planned wake still carries its build
A wake planned onto build B could boot B after B failed on a sibling: the record was written after the plan, a promotion moved every route off B, and the next Pass pruned the record because no route installed B. planCheck reads only the book, so the wake's exec then booted the failed B, in hold mode against its database (Task 16's simulation, I1, seeds 8, 70, 102). The Manager now tracks each host's in-flight planned first wake and the build it boots. prune counts those builds as installed. An entry clears on the wake's outcome (any OnGen for the host), on the host's next Plan, or on a Pass that finds the activator no longer owns the host (asked off every lock; an entry a newer plan replaced meanwhile is kept).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+154
−5
internal/rollout/manager.go+73 −3internal/rollout/wakeplan.go+11 −2internal/rollout/wakeplan_test.go+70 −0
diff --git a/internal/rollout/manager.go b/internal/rollout/manager.go| index 63d115e01..4f0b269be 100644 |
| --- a/internal/rollout/manager.go |
| +++ b/internal/rollout/manager.go |
| @@ -122,6 +122,16 @@ type Manager struct { |
| // observation: opened by a healthy first wake, closed by whatever ends |
| // it (OnGen). Status reports them as settling. |
| observing map[string]bool |
| + // planned holds each host whose planned first wake is in flight, and the |
| + // build it boots: from its Plan to its outcome (OnGen), its next Plan, or |
| + // the activator letting the host go (a Pass that finds Owns false). prune |
| + // keeps those builds' failure records, because a wake's last selection |
| + // check (planCheck) reads only the book: a record pruned after the plan, |
| + // once a promotion moved every route off the build, would let the wake |
| + // boot a build that failed on this box. plannedSeq tells one plan's entry |
| + // from the next for that Pass, which asks Owns off every lock. |
| + planned map[string]plannedWake |
| + plannedSeq uint64 |
| // goodMu serialises every serving- and good-build write with its |
| // acknowledgement check (served, publishServing, writeGood), so an older |
| @@ -139,7 +149,8 @@ func NewManager(ctx context.Context, d Deps) *Manager { |
| } |
| m := &Manager{d: d, ctx: ctx, |
| apps: map[appKey]*scheduler{}, hostApp: map[string]appKey{}, cfg: map[string]*configTrack{}, |
| - retried: map[string]string{}, pointerRead: map[appKey]time.Time{}, nudging: map[string]bool{}, observing: map[string]bool{}} |
| + retried: map[string]string{}, pointerRead: map[appKey]time.Time{}, nudging: map[string]bool{}, observing: map[string]bool{}, |
| + planned: map[string]plannedWake{}} |
| m.policies = &policyCache{load: d.Policy, ttl: 30 * time.Second, now: d.Now, m: map[appKey]policyEntry{}} |
| return m |
| } |
| @@ -196,6 +207,7 @@ func (m *Manager) Pass(routes []registry.Route) { |
| keys := m.observe(routes) |
| m.forget(routes) |
| m.applyPointers(routes, keys) |
| + m.dropUnownedWakes() |
| m.prune(routes) |
| m.liftClearedHalts() |
| } |
| @@ -533,6 +545,57 @@ func (m *Manager) failedUnder(rt registry.Route, build string) time.Time { |
| return time.Time{} |
| } |
| +// plannedWake is a planned first wake in flight (Manager.planned). |
| +type plannedWake struct { |
| + acct, app, build string |
| + seq uint64 |
| +} |
| + |
| +// notePlanned records what host's newest plan boots: an entry for a first |
| +// wake, none for anything else. Either way it replaces the host's previous |
| +// plan, whose wake is over or is being planned again. |
| +func (m *Manager) notePlanned(rt registry.Route, plan activator.WakePlan) { |
| + m.mu.Lock() |
| + defer m.mu.Unlock() |
| + if !plan.FirstWake || plan.Boot.Version == "" || plan.Refuse != "" { |
| + delete(m.planned, rt.Host) |
| + return |
| + } |
| + m.plannedSeq++ |
| + m.planned[rt.Host] = plannedWake{acct: release.RouteAccount(rt), app: rt.App, build: plan.Boot.Version, seq: m.plannedSeq} |
| +} |
| + |
| +// dropUnownedWakes forgets the planned wakes of hosts the activator no |
| +// longer owns: a wake that ended with no event (refused at its lease, |
| +// abandoned at its exec) leaves its entry behind, and only the entry's own |
| +// end may let the record go. Owns is asked with no lock held (the lock order |
| +// on Manager), and an entry is dropped only if no newer plan replaced it |
| +// meanwhile. |
| +func (m *Manager) dropUnownedWakes() { |
| + m.mu.Lock() |
| + seen := make(map[string]uint64, len(m.planned)) |
| + for host, w := range m.planned { |
| + seen[host] = w.seq |
| + } |
| + m.mu.Unlock() |
| + gone := map[string]uint64{} |
| + for host, seq := range seen { |
| + if !m.d.Act.Owns(host) { |
| + gone[host] = seq |
| + } |
| + } |
| + if len(gone) == 0 { |
| + return |
| + } |
| + m.mu.Lock() |
| + for host, seq := range gone { |
| + if w, ok := m.planned[host]; ok && w.seq == seq { |
| + delete(m.planned, host) |
| + } |
| + } |
| + m.mu.Unlock() |
| +} |
| + |
| // prune drops failure records for builds no route installs. A held-down |
| // instance's route still names its failed build, so its record stays (spec |
| // §8.4). Every route counts, whatever its status or shape: a row passing |
| @@ -541,8 +604,10 @@ func (m *Manager) failedUnder(rt registry.Route, build string) time.Time { |
| // then would lift the halt and let a later wake boot the failed hold build |
| // against its database. Pruning errs towards keeping. An empty route list is |
| // never authority to drop anything: registry.Open creates an empty database |
| -// on a wrong path. The installed callback runs under the book's lock and |
| -// reads only this map. |
| +// on a wrong path. A build a planned first wake still carries counts as |
| +// installed (Manager.planned): the wake was planned before the install moved |
| +// off it, and its exec may yet boot it. The installed callback runs under the |
| +// book's lock and reads only this map, built first. |
| func (m *Manager) prune(routes []registry.Route) { |
| if len(routes) == 0 { |
| return |
| @@ -553,6 +618,11 @@ func (m *Manager) prune(routes []registry.Route) { |
| installed[[3]string{release.RouteAccount(rt), rt.App, rt.Version}] = true |
| } |
| } |
| + m.mu.Lock() |
| + for _, w := range m.planned { |
| + installed[[3]string{w.acct, w.app, w.build}] = true |
| + } |
| + m.mu.Unlock() |
| if err := m.d.Book.Prune(func(acct, app, build string) bool { return installed[[3]string{acct, app, build}] }); err != nil { |
| m.d.Logf("rollout: prune failure records: %v", err) |
| } |
diff --git a/internal/rollout/wakeplan.go b/internal/rollout/wakeplan.go| index 099651ae2..842403b46 100644 |
| --- a/internal/rollout/wakeplan.go |
| +++ b/internal/rollout/wakeplan.go |
| @@ -91,10 +91,15 @@ func decideWake(in wakeInputs) (activator.WakePlan, error) { |
| // |
| // A Refuse text reaches the member's "wake failed" event (the activator's |
| // wake error), so it names builds only, never an error's text. |
| -func (m *Manager) Plan(ctx context.Context, rt registry.Route) activator.WakePlan { |
| +// |
| +// Every plan for a managed host, refused or not, replaces the host's entry |
| +// in Manager.planned, so prune keeps the failure record of a first wake's |
| +// build until that wake is over. |
| +func (m *Manager) Plan(ctx context.Context, rt registry.Route) (plan activator.WakePlan) { |
| if !Managed(rt) { |
| return activator.WakePlan{} |
| } |
| + defer func() { m.notePlanned(rt, plan) }() |
| if m.attemptRunning(rt.Host) { |
| // The instance's attempt has not reported yet (spec §6.4, §8.1): a |
| // hold that went down, or a swap whose entry stopped under it, is |
| @@ -135,7 +140,8 @@ func (m *Manager) Plan(ctx context.Context, rt registry.Route) activator.WakePla |
| t := m.track(rt.Host) |
| t.wakeRev = t.delivered |
| m.mu.Unlock() |
| - plan, err := decideWake(in) |
| + var err error |
| + plan, err = decideWake(in) |
| if err != nil { |
| m.stagingRefused(rt, rt.Version, err) |
| } |
| @@ -197,6 +203,9 @@ func (m *Manager) attemptRunning(host string) bool { |
| func (m *Manager) OnGen(ev activator.GenEvent) { |
| rt := ev.Route |
| m.mu.Lock() |
| + // The wake has an outcome: its build is no longer only planned, so its |
| + // failure record need not outlive the install for its sake. |
| + delete(m.planned, rt.Host) |
| switch { |
| case ev.Kind == activator.GenHealthy && ev.FirstWake: |
| m.observing[rt.Host] = true |
diff --git a/internal/rollout/wakeplan_test.go b/internal/rollout/wakeplan_test.go| index 0b3adca5c..b64f89735 100644 |
| --- a/internal/rollout/wakeplan_test.go |
| +++ b/internal/rollout/wakeplan_test.go |
| @@ -562,3 +562,73 @@ func TestNudgesForOneHostCollapse(t *testing.T) { |
| t.Fatalf("calls %+v started %v: repeated nudges re-queued or superseded the running attempt", calls, started) |
| } |
| } |
| + |
| +// wakeHost makes host a sleeper whose wake is in flight: no current |
| +// generation, but the activator owns its entry, as it does from the wake's |
| +// plan to its outcome. |
| +func (h *harness) wakeHost(host string) { |
| + h.act.mu.Lock() |
| + defer h.act.mu.Unlock() |
| + delete(h.act.gens, host) |
| + h.act.owned[host] = true |
| +} |
| + |
| +// I1 (Task 16's simulation, seeds 8, 70 and 102): a wake planned onto build |
| +// B, a sibling's failure of B recorded while the wake is still on its way to |
| +// the exec, then a promotion that moves every route on the box off B. The |
| +// next Pass must not prune B's record: the plan's check reads only the book, |
| +// so a pruned record let the wake boot the failed B, in hold mode against |
| +// its database. The record goes once nothing planned still carries B: the |
| +// wake's outcome, a newer plan for the host, or the activator letting the |
| +// host go. |
| +func TestAPlannedWakeKeepsItsBuildsFailureRecordThroughAPrune(t *testing.T) { |
| + sibling := FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "b", At: t0} |
| + setup := func(t *testing.T) (*harness, activator.WakePlan) { |
| + h := newHarness(t, "a") |
| + h.stage("v2", "v3") |
| + h.install("v2", "a") |
| + h.wakeHost("a") |
| + p := h.m.Plan(context.Background(), h.rows.get("a")) |
| + if !p.FirstWake || p.Boot.Version != "v2" || p.Check == nil { |
| + t.Fatalf("plan %+v: want a first wake onto v2 with its check", p) |
| + } |
| + if _, err := h.book.Record(sibling); err != nil { |
| + t.Fatal(err) |
| + } |
| + h.install("v3", "a") // the promotion: no route on the box installs v2 now |
| + h.pass() |
| + if _, ok := h.book.Lookup("bab", "blog", "v2"); !ok { |
| + t.Fatal("the Pass pruned v2's record while a planned wake still carried v2") |
| + } |
| + if err := p.Check("v2"); err == nil { |
| + t.Fatal("the wake's check let v2 boot: it failed on a sibling after the plan") |
| + } |
| + return h, p |
| + } |
| + pruned := func(t *testing.T, h *harness, why string) { |
| + t.Helper() |
| + h.pass() |
| + if _, ok := h.book.Lookup("bab", "blog", "v2"); ok { |
| + t.Fatalf("%s, and the next Pass still kept v2's record", why) |
| + } |
| + } |
| + t.Run("the wake's outcome", func(t *testing.T) { |
| + h, _ := setup(t) |
| + h.m.OnGen(activator.GenEvent{Kind: activator.GenCancelled, Route: h.rows.get("a"), FirstWake: true}) |
| + pruned(t, h, "the wake reported its outcome") |
| + }) |
| + t.Run("a newer plan", func(t *testing.T) { |
| + h, _ := setup(t) |
| + if p := h.m.Plan(context.Background(), h.rows.get("a")); p.Boot.Version != "v3" { |
| + t.Fatalf("replanned %+v, want v3", p) |
| + } |
| + pruned(t, h, "the host's newer plan boots v3") |
| + }) |
| + t.Run("the activator let the host go", func(t *testing.T) { |
| + h, _ := setup(t) |
| + h.act.mu.Lock() |
| + delete(h.act.owned, "a") // the wake ended with no event: refused at its lease |
| + h.act.mu.Unlock() |
| + pruned(t, h, "the activator no longer owns the host") |
| + }) |
| +} |