amadan

carlos / platform Public

zdd-5 task 8: a sleeper's pending config is dropped at delivery; prune keeps every route's build; lock-order note

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 9260362da7df5704bfb9493da82835ee4be78f45 parent e51bd9b
3 files changed, +102 −9
  • internal/rollout/manager.go +27 −9
  • internal/rollout/manager_test.go +66 −0
  • internal/rollout/queue.go +9 −0
diff --git a/internal/rollout/manager.go b/internal/rollout/manager.go
index 11b4165ba..4a349ed73 100644
--- a/internal/rollout/manager.go
+++ b/internal/rollout/manager.go
@@ -86,9 +86,15 @@ const abandonDeferral = 15 * time.Second
// Manager is the box's rollout scheduler: one goroutine per (account, app),
// fed by the agent pass, the adopt pass, `carlos restart` and config sync.
//
-// Lock order: activator.Service.mu, then Manager.mu, then scheduler.mu, then
-// FailureBook.mu. The activator (Current, Owns, Swap, ReplaceWorker) is never
-// called while Manager.mu or a scheduler.mu is held.
+// Lock order: goodMu, then activator.Service.mu, then Manager.mu, then
+// scheduler.mu, then FailureBook.mu. The activator (Current, Owns, Swap,
+// ReplaceWorker) is never called while Manager.mu or a scheduler.mu is held.
+// goodMu comes first because served, publishServing and writeGood call
+// Act.Current under it, and observeGood takes Manager.mu and then a
+// scheduler.mu under it. So never call served, publishServing or writeGood
+// (or OnGen, which reaches them) while holding activator.Service.mu: that
+// caller would hold Service.mu and wait for goodMu while another holds goodMu
+// and waits for Service.mu.
type Manager struct {
d Deps
ctx context.Context
@@ -408,18 +414,23 @@ func (m *Manager) failedUnder(rt registry.Route, build string) time.Time {
return time.Time{}
}
-// prune drops failure records for builds no managed route installs. A
-// held-down instance's route still names its failed build, so its record
-// stays. 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.
+// 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
+// through "draining", "moving" or "landing" for one Pass, or one the
+// scheduler does not manage, still names its build, and dropping the record
+// 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.
func (m *Manager) prune(routes []registry.Route) {
if len(routes) == 0 {
return
}
installed := map[[3]string]bool{}
for _, rt := range routes {
- if Managed(rt) && rt.Version != "" {
+ if rt.Version != "" {
installed[[3]string{release.RouteAccount(rt), rt.App, rt.Version}] = true
}
}
@@ -522,6 +533,13 @@ func (m *Manager) ConfigDelivered(host, rev string) {
}
m.mu.Unlock()
if !awake {
+ // A sleeper needs no attempt, including one still pending for an
+ // older revision it was awake for: its next wake starts with this
+ // file. Left in place, that attempt would restart a generation
+ // already running rev and then mark the older revision active.
+ s.mu.Lock()
+ s.q.dropConfig(host)
+ s.mu.Unlock()
return
}
s.mu.Lock()
diff --git a/internal/rollout/manager_test.go b/internal/rollout/manager_test.go
index 6d4c4a166..df1b8617f 100644
--- a/internal/rollout/manager_test.go
+++ b/internal/rollout/manager_test.go
@@ -819,6 +819,72 @@ func TestASleepersPendingConfigIsNoAttemptAfterItWakes(t *testing.T) {
}
}
+// Spec §8.1, fix round 1: a config attempt pending for c1 while the instance
+// was awake is no attempt any more once c2 is delivered to it asleep, even
+// if it wakes before the scheduler ever saw it asleep (configSlept never
+// fires). The wake starts with c2. The stale c1 attempt would restart a
+// generation already running c2 and then mark c1 active.
+func TestAConfigDeliveredToASleeperClearsTheOlderPendingAttempt(t *testing.T) {
+ h := newHarness(t, "a")
+ h.act.outcome = func(a activator.Attempt) activator.SwapOutcome {
+ if a.Cause == activator.CauseConfig {
+ return activator.SwapFailed
+ }
+ return activator.SwapDone
+ }
+ h.pass()
+ h.m.ConfigDelivered("a", "sha256:c1")
+ waitFor(t, "the failed c1 attempt", func() bool { return h.events.has("config delivered, not active") })
+ s := h.m.schedulerOf("a")
+ waitFor(t, "the slot to free with c1 pending", func() bool {
+ s.mu.Lock()
+ defer s.mu.Unlock()
+ hs := s.q.hosts["a"]
+ return hs != nil && hs.running == nil && hs.wantConfig == "sha256:c1"
+ })
+ h.act.mu.Lock()
+ delete(h.act.gens, "a") // hibernated inside the backoff; no scheduler iteration runs
+ h.act.mu.Unlock()
+ h.m.ConfigDelivered("a", "sha256:c2")
+ h.act.serve("a", "v1") // woke at once, with c2 in the file
+ h.m.configActivated("a", "sha256:c2")
+ h.advance(2 * time.Minute)
+ h.pass()
+ time.Sleep(50 * time.Millisecond)
+ if calls, _, _ := h.act.snapshot(); len(calls) != 1 {
+ t.Fatalf("calls = %+v: a stale c1 attempt ran against a generation already on c2", calls)
+ }
+ h.m.mu.Lock()
+ tr := h.m.track("a")
+ delivered, active := tr.delivered, tr.active
+ h.m.mu.Unlock()
+ if delivered != "sha256:c2" || active != "sha256:c2" {
+ t.Fatalf("delivered %q, active %q: want both c2", delivered, active)
+ }
+}
+
+// Spec §8.4: a record stays while any instance is on the failed build. A
+// held-down instance whose row is "draining" during a Pass is not managed
+// for that Pass, but it still names the build: the record and the halt stay.
+func TestPruneKeepsTheRecordOfAHeldDownHostWhoseRowLeavesReady(t *testing.T) {
+ h := newHarness(t, "a")
+ h.stage("v2")
+ h.book.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "a", HeldDown: []string{"a"}, PromotedAt: promotedAt, At: t0})
+ r := h.rows.get("a")
+ r.Version, r.Status = "v2", "draining"
+ h.rows.put(r)
+ h.pass()
+ if !h.book.HeldDown("bab", "blog", "v2", "a") {
+ t.Fatal("a Pass dropped the record of a held-down host whose row names the build")
+ }
+ r.Status = "ready"
+ h.rows.put(r)
+ h.pass()
+ if !h.book.HeldDown("bab", "blog", "v2", "a") {
+ t.Fatal("the held-down host lost its record when its row returned to ready")
+ }
+}
+
// Plan 3's deferral (O5): an attempt abandoned while it is still the host's
// newest (a hold whose stop withdrew, a lost re-check) waits before it runs
// again, rather than looping stop cycles. One abandoned by a newer desire
diff --git a/internal/rollout/queue.go b/internal/rollout/queue.go
index d839b1c12..52b0bef59 100644
--- a/internal/rollout/queue.go
+++ b/internal/rollout/queue.go
@@ -183,6 +183,15 @@ func (q *appQueue) configActive(host, rev string) {
}
}
+// dropConfig clears whatever config attempt is pending for host, whichever
+// revision it names. It is for a host with no process: its next wake starts
+// with the delivered file. It never creates a slot.
+func (q *appQueue) dropConfig(host string) {
+ if h := q.hosts[host]; h != nil {
+ h.wantConfig, h.configAfter = "", time.Time{}
+ }
+}
+
// configFailed holds a config-only attempt back until its backoff ends.
func (q *appQueue) configFailed(host string, until time.Time) {
if h := q.hosts[host]; h != nil {