amadan
settings-4 task 5a: fix round 1: a refused record restores the replaced worker
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+19
−2
internal/activator/beforeexec_test.go+6 −0internal/activator/swap.go+3 −2internal/activator/workerswap.go+10 −0
diff --git a/internal/activator/beforeexec_test.go b/internal/activator/beforeexec_test.go| index 6c21d0627..239bf27c9 100644 |
| --- a/internal/activator/beforeexec_test.go |
| +++ b/internal/activator/beforeexec_test.go |
| @@ -400,4 +400,10 @@ func TestBeforeExecRunsBeforeReplaceWorker(t *testing.T) { |
| if got := a.entryServing(host); got.Version == "v2" { |
| t.Fatalf("serving %+v: v2 ran unrecorded", got) |
| } |
| + // Fix round 1: nothing of v2 ran, so the replaced v1 is restored at |
| + // once, with no sweep: the worker is not left down until the next |
| + // crash restart. |
| + if got, w := a.entryServing(host), b.lastWorker(); got != fakeIdentity("v1") || b.workerStarts() != 2 || w == nil || w.Exited() { |
| + t.Fatalf("serving %+v with %d children (last exited: %v); want v1 restored and running without a sweep", got, b.workerStarts(), w != nil && w.Exited()) |
| + } |
| } |
diff --git a/internal/activator/swap.go b/internal/activator/swap.go| index 10b8a86f7..1756114f6 100644 |
| --- a/internal/activator/swap.go |
| +++ b/internal/activator/swap.go |
| @@ -188,8 +188,9 @@ const ( |
| // ErrBuildNotPublished it is a new sentinel, so naming it here changes |
| // nothing main did: a worker respawn it refuses logs a refusal, not |
| // "failed to respawn" and a member error event, and the sweep asks |
| -// again; a ReplaceWorker attempt it refuses is abandoned without a |
| -// restore, and the sweep's crash restart respawns the serving build. |
| +// again; a ReplaceWorker attempt it refuses is abandoned and restores |
| +// the build it replaced at once (its veto switch names this sentinel |
| +// before the NoOwnership case), since nothing of the attempt ran. |
| // |
| // errSwapStopped (the barrier's cancel, from memberCurrentLocked) is |
| // NoOwnership too: the entry is stopping, so nothing may spawn for it. |
diff --git a/internal/activator/workerswap.go b/internal/activator/workerswap.go| index f48a59180..6b9c70503 100644 |
| --- a/internal/activator/workerswap.go |
| +++ b/internal/activator/workerswap.go |
| @@ -121,6 +121,16 @@ func (a *Service) ReplaceWorker(ctx context.Context, at Attempt, fallback *relea |
| // spawns, a restore included, and the entry stops through the |
| // sweep's lease-loss path. |
| switch { |
| + case errors.Is(veto, errBuildStateUnwritten) && !executed: |
| + // The attempt's build state could not be written, so its |
| + // exec was refused (settings spec §4.2): nothing of it ran, |
| + // and ownership is intact. verdictOf reads it as NoOwnership |
| + // (so a plain respawn logs a refusal), but here the replaced |
| + // build comes back at once, as for an unpublished build (the |
| + // incarnation-unchanged arm below). Its restore records its |
| + // own build first, and a refusal there leaves the dead child |
| + // to the sweep's crash restart. |
| + a.restoreLocked(e, prev) |
| case verdictOf(veto) != gateSuperseded: |
| case executed && at.Mode == ModeHold: |
| // A hold build that was exec'd may have migrated the database, |