amadan
rollout: a restriction's generation is durable before use and never repeats (Paul's hard limit)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 files changed,
+972
−244
internal/host/rollout.go+6 −6internal/host/rolloutdeclined.go+53 −33internal/host/rolloutdeclined_test.go+79 −0internal/release/hostretry.go+11 −6internal/rollout/failures.go+342 −151internal/rollout/failures_epoch_test.go+379 −0internal/rollout/finalreview1_test.go+22 −11internal/rollout/manager.go+31 −12internal/rollout/retrygrant_test.go+23 −14internal/rollout/sim_test.go+15 −3internal/rollout/status.go+5 −4internal/status/snapshot.go+6 −4
diff --git a/internal/host/rollout.go b/internal/host/rollout.go| index 126b668cf..d57f6024b 100644 |
| --- a/internal/host/rollout.go |
| +++ b/internal/host/rollout.go |
| @@ -341,12 +341,12 @@ func armRollout(ctx context.Context, getenv func(string) string, svc *activator. |
| // nothing it recorded meanwhile would be kept. |
| log.Printf("agent: %s=overlap: %d execution(s) an earlier agent left unrecorded are now failure records", rollout.SwapEnv, n) |
| } |
| - // Every open owes a save before anything runs (final review round 1, |
| - // finding 3): the open moved the book's sequence past every epoch an |
| - // earlier run may have handed out unsaved, and gave a new book its |
| - // identity. Unsaved, a crash would hand those epochs out again, so a |
| - // grant signed for one restriction could clear another; a box that |
| - // cannot save refuses managed starts, as it does for reconciliation. |
| + // The book is saved before anything runs: an open may owe a save (a new |
| + // book's identity, a book an earlier agent wrote given a new one, |
| + // reconciled intents), and until it lands status publishes no |
| + // generation and no retry grant clears anything (the crash-epoch hard |
| + // limit in rollout.FailureBook's doc). A box that cannot save refuses |
| + // managed starts, as it does for reconciliation. |
| if err := book.Flush(); err != nil { |
| log.Printf("agent: %s=overlap: save the failure book after opening it: %v", rollout.SwapEnv, err) |
| refuseManaged(svc, rolloutBookUnreadable) |
diff --git a/internal/host/rolloutdeclined.go b/internal/host/rolloutdeclined.go| index a2c25040c..f58b31df9 100644 |
| --- a/internal/host/rolloutdeclined.go |
| +++ b/internal/host/rolloutdeclined.go |
| @@ -45,8 +45,10 @@ const refuseNoSwaps = "this box cannot run two builds side by side, which this i |
| // `carlos promote --retry` pointer and a signed rollback pointer |
| // (applyPointers) each clear in this one book and save it. The planner, the |
| // launch gate and the legs read it live, so a clear lets the instance start |
| -// at once, with no agent restart. It is also written once at arming when an |
| -// armed run left intents in it (honourDeclinedFailures). |
| +// at once, with no agent restart. It is also written at arming when it owes |
| +// a save (an armed run left intents in it, or an earlier agent wrote it), |
| +// and by the periodic pass while that save keeps failing (pass; the |
| +// crash-epoch hard limit, rule 6). |
| type failureHolds struct { |
| book *rollout.FailureBook |
| // get reads a route by host, for the legs that are handed only a host. |
| @@ -86,13 +88,10 @@ func honourDeclinedFailures(svc *activator.Service, reg *registry.Registry, dd, |
| } |
| if n := book.Reconciled(); n > 0 { |
| // An armed run's executions left unrecorded (Task 5): their records |
| - // are honoured below whether or not this save lands, since the book |
| - // in memory holds them either way, and the next restart reconciles |
| - // the intents again if it did not. |
| + // are honoured below whether or not the save below lands, since the |
| + // book in memory holds them either way, and the next restart |
| + // reconciles the intents again if it did not. |
| log.Printf("agent: %s=overlap declined: %d execution(s) an armed run left unrecorded are now failure records", rollout.SwapEnv, n) |
| - if err := book.Flush(); err != nil { |
| - log.Printf("agent: %s=overlap declined: save the failure book after reconciling it: %v", rollout.SwapEnv, err) |
| - } |
| } |
| // Records or hold-ran markers (Task 9): a marker alone still bars the good |
| // build from an instance whose database a hold build may have migrated. |
| @@ -100,14 +99,16 @@ func honourDeclinedFailures(svc *activator.Service, reg *registry.Registry, dd, |
| if recs == 0 && marks == 0 { |
| return |
| } |
| - // The open moved the book's sequence past every epoch an armed run may |
| - // have handed out unsaved (final review round 1, finding 3). A declined |
| - // box does not save that here, unlike an armed one: it never writes its |
| - // book except to apply a retry or a pointer (bookSnapshot in its |
| - // tests), and the values those take are clears, never a restriction's |
| - // epoch that status publishes; the only restrictions it makes are |
| - // reconciliation's, saved above. Its first such save carries the |
| - // advance. |
| + // A book that owes a save is saved now, whatever reconciliation found |
| + // (the crash-epoch hard limit, rule 6): reconciled records, a new |
| + // identity for a book an earlier agent wrote (rule 4), epochs given to |
| + // older entries. Until it lands, status publishes no generation and no |
| + // retry grant clears anything; the periodic pass (pass) retries it. A |
| + // book this rule wrote and the open did not change owes nothing, and is |
| + // not written. |
| + if err := book.SaveIfDirty(); err != nil { |
| + log.Printf("agent: %s=overlap declined: save the failure book after opening it: %v (retried by the periodic pass)", rollout.SwapEnv, err) |
| + } |
| h := &failureHolds{book: book, get: reg.Get, list: reg.List, optDir: optDir} |
| declinedHolds.Store(h) |
| svc.Planner = h.plan |
| @@ -234,11 +235,12 @@ func (h *failureHolds) plan(_ context.Context, rt registry.Route) activator.Wake |
| // report is the declined box's per-host rollout status (Task 10, plan |
| // review #5): each managed instance the book holds, with the build that |
| -// holds it (HeldBy) and that restriction's epoch (HeldEpoch), which the |
| -// console's retry doors sign into their grant, attempt down when it stays |
| -// down for the hold and failed otherwise. Nil when nothing is held or the |
| -// routes cannot be read. A declined box runs no scheduler, so it reports no |
| -// serving build and no attempt in progress. |
| +// holds it (HeldBy) and that restriction's generation as published |
| +// (HeldEpoch, rollout.FailureBook.PublishedEpoch: 0 while the book owes a |
| +// save), which the console's retry doors sign into their grant, attempt |
| +// down when it stays down for the hold and failed otherwise. Nil when |
| +// nothing is held or the routes cannot be read. A declined box runs no |
| +// scheduler, so it reports no serving build and no attempt in progress. |
| func (h *failureHolds) report() map[string]status.HostRollout { |
| routes, err := h.list() |
| if err != nil { |
| @@ -259,7 +261,7 @@ func (h *failureHolds) report() map[string]status.HostRollout { |
| out = map[string]status.HostRollout{} |
| } |
| out[rt.Host] = status.HostRollout{Account: acct, App: rt.App, Installed: rt.Version, Good: rt.GoodVersion, |
| - Attempt: attempt, AttemptBuild: hd.by, HeldBy: hd.by, HeldEpoch: h.book.RestrictionEpoch(acct, rt.App, hd.by, rt.Host), |
| + Attempt: attempt, AttemptBuild: hd.by, HeldBy: hd.by, HeldEpoch: h.book.PublishedEpoch(acct, rt.App, hd.by, rt.Host), |
| HeldBook: h.book.ID()} // the book the epoch counts in (final review round 1, finding 3) |
| } |
| return out |
| @@ -284,10 +286,13 @@ func (h *failureHolds) retryHost(host, build, book string, epoch uint64) bool { |
| } |
| acct := release.RouteAccount(*rt) |
| rec, mark, now, err := h.book.RetryHostAt(acct, rt.App, build, host, book, epoch) |
| - if err != nil { |
| + if err != nil && (rec || mark) { |
| log.Printf("agent: retry %s on %s: save the failure book: %v (cleared in memory)", build, host, err) |
| - } |
| - if !rec && !mark && now != epoch { |
| + } else if err != nil { |
| + // The book owed a save and it failed again: nothing is cleared until |
| + // one lands (the crash-epoch hard limit, rule 2). |
| + log.Printf("agent: retry %s on %s: the failure book cannot be saved (%v), so nothing was cleared", build, host, err) |
| + } else if !rec && !mark && now != epoch { |
| log.Printf("agent: retry %s on %s: the grant is for epoch %d, the restriction's is %d, so nothing was cleared", build, host, epoch, now) |
| } else if !rec && !mark && book != h.book.ID() { |
| log.Printf("agent: retry %s on %s: the grant is for book %q, not this box's, so nothing was cleared", build, host, book) |
| @@ -309,16 +314,31 @@ func (h *failureHolds) applyPointers(ctx context.Context, read rollout.PointerRe |
| } |
| } |
| -// pointerLoop runs applyPointers over the registry's routes, at most every |
| -// rollout.PointerReadEvery, for the life of the agent. An app with nothing |
| -// the pass could clear costs no read. |
| -func (h *failureHolds) pointerLoop(ctx context.Context, read rollout.PointerRead) { |
| - for { |
| - if routes, err := h.list(); err != nil { |
| - log.Printf("agent: declined pointer pass: list routes: %v", err) |
| +// pass is one turn of the declined box's periodic pass: the save the book |
| +// owes, if any (the crash-epoch hard limit, rule 6), so a box whose disk |
| +// recovers publishes its generations again with no pointer or grant |
| +// arriving, then the pointer pass over the registry's routes. |
| +func (h *failureHolds) pass(ctx context.Context, read rollout.PointerRead) { |
| + if h.book.SaveOwed() { |
| + if err := h.book.SaveIfDirty(); err != nil { |
| + log.Printf("agent: declined pass: save the failure book: %v", err) |
| } else { |
| - h.applyPointers(ctx, read, routes) |
| + nudgeStatus() |
| } |
| + } |
| + if routes, err := h.list(); err != nil { |
| + log.Printf("agent: declined pointer pass: list routes: %v", err) |
| + } else { |
| + h.applyPointers(ctx, read, routes) |
| + } |
| +} |
| + |
| +// pointerLoop runs pass, at most every rollout.PointerReadEvery, for the |
| +// life of the agent. An app with nothing the pointer pass could clear costs |
| +// no read. |
| +func (h *failureHolds) pointerLoop(ctx context.Context, read rollout.PointerRead) { |
| + for { |
| + h.pass(ctx, read) |
| select { |
| case <-ctx.Done(): |
| return |
diff --git a/internal/host/rolloutdeclined_test.go b/internal/host/rolloutdeclined_test.go| index dfe790875..e7b816e5a 100644 |
| --- a/internal/host/rolloutdeclined_test.go |
| +++ b/internal/host/rolloutdeclined_test.go |
| @@ -913,6 +913,85 @@ func TestDeclinedBoxProcessesAVerifiedSignedRetry(t *testing.T) { |
| } |
| } |
| +// writePreRuleBook writes dd's failure book as an agent before the |
| +// crash-epoch hard limit left it (readiness round 2, Task 1): schema 2 with |
| +// a book identity and per-record epochs, no generations and no |
| +// durable_epochs. v2's record holds host down. |
| +func writePreRuleBook(t *testing.T, dd, acct, host string) { |
| + t.Helper() |
| + raw, err := json.Marshal(map[string]any{ |
| + "schema": 2, |
| + "book": "old", |
| + "seq": 10, |
| + "records": []map[string]any{{"account": acct, "app": "blog", "build": "v2", "host": "sib.example.com", |
| + "mode": "hold", "reason": "health", "held_down": []string{host}, "epochs": map[string]uint64{"": 11, host: 11}}}, |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(filepath.Join(dd, rollout.FailureFile), raw, 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +// Readiness round 2, Task 1 (plan 10): a declined box saves a book that |
| +// owes a save at startup, whatever reconciliation found (here a book an |
| +// earlier agent wrote, whose identity the open rotates), and publishes its |
| +// generation at once. When that save fails it publishes the hold with no |
| +// generation, and the next periodic pass, with no pointer or grant |
| +// arriving, saves the book once the disk recovers and publishes it. |
| +func TestDeclinedBoxPublishesAGenerationOnceItsBookIsSaved(t *testing.T) { |
| + for _, diskFails := range []bool{false, true} { |
| + t.Run(map[bool]string{false: "saved at startup", true: "saved by the periodic pass"}[diskFails], func(t *testing.T) { |
| + if diskFails && os.Geteuid() == 0 { |
| + t.Skip("running as root: a read-only directory does not fail a save") |
| + } |
| + t.Setenv("CARLOS_OPT_DIR", t.TempDir()) |
| + svc, reg, dd := rolloutFixture(t, false) |
| + const host = "h.example.com" |
| + rt := heldRouteIn(t, reg, host, "v2", "v1") |
| + acct := release.RouteAccount(rt) |
| + writePreRuleBook(t, dd, acct, host) |
| + if diskFails { |
| + if err := os.Chmod(dd, 0o500); err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Cleanup(func() { os.Chmod(dd, 0o700) }) |
| + } |
| + declineWithBook(t, svc, reg, dd) |
| + hr, ok := declinedHostReport(t, host) |
| + if !ok || hr.HeldBy != "v2" || hr.Attempt != status.AttemptDown || hr.HeldBook == "old" || hr.HeldBook == "" { |
| + t.Fatalf("published %+v (%v), want v2 holding %s down under a new book identity", hr, ok, host) |
| + } |
| + if diskFails { |
| + if hr.HeldEpoch != 0 { |
| + t.Fatalf("published generation %d while the book owes a save, want 0", hr.HeldEpoch) |
| + } |
| + if err := os.Chmod(dd, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + if hr, _ := declinedHostReport(t, host); hr.HeldEpoch != 0 { |
| + t.Fatalf("published generation %d before any save landed, want 0", hr.HeldEpoch) |
| + } |
| + p := &pointerReads{ptrs: map[string]*release.ChannelPointer{}, errs: map[string]error{"blog/stable": errors.New("no pointer")}} |
| + heldHolds().pass(context.Background(), p.read) |
| + hr, _ = declinedHostReport(t, host) |
| + } |
| + if hr.HeldEpoch == 0 { |
| + t.Fatalf("published %+v: want a generation once the book is saved", hr) |
| + } |
| + b, err := rollout.OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if b.ID() != hr.HeldBook || b.PublishedEpoch(acct, "blog", "v2", host) != hr.HeldEpoch { |
| + t.Fatalf("the book on disk is %q at %d, want the published %q at %d", |
| + b.ID(), b.PublishedEpoch(acct, "blog", "v2", host), hr.HeldBook, hr.HeldEpoch) |
| + } |
| + }) |
| + } |
| +} |
| + |
| // pointerReads is a fake signed pointer read keyed "<app>/<channel>", which |
| // counts the reads it is asked for. |
| type pointerReads struct { |
diff --git a/internal/release/hostretry.go b/internal/release/hostretry.go| index 9ac7cc634..2aba99e9b 100644 |
| --- a/internal/release/hostretry.go |
| +++ b/internal/release/hostretry.go |
| @@ -14,9 +14,11 @@ import ( |
| // The restart-scope log it rides is writable by every box (Astra impl-6 |
| // finding 5), so an unsigned entry is a request anyone with a box could |
| // have written, and it clears nothing. The signature closes forgery; Epoch |
| -// closes replay (plan review finding 13): it is the restriction's persisted |
| -// epoch as the box reported it (rollout.FailureBook.RestrictionEpoch), so a |
| -// grant clears the restriction the member saw and nothing that failed |
| +// and Book close replay (plan review finding 13; the crash-epoch hard limit, |
| +// readiness round 2 Task 1): Epoch is the app's restriction generation as |
| +// the box published it (rollout.FailureBook.PublishedEpoch), which the box |
| +// publishes only once it is on disk and never hands out twice, so a grant |
| +// clears the restriction state the member saw and nothing that changed |
| // after, even across an agent restart. |
| type HostRetryGrant struct { |
| V int `json:"v"` |
| @@ -28,9 +30,12 @@ type HostRetryGrant struct { |
| // names that build (plan review finding 5). It is not the build the |
| // host should move to. |
| Build string `json:"build"` |
| - // Epoch is the restriction's epoch the console read from the box's |
| - // report. Zero is an older box that reported none: the console still |
| - // signs it, and no box accepts it. |
| + // Epoch is the app's restriction generation as published, which the |
| + // console read from the box's report (status.HostRollout.HeldEpoch): |
| + // every change to any restriction of the app moves it, so the grant |
| + // clears only while nothing of the app's restrictions changed since. |
| + // Zero is an older box that reported none, or a box whose book owes a |
| + // save: the console still signs it, and no box accepts it. |
| Epoch uint64 `json:"epoch"` |
| // Book is the identity of the failure book Epoch counts in, as the box |
| // reported it (status.HostRollout.HeldBook): epochs are a counter every |
diff --git a/internal/rollout/failures.go b/internal/rollout/failures.go| index 413fb3ce3..35e359853 100644 |
| --- a/internal/rollout/failures.go |
| +++ b/internal/rollout/failures.go |
| @@ -47,14 +47,12 @@ type FailureRecord struct { |
| // Retried are instances `carlos instances retry` cleared: the record no |
| // longer holds them, and their next attempt is a canary. |
| Retried []string `json:"retried,omitempty"` |
| - // Epochs are the book's sequence values at which this record last |
| - // changed (plan review finding 13): under "" the record as a whole |
| - // (created, or merged with a change), under a host that host's standing |
| - // in it (a hold-down added, a retry spent or granted). A signed host |
| - // retry names the epoch it was issued for (RestrictionEpoch), so it |
| - // clears the restriction the member saw and never a later failure of the |
| - // same build. Saved with the record, so a reopen keeps them; a record an |
| - // earlier agent wrote gets one when the book opens. |
| + // Epochs are the per-record epochs an agent before the crash-epoch hard |
| + // limit (readiness round 2, Task 1) wrote: the book's sequence values at |
| + // which the record last changed. They are read only so a reopen's |
| + // sequence starts past them (OpenFailureBook), then dropped, and never |
| + // written again: what a signed host retry names now is the app's |
| + // generation (FailureBook.gens). |
| Epochs map[string]uint64 `json:"epochs,omitempty"` |
| // MarkHoldRan asks Record to set Host's hold-ran marker onto this |
| // record's build in the same save (final review wave 2, N2): an attempt |
| @@ -222,37 +220,124 @@ type failureFile struct { |
| // Intents are the executions in flight by host (Intent), schema 2. |
| Intents map[string]Intent `json:"intents,omitempty"` |
| // Book is the book's identity (FailureBook.id), additive like HoldRan: |
| - // 16 random bytes, hex, made when the book is first created or first |
| - // opened without one, and kept for its life. |
| + // 16 random bytes, hex, made when the book is first created, and again |
| + // when a file from before the crash-epoch rule is adopted (Durable). |
| Book string `json:"book"` |
| + // Gens are the apps' restriction generations (FailureBook.gens), keyed |
| + // account + "/" + app. Additive: an older agent ignores them. |
| + Gens map[string]uint64 `json:"gens,omitempty"` |
| + // Durable says the file was written under the crash-epoch rule (rule 4 |
| + // in FailureBook's doc): every generation it ever published was on disk |
| + // first. A file without it was written by an earlier agent, which may |
| + // have published epochs it never saved, so OpenFailureBook gives such a |
| + // book a new identity. |
| + Durable bool `json:"durable_epochs,omitempty"` |
| } |
| -// reopenAdvance is how far OpenFailureBook moves the sequence past the one |
| -// on disk (final review round 1, finding 3): epochs handed out after the |
| -// last save that landed live only in memory, and status may have published |
| -// them, so a reopen must not hand the same values out again. The caller |
| -// saves the advance before it runs anything (Flush); an armed box whose save |
| -// fails refuses managed starts. A run would need over a million book events |
| -// between two saves that land to reach it. |
| -const reopenAdvance = 1 << 20 |
| - |
| -// newBookID is a book's identity (FailureBook.id). crypto/rand does not fail |
| -// on the platforms the agent runs on; if it ever did, the clock's |
| -// nanoseconds still tell two books apart. |
| -func newBookID() string { |
| +// highestSaved is the largest sequence value the file holds: its own seq, |
| +// every app's generation, every marker's, and every epoch an earlier agent |
| +// wrote into a record. A reopen's sequence starts there (rule 3). In a file |
| +// this rule wrote it is the file's own seq; the max keeps a hand-edited or |
| +// older file from handing out a value one of its entries already holds. |
| +func (f failureFile) highestSaved() uint64 { |
| + n := f.Seq |
| + for _, g := range f.Gens { |
| + n = max(n, g) |
| + } |
| + for _, h := range f.HoldRan { |
| + n = max(n, h.Seq) |
| + } |
| + for _, r := range f.Records { |
| + for _, e := range r.Epochs { |
| + n = max(n, e) |
| + } |
| + } |
| + return n |
| +} |
| + |
| +// randRead is crypto/rand's Read, the one source of a book's identity |
| +// (rule 5). A variable so a test can make it fail. |
| +var randRead = rand.Read |
| + |
| +// newBookID is a book's identity (FailureBook.id): 16 bytes from |
| +// crypto/rand, hex. It never falls back to anything weaker: an identity |
| +// two books could share would let a grant for one clear the other, so a |
| +// read that fails is an error for the caller (OpenFailureBook) to fail on. |
| +// (Since Go 1.24 crypto/rand.Read crashes the process rather than return an |
| +// error, so in production this path is a guard; the seam reaches it.) |
| +func newBookID() (string, error) { |
| var b [16]byte |
| - if _, err := rand.Read(b[:]); err != nil { |
| - return fmt.Sprintf("t%x", time.Now().UnixNano()) |
| + if _, err := randRead(b[:]); err != nil { |
| + return "", fmt.Errorf("make the failure book's identity: %w", err) |
| } |
| - return hex.EncodeToString(b[:]) |
| + return hex.EncodeToString(b[:]), nil |
| } |
| +// genKey is an app's key in FailureBook.gens and failureFile.Gens. |
| +func genKey(acct, app string) string { return acct + "/" + app } |
| + |
| // FailureBook is the box's failure records. An empty path keeps them in |
| // memory only, which is what tests and the simulation use. |
| // |
| // Records handed out (Lookup, ForApp, List) are copies, inner slices |
| // included, because a status reader holds them outside the lock while |
| // RetryHost and Record edit the book. |
| +// |
| +// # The crash-epoch hard limit |
| +// |
| +// A signed `carlos instances retry` grant (release.HostRetryGrant) names a |
| +// book, an app and a generation, which the box's status published, and a |
| +// stale grant must never clear a later failure (Paul's ruling 2026-10-12, |
| +// readiness round 2 Task 1). The book keeps these rules: |
| +// |
| +// 1. One generation per app. gens[acct/app] takes bumpLocked's next value |
| +// (touchLocked) on every change to any restriction of that app: a |
| +// record created, a merge that changes a record, a host retried, a |
| +// record removed (ClearOnPointer, Prune), a marker of the app set, |
| +// renewed or replaced (markLocked; a replaced marker of another app |
| +// moves that app's too) or removed (ClearHoldRan, ClearHoldRanBefore, |
| +// ClearHoldRanOnPointer, RetryHostAt), an intent reconciled into a |
| +// record, and an epoch given at open to an older marker or an app |
| +// without a generation (epochOlderLocked). A call that changes nothing |
| +// (PreRetry, Unchanged, a repeated marker of the same execution) moves |
| +// nothing. The generation of build's restriction of host is the app's |
| +// when build's record blocks host or host's marker names build, else 0 |
| +// (restrictionEpochLocked). Prune drops an app's generation only when |
| +// the app has no record and no marker left; a later restriction of it |
| +// takes a fresh value, larger than every earlier one. |
| +// 2. Durable before use. dirty is true whenever memory differs from the |
| +// last save that landed: every call that changes recs, held, intents, |
| +// gens or seq saves under the same hold of mu (a failed save sets |
| +// dirty), or the open leaves dirty set. PublishedEpoch answers 0 while |
| +// dirty, and RetryHostAt first retries an owed save and clears nothing |
| +// while the book is still dirty. |
| +// 3. Monotonic across restarts. Every landed save writes the in-memory |
| +// seq, which only grows, and a reopen starts from the file's seq, maxed |
| +// with every saved generation, marker and epoch (highestSaved). So the |
| +// disk's seq only grows, every published generation is at most it, and |
| +// every value a later run hands out is larger. |
| +// 4. Adoption rotates the identity. A file without durable_epochs, written |
| +// by an earlier agent that may have published epochs it never saved, |
| +// opens with a new identity and owes a save (dirty), so nothing is |
| +// published until the rotation is on disk, and every grant issued |
| +// before names the old identity. |
| +// 5. The identity comes from crypto/rand only (newBookID). An open that |
| +// needs one and cannot get one fails. Only a memory-only book, which |
| +// tests and the simulation use, falls back to the clock. |
| +// 6. An owed save is retried on its own: the armed manager's pass and a |
| +// declined box's periodic pass call SaveIfDirty, and a declined box |
| +// saves a dirty book at startup. |
| +// |
| +// Proof sketch: a grant G names book X, app A and generation E, which |
| +// status published, so E was on X's disk (2, 4). RetryHostAt clears only |
| +// when X is this book, memory equals disk, and A's generation now equals E |
| +// with build still restricting host. Within a run every change takes a |
| +// fresh larger value (1), so equality means no change of A's restrictions |
| +// since publication. Across a crash the reopened book starts at the disk's |
| +// seq, which is at least E, and hands out larger values (3). A book written |
| +// before the rule has another identity (4), and another book has another |
| +// random identity (5). So G clears only the restriction state it was |
| +// issued for. |
| type FailureBook struct { |
| path string |
| mu sync.Mutex |
| @@ -264,27 +349,34 @@ type FailureBook struct { |
| // records. |
| intents map[string]Intent |
| reconciled int |
| - // dirty is set when a save failed. Memory is the truth meanwhile, and |
| - // every later mutating call and Prune retries the save until it lands. |
| - // What keeps a failure across an agent restart before that is the |
| - // execution's intent (Begin), saved before the execution ran anything and |
| - // removed only after its outcome was written (End): a restart finds it |
| - // on disk and records the failure from it (spec §8.4, Astra R1 #5). |
| + // dirty is set whenever memory differs from the last save that landed |
| + // (rule 2): a save failed, or the open changed something (a rotated |
| + // identity, reconciled intents, epochs given to older entries). Memory is |
| + // the truth meanwhile, and every later mutating call, Prune and |
| + // SaveIfDirty retry the save until it lands; nothing is published or |
| + // retried until then. What keeps a failure across an agent restart before |
| + // that is the execution's intent (Begin), saved before the execution ran |
| + // anything and removed only after its outcome was written (End): a |
| + // restart finds it on disk and records the failure from it (spec §8.4, |
| + // Astra R1 #5). |
| dirty bool |
| // 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, every retry clear and |
| - // every change to a record (its epochs, plan review finding 13) 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 and the |
| - // records' epochs, so snapshots and epochs after a restart are newer than |
| - // every one on disk. The cleared map below is not saved; a host retry's |
| - // clear value is, as the host's epoch in its record, so a spent grant |
| - // stays spent across a restart. |
| + // every app generation (gens) 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 and the generations, so snapshots and |
| + // generations after a restart are newer than every one on disk (rule 3). |
| + // The cleared map below is not saved; a host retry's clear moves the |
| + // app's generation, which is, so a spent grant stays spent across a |
| + // restart. |
| seq uint64 |
| + // gens are the apps' restriction generations by genKey (rule 1), saved |
| + // with the records. A signed host retry names the one it was issued for. |
| + gens map[string]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 |
| @@ -298,8 +390,9 @@ type FailureBook struct { |
| // naming it (final review round 1, finding 3, Astra): epochs are a |
| // counter every book starts alike, so two books (a host relocated, a box |
| // rebuilt) can give one value to two restrictions, and without the |
| - // identity a grant for one would clear the other. A memory-only book |
| - // makes one on first use (idLocked). |
| + // identity a grant for one would clear the other. OpenFailureBook sets |
| + // it (rules 4 and 5); a memory-only book makes one on first use |
| + // (idLocked). |
| id string |
| } |
| @@ -310,13 +403,29 @@ func (b *FailureBook) ID() string { |
| return b.idLocked() |
| } |
| +// idLocked is the book's identity. Only a memory-only book (tests, the |
| +// simulation) reaches it without one, since OpenFailureBook always sets |
| +// it; such a book falls back to the clock if crypto/rand fails (rule 5). |
| func (b *FailureBook) idLocked() string { |
| if b.id == "" { |
| - b.id = newBookID() |
| + id, err := newBookID() |
| + if err != nil { |
| + id = fmt.Sprintf("m%x", time.Now().UnixNano()) |
| + } |
| + b.id = id |
| } |
| return b.id |
| } |
| +// touchLocked moves (acct, app)'s restriction generation to the book's next |
| +// sequence value (rule 1). The caller holds b.mu and saves. |
| +func (b *FailureBook) touchLocked(acct, app string) { |
| + if b.gens == nil { |
| + b.gens = map[string]uint64{} |
| + } |
| + b.gens[genKey(acct, app)] = b.bumpLocked() |
| +} |
| + |
| func (b *FailureBook) clock() time.Time { |
| if b.now != nil { |
| return b.now() |
| @@ -324,12 +433,11 @@ func (b *FailureBook) clock() time.Time { |
| return time.Now() |
| } |
| -// cloneRec copies r's host lists and epochs so the copy shares nothing |
| -// with r. |
| +// cloneRec copies r's host lists so the copy shares nothing with r. A |
| +// record in memory has no Epochs (OpenFailureBook drops them). |
| func cloneRec(r FailureRecord) FailureRecord { |
| r.HeldDown = slices.Clone(r.HeldDown) |
| r.Retried = slices.Clone(r.Retried) |
| - r.Epochs = maps.Clone(r.Epochs) |
| return r |
| } |
| @@ -339,17 +447,25 @@ func cloneRec(r FailureRecord) FailureRecord { |
| // the file become failure records (reconcileLocked, Reconciled), which the |
| // caller saves with Flush. |
| // |
| -// Every open moves the sequence reopenAdvance past the file's (final review |
| -// round 1, finding 3): an epoch handed out after the last save that landed |
| -// was never written, and must not be handed out again for another |
| -// restriction. A book with no identity on disk (a new one, or a file an |
| -// earlier agent wrote) gets one. Both are owed a save (dirty), which the |
| -// caller makes with Flush before it runs anything. |
| +// The sequence starts at the largest value the file holds (rule 3 in |
| +// FailureBook's doc), so nothing handed out from here repeats a generation |
| +// status published: that was on disk. A book with no file gets a new |
| +// identity, and so does a file without durable_epochs (rule 4): an earlier |
| +// agent wrote it and may have published epochs it never saved. Either owes |
| +// a save (dirty), as does an open that reconciled intents or gave older |
| +// entries epochs; nothing is published until it lands. An identity |
| +// crypto/rand cannot make is an error (rule 5): the caller treats the book |
| +// as unreadable. A book this rule wrote opens with no save owed unless the |
| +// open changed something. |
| func OpenFailureBook(dataDir string) (*FailureBook, error) { |
| b := &FailureBook{path: filepath.Join(dataDir, FailureFile)} |
| raw, err := os.ReadFile(b.path) |
| if errors.Is(err, os.ErrNotExist) { |
| - b.id, b.seq, b.dirty = newBookID(), reopenAdvance, true |
| + id, err := newBookID() |
| + if err != nil { |
| + return nil, fmt.Errorf("%s: %w", b.path, err) |
| + } |
| + b.id, b.dirty = id, true |
| return b, nil |
| } |
| if err != nil { |
| @@ -365,25 +481,27 @@ func OpenFailureBook(dataDir string) (*FailureBook, error) { |
| } |
| b.recs = f.Records |
| b.held = f.HoldRan |
| - b.seq = f.Seq |
| b.intents = f.Intents |
| + b.gens = f.Gens |
| b.id = f.Book |
| - if b.id == "" { |
| - b.id = newBookID() |
| - } |
| - 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) |
| + b.seq = f.highestSaved() // the reopen's sequence (rule 3) |
| + for i := range b.recs { |
| + // An earlier agent's per-record epochs: read for the sequence above, |
| + // never written again, and never what a grant is checked against. |
| + if b.recs[i].Epochs != nil { |
| + b.recs[i].Epochs = nil |
| + b.dirty = true |
| + } |
| } |
| - for _, r := range b.recs { |
| - for _, e := range r.Epochs { // the same, for the records' epochs |
| - b.seq = max(b.seq, e) |
| + if !f.Durable || b.id == "" { |
| + // Adoption of the rule (rule 4): grants issued before name the old |
| + // identity, or none. |
| + id, err := newBookID() |
| + if err != nil { |
| + return nil, fmt.Errorf("%s: %w", b.path, err) |
| } |
| + b.id, b.dirty = id, true |
| } |
| - b.seq += reopenAdvance |
| - b.dirty = true |
| b.mu.Lock() |
| b.epochOlderLocked() |
| b.reconcileLocked() |
| @@ -391,23 +509,15 @@ func OpenFailureBook(dataDir string) (*FailureBook, error) { |
| return b, nil |
| } |
| -// epochOlderLocked gives every record and marker an earlier agent wrote |
| -// without an epoch one now (plan review finding 13): a zero epoch is never |
| -// accepted by a signed retry, so without one its hold could not be retried |
| -// from the console. Records first, in file order, then markers by host, so |
| -// two opens of the same file hand out the same values. The book then owes a |
| -// save (dirty), which the next mutating call or Prune makes; nothing is |
| -// written here. The caller holds b.mu. |
| +// epochOlderLocked gives what an earlier agent wrote without a sequence |
| +// value one now: each marker with no Seq (by host, sorted), then each app |
| +// that has a record or a marker and no generation (sorted by key), so two |
| +// opens of the same file hand out the same values. A zero generation is |
| +// never accepted by a signed retry, so without one such a hold could not be |
| +// retried from the console. A marker given a Seq moves its app's generation |
| +// (rule 1). The book then owes a save (dirty), which the caller or the next |
| +// mutating call makes; nothing is written here. The caller holds b.mu. |
| func (b *FailureBook) epochOlderLocked() { |
| - for i := range b.recs { |
| - if b.recs[i].Epochs[""] == 0 { |
| - if b.recs[i].Epochs == nil { |
| - b.recs[i].Epochs = map[string]uint64{} |
| - } |
| - b.recs[i].Epochs[""] = b.bumpLocked() |
| - b.dirty = true |
| - } |
| - } |
| hosts := make([]string, 0, len(b.held)) |
| for host, h := range b.held { |
| if h.Seq == 0 { |
| @@ -415,10 +525,25 @@ func (b *FailureBook) epochOlderLocked() { |
| } |
| } |
| slices.Sort(hosts) |
| + apps := map[string][2]string{} |
| for _, host := range hosts { |
| h := b.held[host] |
| h.Seq = b.bumpLocked() |
| b.held[host] = h |
| + apps[genKey(h.Account, h.App)] = [2]string{h.Account, h.App} |
| + } |
| + for _, r := range b.recs { |
| + if b.gens[genKey(r.Account, r.App)] == 0 { |
| + apps[genKey(r.Account, r.App)] = [2]string{r.Account, r.App} |
| + } |
| + } |
| + for _, h := range b.held { |
| + if b.gens[genKey(h.Account, h.App)] == 0 { |
| + apps[genKey(h.Account, h.App)] = [2]string{h.Account, h.App} |
| + } |
| + } |
| + for _, k := range slices.Sorted(maps.Keys(apps)) { |
| + b.touchLocked(apps[k][0], apps[k][1]) |
| b.dirty = true |
| } |
| } |
| @@ -479,6 +604,25 @@ func (b *FailureBook) Flush() error { |
| return b.saveLocked() |
| } |
| +// SaveIfDirty saves the book when a save is owed (dirty: a save failed, or |
| +// the open changed something), else does nothing and returns nil (rule 6). |
| +// The armed manager's pass and a declined box's periodic pass call it, so a |
| +// book whose disk recovers publishes its generations again without waiting |
| +// for another change, pointer or grant. |
| +func (b *FailureBook) SaveIfDirty() error { |
| + b.mu.Lock() |
| + defer b.mu.Unlock() |
| + return b.retryLocked() |
| +} |
| + |
| +// SaveOwed reports whether the book owes a save (dirty): while it does, |
| +// PublishedEpoch answers 0 and RetryHostAt clears nothing. |
| +func (b *FailureBook) SaveOwed() bool { |
| + b.mu.Lock() |
| + defer b.mu.Unlock() |
| + return b.dirty |
| +} |
| + |
| // Begin writes host's intent and saves it before the execution runs |
| // anything (rule 1). On a save error nothing of it stays in memory, the host's |
| // earlier intent (if this one replaced it) is put back, and the error is |
| @@ -539,12 +683,16 @@ func (b *FailureBook) End(host, id string) error { |
| } |
| // saveLocked writes the book and records whether the file is now behind |
| -// memory, so a failure is retried by retryLocked rather than forgotten. |
| +// memory, so a failure is retried by retryLocked rather than forgotten. It |
| +// writes the in-memory seq, which only grows (rule 3), the generations, and |
| +// durable_epochs, since everything this agent publishes is on disk first |
| +// (rule 2). |
| func (b *FailureBook) saveLocked() error { |
| if b.path == "" { |
| return nil |
| } |
| - raw, err := json.MarshalIndent(failureFile{Schema: failureSchema, Records: b.recs, HoldRan: b.held, Seq: b.seq, Intents: b.intents, Book: b.idLocked()}, "", " ") |
| + raw, err := json.MarshalIndent(failureFile{Schema: failureSchema, Records: b.recs, HoldRan: b.held, Seq: b.seq, Intents: b.intents, |
| + Book: b.idLocked(), Gens: b.gens, Durable: true}, "", " ") |
| if err == nil { |
| err = release.WriteFileAtomic(b.path, raw, 0o644) |
| } |
| @@ -657,47 +805,32 @@ func (b *FailureBook) recordLocked(r FailureRecord) (RecordResult, bool) { |
| } |
| i := b.find(r.Account, r.App, r.Build) |
| if i < 0 { |
| - r.Retried = nil |
| - // A new record is a new restriction for every host it holds: the |
| - // record-wide epoch and the failing host's (plan review finding 13). |
| - e := b.bumpLocked() |
| - r.Epochs = map[string]uint64{"": e, r.Host: e} |
| - for _, h := range r.HeldDown { |
| - r.Epochs[h] = e |
| - } |
| + r.Retried, r.Epochs = nil, nil |
| + // A new record is a new restriction of every host of the app (rule 1). |
| b.recs = append(b.recs, cloneRec(r)) |
| + b.touchLocked(r.Account, r.App) |
| return Recorded, true |
| } |
| cur := &b.recs[i] |
| - var moved []string // hosts whose standing in the record changed |
| - recChanged := false |
| + changed := false |
| if j := slices.Index(cur.Retried, r.Host); j >= 0 { |
| cur.Retried = slices.Delete(slices.Clone(cur.Retried), j, j+1) |
| - moved = append(moved, r.Host) |
| + changed = true |
| } |
| if r.PromotedAt.After(cur.PromotedAt) { |
| cur.PromotedAt = r.PromotedAt |
| - recChanged = true |
| + changed = true |
| } |
| for _, h := range r.HeldDown { |
| if !slices.Contains(cur.HeldDown, h) { |
| cur.HeldDown = append(slices.Clone(cur.HeldDown), h) |
| - moved = append(moved, h) |
| + changed = true |
| } |
| } |
| - if recChanged || len(moved) > 0 { |
| - // A merge with a change moves the record-wide epoch and the epochs |
| - // of the hosts it moved, so a grant signed before it clears nothing |
| - // (plan review finding 13). |
| - cur.Epochs = maps.Clone(cur.Epochs) |
| - if cur.Epochs == nil { |
| - cur.Epochs = map[string]uint64{} |
| - } |
| - e := b.bumpLocked() |
| - cur.Epochs[""] = e |
| - for _, h := range moved { |
| - cur.Epochs[h] = e |
| - } |
| + if changed { |
| + // A merge with a change moves the app's generation, so a grant |
| + // signed before it clears nothing (plan review finding 13, rule 1). |
| + b.touchLocked(r.Account, r.App) |
| return Merged, true |
| } |
| return Unchanged, marked |
| @@ -716,7 +849,11 @@ func (b *FailureBook) recordLocked(r FailureRecord) (RecordResult, bool) { |
| // 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. |
| +// runs never looks newer than the marker. |
| +// |
| +// Every marker set, renewed or replaced moves its app's generation (rule |
| +// 1); a marker of another app it replaces (a host now of another app) moves |
| +// that app's too. 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 { |
| @@ -732,6 +869,10 @@ func (b *FailureBook) markLocked(host string, h HoldRan) bool { |
| h.At = cur.At |
| } |
| h.Seq = b.bumpLocked() // rolloutsim:M12 |
| + if ok && (cur.Account != h.Account || cur.App != h.App) { |
| + b.touchLocked(cur.Account, cur.App) // the replaced marker's app lost a restriction |
| + } |
| + b.touchLocked(h.Account, h.App) // the marker set, renewed or replaced: its app's generation moves |
| b.held[host] = h |
| return true |
| } |
| @@ -774,10 +915,12 @@ func (b *FailureBook) HoldRanCount() int { |
| func (b *FailureBook) ClearHoldRan(host string) (bool, error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| - if _, ok := b.held[host]; !ok { |
| + cur, ok := b.held[host] |
| + if !ok { |
| return false, b.retryLocked() |
| } |
| delete(b.held, host) |
| + b.touchLocked(cur.Account, cur.App) |
| return true, b.saveLocked() |
| } |
| @@ -796,6 +939,7 @@ func (b *FailureBook) ClearHoldRanBefore(host string, snap uint64) (bool, error) |
| return false, b.retryLocked() |
| } |
| delete(b.held, host) |
| + b.touchLocked(cur.Account, cur.App) |
| return true, b.saveLocked() |
| } |
| @@ -817,6 +961,7 @@ func (b *FailureBook) ClearHoldRanOnPointer(host, acct, app string, promotedAt t |
| return false, b.retryLocked() |
| } |
| delete(b.held, host) |
| + b.touchLocked(acct, app) |
| return true, b.saveLocked() |
| } |
| @@ -885,10 +1030,9 @@ func (b *FailureBook) RetryHost(acct, app, build, host string) (bool, error) { |
| } |
| // retryHostLocked is RetryHost's change to memory, and reports whether it |
| -// un-retried the host. The clear's sequence value is also the host's epoch |
| -// in the record (plan review finding 13), saved with it: a grant that was |
| -// spent here never matches again, even after a reopen. The caller holds |
| -// b.mu and saves. |
| +// un-retried the host. It moves the app's generation (rule 1), saved with |
| +// the book: a grant that was spent here never matches again, even after a |
| +// reopen. The caller holds b.mu and saves. |
| func (b *FailureBook) retryHostLocked(acct, app, build, host string) bool { |
| i := b.find(acct, app, build) |
| if i < 0 || slices.Contains(b.recs[i].Retried, host) { |
| @@ -900,55 +1044,72 @@ func (b *FailureBook) retryHostLocked(acct, app, build, host string) bool { |
| if j := slices.Index(cur.HeldDown, host); j >= 0 { |
| cur.HeldDown = slices.Delete(slices.Clone(cur.HeldDown), j, j+1) |
| } |
| - cur.Epochs = maps.Clone(cur.Epochs) |
| - if cur.Epochs == nil { |
| - cur.Epochs = map[string]uint64{} |
| - } |
| - cur.Epochs[host] = b.cleared[clearKey(acct, app, build, host)] |
| + b.touchLocked(acct, app) |
| return true |
| } |
| -// RestrictionEpoch is the epoch of build's restriction of host (plan review |
| -// finding 13): the larger of the build's record's record-wide and host |
| -// epochs (FailureRecord.Epochs), and host's hold-ran marker's sequence when |
| -// the marker names build. Zero when nothing of build restricts host. Status |
| -// publishes it beside HeldBy (HostRollout.HeldEpoch), the console signs it |
| -// into its retry grant, and RetryHostAt clears only while it is unchanged. |
| +// RestrictionEpoch is the generation of build's restriction of host as it |
| +// stands in memory (restrictionEpochLocked), saved or not. It is for logs |
| +// and tests: status publishes PublishedEpoch, never this. |
| func (b *FailureBook) RestrictionEpoch(acct, app, build, host string) uint64 { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| return b.restrictionEpochLocked(acct, app, build, host) |
| } |
| -func (b *FailureBook) restrictionEpochLocked(acct, app, build, host string) uint64 { |
| - var e uint64 |
| - if i := b.find(acct, app, build); i >= 0 { |
| - ep := b.recs[i].Epochs |
| - e = max(ep[""], ep[host]) |
| +// PublishedEpoch is the generation of build's restriction of host that |
| +// status publishes beside HeldBy (HostRollout.HeldEpoch) and the console |
| +// signs into its retry grant: the app's generation when build's record |
| +// blocks host or host's marker names build, and the book's memory equals |
| +// its last landed save (rule 2). Zero otherwise: nothing of build restricts |
| +// host, or the book owes a save, so the value might not survive a crash. |
| +func (b *FailureBook) PublishedEpoch(acct, app, build, host string) uint64 { |
| + b.mu.Lock() |
| + defer b.mu.Unlock() |
| + if b.dirty { // durable before use (rule 2) |
| + return 0 |
| } |
| - if m, ok := b.held[host]; ok && m.Account == acct && m.App == app && m.Build == build { |
| - e = max(e, m.Seq) |
| + return b.restrictionEpochLocked(acct, app, build, host) |
| +} |
| + |
| +// restrictionEpochLocked is (acct, app)'s generation when build restricts |
| +// host, through its record (which blocks every host of the app it does not |
| +// list as retried) or host's hold-ran marker naming build; else 0 (rule 1). |
| +// The caller holds b.mu. |
| +func (b *FailureBook) restrictionEpochLocked(acct, app, build, host string) uint64 { |
| + i := b.find(acct, app, build) |
| + byRecord := i >= 0 && !slices.Contains(b.recs[i].Retried, host) |
| + m, ok := b.held[host] |
| + byMarker := ok && m.Account == acct && m.App == app && m.Build == build |
| + if !byRecord && !byMarker { |
| + return 0 |
| } |
| - return e |
| + return b.gens[genKey(acct, app)] |
| } |
| // RetryHostAt is a signed `carlos instances retry <host>` grant (spec §8.4, |
| // §11; HostRetryGrant) applied to the book: when the grant names this book |
| -// (book, the book's ID) and epoch is build's restriction of host now |
| -// (RestrictionEpoch), it clears build's record for |
| -// host, when that record holds host (RetryHost), and host's hold-ran |
| -// marker, when the marker names build (plan review finding 5). A grant from |
| -// before the restriction last changed, a zero epoch, or one for a build that |
| -// restricts nothing clears nothing. Nor does a grant for another book, or |
| -// one that names no book (a console or a box from before the identity): |
| -// its epoch could be another book's (final review round 1, finding 3). The check and the clears are one hold of |
| -// the lock. now is the epoch at the check, for the caller's log. |
| +// (book, the book's ID), the book owes no save, and epoch is build's |
| +// restriction of host now (the app's generation, PublishedEpoch), it clears |
| +// build's record for host, when that record holds host (RetryHost), and |
| +// host's hold-ran marker, when the marker names build (plan review finding |
| +// 5). A grant from before any restriction of the app last changed, a zero |
| +// epoch, or one for a build that restricts nothing clears nothing. Nor does |
| +// a grant for another book, or one that names no book (a console or a box |
| +// from before the identity): its epoch could be another book's (final |
| +// review round 1, finding 3). An owed save is retried first, and while it |
| +// fails nothing is cleared (rule 2): the generation in memory may be one no |
| +// disk holds. The check and the clears are one hold of the lock. now is the |
| +// generation at the check, for the caller's log; err is the save's. |
| func (b *FailureBook) RetryHostAt(acct, app, build, host, book string, epoch uint64) (rec, mark bool, now uint64, err error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| now = b.restrictionEpochLocked(acct, app, build, host) |
| + if err := b.retryLocked(); b.dirty { |
| + return false, false, now, err |
| + } |
| if book == "" || book != b.idLocked() || epoch == 0 || now != epoch { |
| - return false, false, now, b.retryLocked() |
| + return false, false, now, nil |
| } |
| if m, ok := b.held[host]; ok && m.Account == acct && m.App == app && m.Build == build { |
| delete(b.held, host) |
| @@ -956,7 +1117,7 @@ func (b *FailureBook) RetryHostAt(acct, app, build, host, book string, epoch uin |
| } |
| rec = b.retryHostLocked(acct, app, build, host) |
| if !rec && !mark { |
| - return false, false, now, b.retryLocked() |
| + return false, false, now, nil |
| } |
| if !rec { |
| // Only the marker was removed (no record held the host): still a |
| @@ -964,9 +1125,10 @@ func (b *FailureBook) RetryHostAt(acct, app, build, host, book string, epoch uin |
| // round 1, finding 4, Astra). A failure of an execution of build on |
| // host that began before it is then stale (Record, PreRetry): no |
| // record, no halt, though a late hold outcome still sets its marker. |
| - // retryHostLocked has noted it when it cleared a record, and a retry |
| - // that removed nothing notes nothing. |
| + // retryHostLocked has noted it, and moved the generation, when it |
| + // cleared a record, and a retry that removed nothing notes nothing. |
| b.clearedLocked(acct, app, build, host) |
| + b.touchLocked(acct, app) |
| } |
| return rec, mark, now, b.saveLocked() |
| } |
| @@ -987,6 +1149,7 @@ func (b *FailureBook) ClearOnPointer(acct, app, build string, promotedAt time.Ti |
| } |
| b.recs = slices.Delete(b.recs, i, i+1) |
| b.clearedLocked(acct, app, build, "") |
| + b.touchLocked(acct, app) |
| return true, b.saveLocked() |
| } |
| @@ -1011,6 +1174,11 @@ func (b *FailureBook) ForApp(acct, app string) []FailureRecord { |
| // 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. |
| +// |
| +// An app that lost a record and still has a record or a marker gets a new |
| +// generation; an app with neither left loses its generation, which is the |
| +// only way one is dropped (rule 1): nothing of it restricts any host, and a |
| +// later restriction takes a fresh value, larger than every earlier one. |
| func (b *FailureBook) Prune(installed func(acct, app, build string) bool) error { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| @@ -1019,16 +1187,39 @@ func (b *FailureBook) Prune(installed func(acct, app, build string) bool) error |
| delete(b.cleared, k) |
| } |
| } |
| + lost := map[string][2]string{} // apps that lost a record |
| kept := b.recs[:0] |
| for _, r := range b.recs { |
| if installed(r.Account, r.App, r.Build) { |
| kept = append(kept, r) |
| + } else { |
| + lost[genKey(r.Account, r.App)] = [2]string{r.Account, r.App} |
| + } |
| + } |
| + b.recs = kept |
| + standing := map[string]bool{} // apps with a record or a marker left |
| + for _, r := range b.recs { |
| + standing[genKey(r.Account, r.App)] = true |
| + } |
| + for _, h := range b.held { |
| + standing[genKey(h.Account, h.App)] = true |
| + } |
| + changed := false |
| + for k := range b.gens { |
| + if !standing[k] { |
| + delete(b.gens, k) |
| + changed = true |
| + } |
| + } |
| + for _, k := range slices.Sorted(maps.Keys(lost)) { |
| + if standing[k] { |
| + b.touchLocked(lost[k][0], lost[k][1]) |
| } |
| + changed = true |
| } |
| - if len(kept) == len(b.recs) { |
| + if !changed { |
| return b.retryLocked() |
| } |
| - b.recs = kept |
| return b.saveLocked() |
| } |
diff --git a/internal/rollout/failures_epoch_test.go b/internal/rollout/failures_epoch_test.go| new file mode 100644 |
| index 000000000..25e24508d |
| --- /dev/null |
| +++ b/internal/rollout/failures_epoch_test.go |
| @@ -0,0 +1,379 @@ |
| +package rollout |
| + |
| +import ( |
| + "encoding/json" |
| + "errors" |
| + "os" |
| + "path/filepath" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/activator" |
| +) |
| + |
| +// The crash-epoch hard limit (zero-downtime flag readiness round 2, Task 1; |
| +// Paul's ruling 2026-10-12): a stale signed retry grant never clears a later |
| +// failure. A restriction's generation is per (account, app), moves on every |
| +// change of the app's restrictions, is published only once it is on disk, |
| +// never repeats across a restart, and counts in a book identity that comes |
| +// from crypto/rand and is rotated when the rule is adopted. |
| + |
| +// diskBook is a book saved under a data dir of its own, its owed save |
| +// landed, and the dir. |
| +func diskBook(t *testing.T) (*FailureBook, string) { |
| + t.Helper() |
| + dd := t.TempDir() |
| + b, err := OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := b.Flush(); err != nil { |
| + t.Fatal(err) |
| + } |
| + return b, dd |
| +} |
| + |
| +// published is the generation status publishes for build's restriction of |
| +// host in bab/blog. |
| +func published(b *FailureBook, build, host string) uint64 { |
| + return b.PublishedEpoch("bab", "blog", build, host) |
| +} |
| + |
| +// grantAt applies a signed host retry of build on host naming b's identity |
| +// at epoch. |
| +func grantAt(b *FailureBook, build, host string, epoch uint64) (rec, mark bool) { |
| + rec, mark, _, _ = b.RetryHostAt("bab", "blog", build, host, b.ID(), epoch) |
| + return rec, mark |
| +} |
| + |
| +func mustRecord(t *testing.T, b *FailureBook, r FailureRecord) RecordResult { |
| + t.Helper() |
| + res, err := b.Record(r) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return res |
| +} |
| + |
| +func mustMark(t *testing.T, b *FailureBook, host string, h HoldRan) { |
| + t.Helper() |
| + if err := b.MarkHoldRan(host, h); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +// Plan 8: a book whose save fails publishes nothing, and a grant for the |
| +// generation published before the change clears nothing. Once the owed save |
| +// lands the new generation is published and a grant at it clears. |
| +func TestAFailingDiskPublishesNoEpoch(t *testing.T) { |
| + b, dd := diskBook(t) |
| + mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h1", Mode: string(activator.ModeHold), HeldDown: []string{"h1"}}) |
| + e1 := published(b, "v2", "h1") |
| + if e1 == 0 { |
| + t.Fatal("a saved restriction published no generation") |
| + } |
| + readOnly(t, dd) |
| + if _, err := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v3", Host: "h2"}); err == nil { |
| + t.Fatal("the save of v3's record did not fail: the case needs a failing disk") |
| + } |
| + if e := published(b, "v2", "h1"); e != 0 { |
| + t.Fatalf("published %d while the book's save is owed, want 0", e) |
| + } |
| + if rec, mark, _, err := b.RetryHostAt("bab", "blog", "v2", "h1", b.ID(), e1); rec || mark { |
| + t.Fatalf("a grant from before the unsaved change cleared record %v, marker %v (%v)", rec, mark, err) |
| + } |
| + if !b.HeldDown("bab", "blog", "v2", "h1") { |
| + t.Fatal("h1 is no longer held down") |
| + } |
| + writable(t, dd) |
| + if err := b.SaveIfDirty(); err != nil { |
| + t.Fatalf("the owed save did not land: %v", err) |
| + } |
| + e2 := published(b, "v2", "h1") |
| + if e2 <= e1 { |
| + t.Fatalf("published %d after the save landed, want past %d", e2, e1) |
| + } |
| + if rec, _ := grantAt(b, "v2", "h1", e2); !rec { |
| + t.Fatal("a grant at the published generation cleared nothing") |
| + } |
| + again, err := OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if again.HeldDown("bab", "blog", "v2", "h1") { |
| + t.Fatal("the retry was not saved") |
| + } |
| +} |
| + |
| +// Plan 1: every removal of a restriction of the app moves the app's |
| +// generation, so a grant signed before it clears nothing. |
| +func TestEveryRemovalMovesTheGeneration(t *testing.T) { |
| + const host = "h1" |
| + // v2's record (from a sibling's failure) holds h1, and v2's marker |
| + // stands on h1. |
| + setup := func(t *testing.T) *FailureBook { |
| + b, _ := diskBook(t) |
| + mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib", PromotedAt: promotedAt, At: t0}) |
| + mustMark(t, b, host, HoldRan{Account: "bab", App: "blog", Build: "v2", Exec: "a1", At: t0}) |
| + return b |
| + } |
| + for name, change := range map[string]func(t *testing.T, b *FailureBook){ |
| + "the marker replaced by another build's": func(t *testing.T, b *FailureBook) { |
| + mustMark(t, b, host, HoldRan{Account: "bab", App: "blog", Build: "v3", Exec: "a2", At: t0}) |
| + }, |
| + "the record cleared by a pointer": func(t *testing.T, b *FailureBook) { |
| + if ok, err := b.ClearOnPointer("bab", "blog", "v2", promotedAt.Add(time.Minute)); !ok || err != nil { |
| + t.Fatalf("ClearOnPointer = %v, %v", ok, err) |
| + } |
| + }, |
| + "the record pruned": func(t *testing.T, b *FailureBook) { |
| + if err := b.Prune(func(string, string, string) bool { return false }); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, ok := b.Lookup("bab", "blog", "v2"); ok { |
| + t.Fatal("the record was not pruned") |
| + } |
| + }, |
| + "the marker cleared by a good write": func(t *testing.T, b *FailureBook) { |
| + if ok, err := b.ClearHoldRanBefore(host, b.Snapshot()); !ok || err != nil { |
| + t.Fatalf("ClearHoldRanBefore = %v, %v", ok, err) |
| + } |
| + }, |
| + "the marker cleared by a rollback": func(t *testing.T, b *FailureBook) { |
| + if ok, err := b.ClearHoldRanOnPointer(host, "bab", "blog", t0.Add(time.Minute)); !ok || err != nil { |
| + t.Fatalf("ClearHoldRanOnPointer = %v, %v", ok, err) |
| + } |
| + }, |
| + "the marker cleared": func(t *testing.T, b *FailureBook) { |
| + if ok, err := b.ClearHoldRan(host); !ok || err != nil { |
| + t.Fatalf("ClearHoldRan = %v, %v", ok, err) |
| + } |
| + }, |
| + "the record merged with a newer pointer": func(t *testing.T, b *FailureBook) { |
| + if res := mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib2", PromotedAt: promotedAt.Add(time.Minute)}); res != Merged { |
| + t.Fatalf("Record = %v, want merged", res) |
| + } |
| + }, |
| + } { |
| + t.Run(name, func(t *testing.T) { |
| + b := setup(t) |
| + e := published(b, "v2", host) |
| + if e == 0 { |
| + t.Fatal("no generation published before the change") |
| + } |
| + change(t, b) |
| + now := published(b, "v2", host) |
| + if now == e { |
| + t.Fatalf("the generation stayed at %d across the change", e) |
| + } |
| + if now == 0 { |
| + t.Fatal("v2 still restricts h1 through its record or marker, but no generation is published") |
| + } |
| + if rec, mark := grantAt(b, "v2", host, e); rec || mark { |
| + t.Fatalf("a grant from before the change cleared record %v, marker %v", rec, mark) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// A call that changes nothing (a stale failure, an unchanged one, a repeated |
| +// marker of the same execution) moves nothing: a grant at the generation |
| +// published before it still clears. |
| +func TestANoOpOutcomeDoesNotMoveTheGeneration(t *testing.T) { |
| + b, _ := diskBook(t) |
| + mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib", Mode: string(activator.ModeHold), |
| + HeldDown: []string{"sib"}, MarkHoldRan: true, HoldRanExec: "a1", HoldRanAt: t0}) |
| + snap := b.Snapshot() // an execution of v2 on h1 starts |
| + if rec, _ := grantAt(b, "v2", "h1", published(b, "v2", "h1")); !rec { |
| + t.Fatal("the retry of h1 cleared nothing") |
| + } |
| + e := published(b, "v2", "h2") |
| + if e == 0 { |
| + t.Fatal("no generation published for h2") |
| + } |
| + steady := func(what string) { |
| + t.Helper() |
| + if now := published(b, "v2", "h2"); now != e { |
| + t.Fatalf("%s moved the generation from %d to %d", what, e, now) |
| + } |
| + } |
| + if res := mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h1", Seq: snap}); res != PreRetry { |
| + t.Fatalf("the failure that began before the retry = %v, want pre-retry", res) |
| + } |
| + steady("a pre-retry failure") |
| + if res := mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib"}); res != Unchanged { |
| + t.Fatalf("a repeated failure = %v, want unchanged", res) |
| + } |
| + steady("an unchanged failure") |
| + mustMark(t, b, "sib", HoldRan{Account: "bab", App: "blog", Build: "v2", Exec: "a1", At: t0}) |
| + steady("a repeated marker of the same execution") |
| + if res := mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib", Mode: string(activator.ModeHold), |
| + HeldDown: []string{"sib"}, MarkHoldRan: true, HoldRanExec: "a1", HoldRanAt: t0}); res != Unchanged { |
| + t.Fatalf("a repeated hold failure = %v, want unchanged", res) |
| + } |
| + steady("a repeated hold failure of the same execution") |
| + if rec, _ := grantAt(b, "v2", "h2", e); !rec { |
| + t.Fatal("a grant at the unchanged generation cleared nothing") |
| + } |
| +} |
| + |
| +// Plan 2: a file an earlier binary wrote (no durable_epochs) may hold |
| +// epochs that binary published and never saved. Opening it under the rule |
| +// rotates the book's identity, so no grant issued before clears anything, |
| +// and nothing is published until the rotation is on disk. |
| +func TestABookFromBeforeTheRuleGetsANewIdentity(t *testing.T) { |
| + dd := t.TempDir() |
| + raw, err := json.Marshal(map[string]any{ |
| + "schema": 2, |
| + "book": "old", |
| + "seq": 10, |
| + "records": []map[string]any{{"account": "bab", "app": "blog", "build": "v2", "host": "h1", |
| + "held_down": []string{"h1"}, "epochs": map[string]uint64{"": 11, "h1": 11}}}, |
| + }) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(filepath.Join(dd, FailureFile), raw, 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + b, err := OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + id := b.ID() |
| + if id == "old" || id == "" { |
| + t.Fatalf("identity %q after adopting the rule, want a new one", id) |
| + } |
| + if e := published(b, "v2", "h1"); e != 0 { |
| + t.Fatalf("published %d before the rotation was saved, want 0", e) |
| + } |
| + for _, epoch := range []uint64{11, 12} { |
| + if rec, mark, _, _ := b.RetryHostAt("bab", "blog", "v2", "h1", "old", epoch); rec || mark { |
| + t.Fatalf("a grant naming the old identity at %d cleared record %v, marker %v", epoch, rec, mark) |
| + } |
| + } |
| + if err := b.Flush(); err != nil { |
| + t.Fatal(err) |
| + } |
| + e := published(b, "v2", "h1") |
| + if e <= 11 { |
| + t.Fatalf("published %d after the save, want a generation past the file's 11", e) |
| + } |
| + again, err := OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if again.ID() != id || published(again, "v2", "h1") != e { |
| + t.Fatalf("reopened as %q at %d, want %q at %d", again.ID(), published(again, "v2", "h1"), id, e) |
| + } |
| + if f := fileOf(t, dd); !f.Durable || f.Book != id { |
| + t.Fatalf("file durable_epochs %v, book %q: want true and %q", f.Durable, f.Book, id) |
| + } |
| + if rec, _ := grantAt(again, "v2", "h1", e); !rec { |
| + t.Fatal("a grant naming the new identity at the published generation cleared nothing") |
| + } |
| +} |
| + |
| +// Rule 3: a reopen starts from the disk's sequence, so every generation |
| +// handed out after it is larger than every one published before it. |
| +func TestAReopenNeverReissuesAPublishedGeneration(t *testing.T) { |
| + b, dd := diskBook(t) |
| + var before uint64 |
| + read := func(b *FailureBook) uint64 { |
| + t.Helper() |
| + e := max(published(b, "v2", "h1"), published(b, "v3", "h1"), published(b, "v4", "h1"), published(b, "v5", "h2")) |
| + if e == 0 { |
| + t.Fatal("nothing published") |
| + } |
| + return e |
| + } |
| + mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib", PromotedAt: promotedAt}) |
| + before = max(before, read(b)) |
| + mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "sib", HeldDown: []string{"h1"}}) |
| + before = max(before, read(b)) |
| + mustMark(t, b, "h1", HoldRan{Account: "bab", App: "blog", Build: "v3", Exec: "a1", At: t0}) |
| + before = max(before, read(b)) |
| + mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v4", Host: "sib"}) |
| + before = max(before, read(b)) |
| + |
| + again, err := OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := again.Flush(); err != nil { |
| + t.Fatal(err) |
| + } |
| + for i, change := range []func(){ |
| + func() { mustRecord(t, again, FailureRecord{Account: "bab", App: "blog", Build: "v5", Host: "sib"}) }, |
| + func() { |
| + mustMark(t, again, "h2", HoldRan{Account: "bab", App: "blog", Build: "v5", Exec: "a2", At: t0}) |
| + }, |
| + func() { |
| + if ok, err := again.ClearOnPointer("bab", "blog", "v4", t0); !ok || err != nil { |
| + t.Fatalf("ClearOnPointer = %v, %v", ok, err) |
| + } |
| + }, |
| + } { |
| + change() |
| + for _, c := range [][2]string{{"v2", "h1"}, {"v3", "h1"}, {"v5", "h2"}} { |
| + if e := published(again, c[0], c[1]); e <= before { |
| + t.Fatalf("after change %d of the reopened book, %s on %s published %d, not past %d", i+1, c[0], c[1], e, before) |
| + } |
| + } |
| + } |
| +} |
| + |
| +// Rule 5 and finding 3: two books opened with equal numeric state publish |
| +// the same generation; a grant for one clears nothing in the other. |
| +func TestAGrantForAnotherBookClearsNothing(t *testing.T) { |
| + one, _ := diskBook(t) |
| + other, _ := diskBook(t) |
| + for _, b := range []*FailureBook{one, other} { |
| + mustRecord(t, b, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Mode: string(activator.ModeHold), |
| + HeldDown: []string{"h"}, MarkHoldRan: true, HoldRanExec: "a1", HoldRanAt: t0}) |
| + } |
| + e := published(one, "v2", "h") |
| + if e == 0 || e != published(other, "v2", "h") { |
| + t.Fatalf("published %d and %d: the case needs equal generations", e, published(other, "v2", "h")) |
| + } |
| + if one.ID() == other.ID() { |
| + t.Fatalf("two books share the identity %q", one.ID()) |
| + } |
| + if rec, mark, _, _ := other.RetryHostAt("bab", "blog", "v2", "h", one.ID(), e); rec || mark { |
| + t.Fatalf("a grant for one book cleared record %v, marker %v in another", rec, mark) |
| + } |
| + if !other.HeldDown("bab", "blog", "v2", "h") { |
| + t.Fatal("the other book's hold-down was lifted") |
| + } |
| + if rec, mark := grantAt(one, "v2", "h", e); !rec || !mark { |
| + t.Fatalf("the grant cleared record %v, marker %v in its own book; want both", rec, mark) |
| + } |
| +} |
| + |
| +// Rule 5: a book's identity comes from crypto/rand only. A book that needs |
| +// one and cannot get it does not open; one already on disk under the rule |
| +// needs none. |
| +func TestABookThatCannotGetAnIdentityDoesNotOpen(t *testing.T) { |
| + durable, ddDurable := diskBook(t) |
| + mustRecord(t, durable, FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h"}) |
| + ddOld := t.TempDir() |
| + if err := os.WriteFile(filepath.Join(ddOld, FailureFile), []byte(`{"schema": 2, "book": "old", "records": []}`), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + broken := errors.New("no entropy") |
| + old := randRead |
| + randRead = func([]byte) (int, error) { return 0, broken } |
| + t.Cleanup(func() { randRead = old }) |
| + for name, dd := range map[string]string{"a new book": t.TempDir(), "a book from before the rule": ddOld} { |
| + if b, err := OpenFailureBook(dd); !errors.Is(err, broken) || b != nil { |
| + t.Fatalf("%s opened (%v, %v) without an identity", name, b, err) |
| + } |
| + } |
| + b, err := OpenFailureBook(ddDurable) |
| + if err != nil { |
| + t.Fatalf("a book with its identity on disk did not open: %v", err) |
| + } |
| + if b.ID() != durable.ID() { |
| + t.Fatalf("identity %q, want %q", b.ID(), durable.ID()) |
| + } |
| +} |
diff --git a/internal/rollout/finalreview1_test.go b/internal/rollout/finalreview1_test.go| index 157ccb93f..8d52d7ff8 100644 |
| --- a/internal/rollout/finalreview1_test.go |
| +++ b/internal/rollout/finalreview1_test.go |
| @@ -169,12 +169,13 @@ func TestARetryGrantForOneBookClearsNothingInAnother(t *testing.T) { |
| } |
| } |
| -// Finding 3's failed-save half: epochs handed out after a save failed live |
| -// only in memory, and status may already have published them. An agent |
| -// crash then a reopen must never hand the same values out again for |
| -// another restriction, or the grant the console signed for the first |
| -// would clear the second. |
| -func TestAReopenAfterUnsavedEpochsNeverReissuesThem(t *testing.T) { |
| +// Finding 3's failed-save half, under the crash-epoch hard limit (readiness |
| +// round 2, Task 1, which replaced the reopen's bounded advance): a |
| +// generation handed out after a save failed lives only in memory, so status |
| +// never publishes it and no grant at it clears anything, before a crash or |
| +// after. A reopen may hand the same number out again, which no grant can |
| +// name; every value it hands out is past every one status published. |
| +func TestAnUnsavedGenerationIsNeverPublished(t *testing.T) { |
| dd := t.TempDir() |
| b, err := OpenFailureBook(dd) |
| if err != nil { |
| @@ -186,11 +187,21 @@ func TestAReopenAfterUnsavedEpochsNeverReissuesThem(t *testing.T) { |
| if _, err := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h"}); err != nil { |
| t.Fatal(err) |
| } |
| + saved := b.PublishedEpoch("bab", "blog", "v2", "h") |
| + if saved == 0 { |
| + t.Fatal("a saved restriction published nothing") |
| + } |
| readOnly(t, dd) |
| if _, err := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v3", Host: "h", HeldDown: []string{"h"}}); err == nil { |
| t.Skip("the save did not fail") |
| } |
| - published := grantFor(b, "v3", "h") // status publishes it; the console signs it |
| + if e := b.PublishedEpoch("bab", "blog", "v3", "h"); e != 0 { |
| + t.Fatalf("status published %d for a restriction not on disk, want 0", e) |
| + } |
| + unsaved := grantFor(b, "v3", "h") // a console that signed the in-memory value anyway |
| + if rec, mark := applyGrant(b, "v3", "h", unsaved); rec || mark { |
| + t.Fatal("a grant at the unsaved generation cleared before the save landed") |
| + } |
| writable(t, dd) |
| b2, err := OpenFailureBook(dd) // the agent crashed: v3's record never reached the disk |
| @@ -201,11 +212,11 @@ func TestAReopenAfterUnsavedEpochsNeverReissuesThem(t *testing.T) { |
| t.Fatal(err) |
| } |
| b2.Record(FailureRecord{Account: "bab", App: "blog", Build: "v3", Host: "h", HeldDown: []string{"h"}}) |
| - if e := b2.RestrictionEpoch("bab", "blog", "v3", "h"); e <= published.epoch { |
| - t.Fatalf("the reopened book handed out epoch %d, not past the unsaved %d", e, published.epoch) |
| + if e := b2.PublishedEpoch("bab", "blog", "v3", "h"); e <= saved { |
| + t.Fatalf("the reopened book published %d, not past the published %d", e, saved) |
| } |
| - if rec, mark := applyGrant(b2, "v3", "h", published); rec || mark { |
| - t.Fatal("the grant for the unsaved restriction cleared the new one") |
| + if rec, mark := applyGrant(b2, "v2", "h", grant{book: b.ID(), epoch: saved}); rec || mark { |
| + t.Fatal("the grant for the published restriction cleared after the app's restrictions changed") |
| } |
| } |
diff --git a/internal/rollout/manager.go b/internal/rollout/manager.go| index e6cbc0e87..fd239baca 100644 |
| --- a/internal/rollout/manager.go |
| +++ b/internal/rollout/manager.go |
| @@ -804,11 +804,13 @@ func (m *Manager) retryBuild(k appKey, build, host string) { |
| // and that restriction's epoch and book as the box last reported them |
| // (plan review findings 5 and 13; final review round 1, finding 3). The |
| // book clears build's record for host, when one holds it, and host's |
| -// hold-ran marker, when the marker names build, and only while the |
| -// restriction's epoch is still epoch and the grant names this box's book |
| -// (RetryHostAt): a grant from before a later failure of the same build, one |
| -// replayed after it was spent, or one for another book's restriction at the |
| -// same epoch, clears nothing. True means either was cleared. |
| +// hold-ran marker, when the marker names build, and only while the app's |
| +// restriction generation is still epoch, the book owes no save, and the |
| +// grant names this box's book (RetryHostAt; the crash-epoch hard limit in |
| +// FailureBook's doc): a grant from before any later change of the app's |
| +// restrictions, one replayed after it was spent, or one for another book's |
| +// restriction at the same value, clears nothing. True means either was |
| +// cleared. |
| // |
| // When the record was cleared and host's slot wants build, its next attempt |
| // is a canary. The book entry is cleared and the host's retry flag bound in |
| @@ -851,10 +853,13 @@ func (m *Manager) RetryHost(host, build, book string, epoch uint64) bool { |
| s.q.wantBuild(host, activator.CauseRetry, build, m.d.Now()) |
| } |
| s.mu.Unlock() |
| - if err != nil { |
| + if err != nil && (cleared || unmarked) { |
| m.d.Logf("rollout: retry %s on %s: %v (cleared in memory)", build, host, err) |
| - } |
| - if !cleared && !unmarked && cur != epoch { |
| + } else if err != nil { |
| + // The book owed a save and it failed again: nothing is cleared until |
| + // one lands (the crash-epoch hard limit, rule 2). |
| + m.d.Logf("rollout: retry %s on %s: the failure book cannot be saved (%v), so nothing was cleared", build, host, err) |
| + } else if !cleared && !unmarked && cur != epoch { |
| m.d.Logf("rollout: retry %s on %s: the grant is for epoch %d, the restriction's is %d, so nothing was cleared", build, host, epoch, cur) |
| } else if !cleared && !unmarked && book != m.d.Book.ID() { |
| m.d.Logf("rollout: retry %s on %s: the grant is for book %q, not this box's, so nothing was cleared", build, host, book) |
| @@ -1096,9 +1101,25 @@ func (m *Manager) dropUnownedWakes() { |
| // 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. |
| +// |
| +// The pass also retries a save the book owes (the crash-epoch hard limit, |
| +// rule 6; SaveIfDirty): while one is owed status publishes no generation |
| +// and no retry grant clears anything, so a box whose disk recovered must |
| +// not wait for another change, pointer or grant to save. Prune retries it |
| +// when it runs; a pass with no routes still calls SaveIfDirty, and so does |
| +// one whose Prune landed, for a save owed since. |
| func (m *Manager) prune(routes []registry.Route) { |
| + if err := m.pruneBook(routes); err != nil { |
| + m.d.Logf("rollout: prune failure records: %v", err) |
| + } else if err := m.d.Book.SaveIfDirty(); err != nil { |
| + m.d.Logf("rollout: save the failure book: %v", err) |
| + } |
| +} |
| + |
| +// pruneBook is prune's Prune call, nil when there are no routes to judge by. |
| +func (m *Manager) pruneBook(routes []registry.Route) error { |
| if len(routes) == 0 { |
| - return |
| + return nil |
| } |
| installed := map[[3]string]bool{} |
| for _, rt := range routes { |
| @@ -1111,9 +1132,7 @@ func (m *Manager) prune(routes []registry.Route) { |
| 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) |
| - } |
| + return m.d.Book.Prune(func(acct, app, build string) bool { return installed[[3]string{acct, app, build}] }) |
| } |
| // liftClearedHalts reopens every halted adoption cohort whose failure record |
diff --git a/internal/rollout/retrygrant_test.go b/internal/rollout/retrygrant_test.go| index 10f16dbef..ecd7dde3c 100644 |
| --- a/internal/rollout/retrygrant_test.go |
| +++ b/internal/rollout/retrygrant_test.go |
| @@ -11,13 +11,14 @@ import ( |
| ) |
| // retry is the console's signed host retry as the box applies it: a grant |
| -// naming build, carrying the epoch the box's status publishes for it. |
| +// naming build, carrying the generation the box's status publishes for it. |
| func (h *harness) retry(host, build string) bool { |
| - return h.m.RetryHost(host, build, h.book.ID(), h.book.RestrictionEpoch("bab", "blog", build, host)) |
| + return h.m.RetryHost(host, build, h.book.ID(), h.book.PublishedEpoch("bab", "blog", build, host)) |
| } |
| -// Plan review finding 13: a restriction's epoch is a persisted book |
| -// sequence that moves whenever the restriction changes for the host. |
| +// Plan review finding 13, under the crash-epoch hard limit (readiness round |
| +// 2, Task 1): a restriction's epoch is its app's generation, a persisted |
| +// book sequence that moves whenever any restriction of the app changes. |
| func TestRestrictionEpochFollowsTheRecordAndTheMarker(t *testing.T) { |
| dd := t.TempDir() |
| b, err := OpenFailureBook(dd) |
| @@ -42,12 +43,15 @@ func TestRestrictionEpochFollowsTheRecordAndTheMarker(t *testing.T) { |
| t.Fatalf("a merged hold-down: a %d, b %d; both must move past %d", e2, b.RestrictionEpoch("bab", "blog", "v2", "b"), e1) |
| } |
| b.MarkHoldRan("c", HoldRan{Account: "bab", App: "blog", Build: "v3"}) |
| - m, _ := b.HoldRanOn("c") |
| - if e := b.RestrictionEpoch("bab", "blog", "v3", "c"); e != m.Seq || e == 0 { |
| - t.Fatalf("a marker-only restriction's epoch = %d, want the marker's sequence %d", e, m.Seq) |
| + e3 := b.RestrictionEpoch("bab", "blog", "v3", "c") |
| + if e3 <= e2 { |
| + t.Fatalf("a marker-only restriction's epoch = %d, want the app's generation moved past %d", e3, e2) |
| } |
| - if e := b.RestrictionEpoch("bab", "blog", "v2", "c"); e != b.RestrictionEpoch("bab", "blog", "v2", "b") { |
| - t.Fatalf("c's epoch for v2 = %d: a marker of v3 must not count for v2", e) |
| + if e := b.RestrictionEpoch("bab", "blog", "v2", "b"); e != e3 { |
| + t.Fatalf("v2's epoch for b = %d after c's marker: one generation per app, want %d", e, e3) |
| + } |
| + if e := b.RestrictionEpoch("bab", "blog", "v3", "b"); e != 0 { |
| + t.Fatalf("b's epoch for v3 = %d: c's marker of v3 restricts only c, want 0", e) |
| } |
| if e := b.RestrictionEpoch("bab", "shop", "v3", "c"); e != 0 { |
| t.Fatalf("another app's epoch = %d, want 0", e) |
| @@ -64,9 +68,11 @@ func TestRestrictionEpochFollowsTheRecordAndTheMarker(t *testing.T) { |
| } |
| } |
| -// A book an earlier agent wrote has no epochs: the open gives each record |
| -// and marker one, newer than anything in the file, so its holds can still be |
| -// retried (a zero epoch is never accepted). |
| +// A book an earlier agent wrote has no generations: the open gives its app |
| +// one, and its marker a sequence, newer than anything in the file, so its |
| +// holds can still be retried (a zero epoch is never accepted). One app has |
| +// one generation (the crash-epoch hard limit, rule 1), so the record's and |
| +// the marker's restrictions share it. |
| func TestOpenGivesAnOlderFilesRestrictionsAnEpoch(t *testing.T) { |
| dd := t.TempDir() |
| raw, _ := json.Marshal(map[string]any{ |
| @@ -84,8 +90,11 @@ func TestOpenGivesAnOlderFilesRestrictionsAnEpoch(t *testing.T) { |
| } |
| rec := b.RestrictionEpoch("bab", "blog", "v2", "a") |
| mark := b.RestrictionEpoch("bab", "blog", "v3", "c") |
| - if rec <= 5 || mark <= 5 || rec == mark { |
| - t.Fatalf("epochs record %d, marker %d: want two distinct values newer than the file's 5", rec, mark) |
| + if rec <= 5 || rec != mark { |
| + t.Fatalf("epochs record %d, marker %d: want the app's one generation, newer than the file's 5", rec, mark) |
| + } |
| + if m, _ := b.HoldRanOn("c"); m.Seq <= 5 { |
| + t.Fatalf("the marker's sequence = %d, want one newer than the file's 5", m.Seq) |
| } |
| if b.Snapshot() <= max(rec, mark) { |
| t.Fatal("a snapshot after the open must be newer than the epochs it handed out") |
diff --git a/internal/rollout/sim_test.go b/internal/rollout/sim_test.go| index 06f29fdbb..f075fa81a 100644 |
| --- a/internal/rollout/sim_test.go |
| +++ b/internal/rollout/sim_test.go |
| @@ -1508,9 +1508,21 @@ func (s *sim) hostRetry() { |
| // that was cleared and written again. |
| rec, recorded := s.book.Lookup("bab", "blog", b) |
| clears := recorded && !slices.Contains(rec.Retried, h.name) |
| - // The console's grant names the build and the epoch the box reports |
| - // for it (Task 8); the sim reads the epoch just as the console would. |
| - ok := s.m.RetryHost(h.name, b, s.book.ID(), s.book.RestrictionEpoch("bab", "blog", b, h.name)) |
| + // The console's grant names the build and the generation the box |
| + // publishes for it (Task 8; readiness round 2 Task 1): the sim reads it |
| + // just as the console would. A book that owes a save publishes none, and |
| + // clears nothing for a grant (the crash-epoch hard limit, rule 2): the |
| + // retry then changes nothing, in the code or the model. |
| + epoch := s.book.PublishedEpoch("bab", "blog", b, h.name) |
| + if epoch == 0 && s.book.SaveOwed() { |
| + s.logf("instances retry %s onto %s: no generation published, the book owes a save", h.name, b) |
| + return |
| + } |
| + ok := s.m.RetryHost(h.name, b, s.book.ID(), epoch) |
| + if !ok && s.book.SaveOwed() { |
| + s.logf("instances retry %s onto %s: refused, the book owes a save", h.name, b) |
| + return |
| + } |
| // The retry exempts h from the record that stands now, and from no |
| // record written after that one goes (pass): with no record there is |
| // nothing to exempt it from. |
diff --git a/internal/rollout/status.go b/internal/rollout/status.go| index 5cedf5a4c..7c6bde9b5 100644 |
| --- a/internal/rollout/status.go |
| +++ b/internal/rollout/status.go |
| @@ -84,9 +84,10 @@ func (m *Manager) Status() status.RolloutReport { |
| if rec, ok := m.d.Book.Lookup(s.key.acct, s.key.app, h.wantBuild); ok && |
| m.d.Book.Blocks(s.key.acct, s.key.app, h.wantBuild, host, h.wantBuild == h.good) { |
| hr.Attempt, hr.AttemptBuild, hr.HeldBy, hr.Reason = status.AttemptFailed, rec.Build, rec.Build, reasonWord(rec.Reason) |
| - // The epoch the console signs into its retry grant |
| - // (plan review finding 13). |
| - hr.HeldEpoch = m.d.Book.RestrictionEpoch(s.key.acct, s.key.app, rec.Build, host) |
| + // The generation the console signs into its retry grant |
| + // (plan review finding 13), 0 while the book owes a |
| + // save (the crash-epoch hard limit, rule 2). |
| + hr.HeldEpoch = m.d.Book.PublishedEpoch(s.key.acct, s.key.app, rec.Build, host) |
| hr.HeldBook = m.d.Book.ID() // final review round 1, finding 3 |
| if m.d.Book.HeldDown(s.key.acct, s.key.app, h.wantBuild, host) { |
| hr.Attempt = status.AttemptDown |
| @@ -107,7 +108,7 @@ func (m *Manager) Status() status.RolloutReport { |
| hr.Attempt = status.AttemptDown |
| case hr.Attempt == "": |
| hr.HeldBy = mark.Build |
| - hr.HeldEpoch = m.d.Book.RestrictionEpoch(s.key.acct, s.key.app, mark.Build, host) |
| + hr.HeldEpoch = m.d.Book.PublishedEpoch(s.key.acct, s.key.app, mark.Build, host) |
| hr.HeldBook = m.d.Book.ID() |
| if rec, ok := m.d.Book.Lookup(s.key.acct, s.key.app, mark.Build); ok { |
| hr.Reason = reasonWord(rec.Reason) |
diff --git a/internal/status/snapshot.go b/internal/status/snapshot.go| index 86e21d544..375ffa728 100644 |
| --- a/internal/status/snapshot.go |
| +++ b/internal/status/snapshot.go |
| @@ -325,10 +325,12 @@ type HostRollout struct { |
| ConfigPending bool `json:"config_pending,omitempty"` |
| // ConfigError is a reason word (Reason*), never raw error text. |
| ConfigError string `json:"config_error,omitempty"` |
| - // HeldEpoch is the epoch of HeldBy's restriction of this instance |
| - // (rollout.FailureBook.RestrictionEpoch): the console signs it into a |
| - // `carlos instances retry` grant, which the box honours only while the |
| - // restriction is unchanged. Absent from an older box, which is 0. |
| + // HeldEpoch is the generation of HeldBy's restriction of this instance |
| + // as published (rollout.FailureBook.PublishedEpoch): the console signs |
| + // it into a `carlos instances retry` grant, which the box honours only |
| + // while the app's restrictions are unchanged. 0 on a held instance means |
| + // the box's failure book is not saved yet; absent from an older box, |
| + // which is 0 too. |
| HeldEpoch uint64 `json:"held_epoch,omitempty"` |
| // HeldBook is the identity of the failure book HeldEpoch counts in |
| // (rollout.FailureBook.ID): epochs are a counter every book starts |