amadan
zdd-readiness: an uncertain nested start is a hold failure, never a re-runnable abandon (ruling 206)
A nested start root accepted whose outcome is uncertain may have run its build (Astra R3 ruling 206). holdResult now answers SwapDown for it (proc nil, startMayHaveRun), so done() holds the build down and halts the cohort, where it used to abandon with Executed true. spawnPlanned treats it as B's failure (w.failed, so GenFailed under the outcome fence) in hold mode always, and in overlap only for errStartedThenExited, the unit that exited before root observed it; other uncertain overlap starts keep GenCancelled{Executed:true}. With no process handle, the good build starts only when the backend's kill proved the crashed generation gone (no errGenUnproven in the chain; plan review #9); unproven, the wake serves nothing and stays an unproven wake. errStartedThenExited is found through exitedUnobserved, whose text is exactly the old one (flag-off nested wakes and members read it).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8 files changed,
+239
−12
internal/activator/backend_exec.go+4 −2internal/activator/containment.go+17 −0internal/activator/containment_test.go+24 −0internal/activator/swap_executed_test.go+52 −0internal/activator/swap_nested_test.go+9 −2internal/activator/swaphold.go+11 −5internal/activator/wakeplan.go+26 −3internal/activator/wakeplan_test.go+96 −0
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go| index 34592be3c..67a0ae771 100644 |
| --- a/internal/activator/backend_exec.go |
| +++ b/internal/activator/backend_exec.go |
| @@ -1704,7 +1704,9 @@ func (b *ExecBackend) spawnUnitFrom(ctx context.Context, rt registry.Route, sock |
| } |
| if gen != 0 && (err != nil || inv == "") { |
| if err == nil { |
| - err = fmt.Errorf("%s exited before it was observed", unitName(instancectl.RoleRun, rt.Host, gen)) |
| + // The unit ran and exited before root observed it: the build |
| + // crashed at its start (errStartedThenExited, ruling 206). |
| + err = exitedUnobserved{unit: unitName(instancectl.RoleRun, rt.Host, gen)} |
| } |
| if kerr := b.killFailedGen(ctx, rt, gen); kerr != nil { |
| // Nothing is released: the generation may still be running. |
| @@ -1720,7 +1722,7 @@ func (b *ExecBackend) spawnUnitFrom(ctx context.Context, rt registry.Route, sock |
| // The server read the invocation after the start and found none: |
| // the unit already exited (plan 2 final review). Nothing is |
| // running, so there is nothing to stop; the wake failed. |
| - return fail(markStartUncertain(fmt.Errorf("%s: %s exited before it was observed", rt.Host, unitName(instancectl.RoleRun, rt.Host, gen)))) |
| + return fail(markStartUncertain(fmt.Errorf("%s: %w", rt.Host, exitedUnobserved{unit: unitName(instancectl.RoleRun, rt.Host, gen)}))) |
| } |
| started := tier |
| if started == (cgroup.Limit{}) { |
diff --git a/internal/activator/containment.go b/internal/activator/containment.go| index fdae43c47..2c213f67c 100644 |
| --- a/internal/activator/containment.go |
| +++ b/internal/activator/containment.go |
| @@ -37,6 +37,23 @@ func (e startUncertain) Unwrap() error { return e.err } |
| // markStartUncertain wraps err as startUncertain. |
| func markStartUncertain(err error) error { return startUncertain{err: err} } |
| +// errStartedThenExited is the one uncertain start whose outcome is known to |
| +// be a crash at the start: root started the unit and it had exited before |
| +// root read its invocation (Astra R3 ruling 206, zdd-readiness Task 6). The |
| +// build ran and died on its own, so a planned wake fails the build in |
| +// overlap mode too, where any other uncertain start is only "may have run". |
| +// It is wrapped by markStartUncertain, so startMayHaveRun still reads it. |
| +var errStartedThenExited = errors.New("generation exited before it was observed") |
| + |
| +// exitedUnobserved is errStartedThenExited in the unit's own words. Its |
| +// text is exactly the one spawnUnitFrom always wrote, because a flag-off |
| +// nested wake fails with it and memberSafeWakeReason passes it to the |
| +// member; only errors.Is learns the sentinel. |
| +type exitedUnobserved struct{ unit string } |
| + |
| +func (e exitedUnobserved) Error() string { return e.unit + " exited before it was observed" } |
| +func (e exitedUnobserved) Is(target error) bool { return target == errStartedThenExited } |
| + |
| // startMayHaveRun is a spawn's execution evidence, tri-state collapsed to |
| // what an attempt may rely on (final review wave 2, N4): false only when it |
| // is PROVEN that nothing of the build ran (no process came back, and the |
diff --git a/internal/activator/containment_test.go b/internal/activator/containment_test.go| index f3c0b9a37..feae25cfb 100644 |
| --- a/internal/activator/containment_test.go |
| +++ b/internal/activator/containment_test.go |
| @@ -280,3 +280,27 @@ func TestSpawnInCgroupAtRefusesAStrictHost(t *testing.T) { |
| t.Errorf("a refused spawn made a child cgroup: %v", err) |
| } |
| } |
| + |
| +// zdd-readiness Task 6: errStartedThenExited is found through every wrap |
| +// spawnUnitFrom gives it (the uncertain mark, and beside errGenUnproven when |
| +// the kill is unproven), and the text is the one it always was: a flag-off |
| +// nested wake fails with it, and the member reads it. |
| +func TestStartedThenExitedIsUncertainAndKeepsItsText(t *testing.T) { |
| + unit := exitedUnobserved{unit: "carlos-instance-gen@h.example.com:7.service"} |
| + proven := markStartUncertain(fmt.Errorf("%s: start unit: %w", "h.example.com", unit)) |
| + unproven := markStartUncertain(fmt.Errorf("%s: start unit: %w; %w: %v", "h.example.com", unit, errGenUnproven, errors.New("stop timed out"))) |
| + for _, err := range []error{proven, unproven} { |
| + if !errors.Is(err, errStartedThenExited) || !startMayHaveRun(nil, err) { |
| + t.Fatalf("%v: errStartedThenExited %v, may have run %v; want both", err, errors.Is(err, errStartedThenExited), startMayHaveRun(nil, err)) |
| + } |
| + } |
| + if want := "h.example.com: start unit: carlos-instance-gen@h.example.com:7.service exited before it was observed"; proven.Error() != want { |
| + t.Fatalf("text %q, want %q", proven.Error(), want) |
| + } |
| + if !genStartUnproven(unproven) || genStartUnproven(proven) { |
| + t.Fatal("only the unproven kill carries errGenUnproven") |
| + } |
| + if errors.Is(markStartUncertain(errors.New("the unit failed to start")), errStartedThenExited) { |
| + t.Fatal("another uncertain start reads as a crash at the start") |
| + } |
| +} |
diff --git a/internal/activator/swap_executed_test.go b/internal/activator/swap_executed_test.go| index d1fef93b7..1cb563549 100644 |
| --- a/internal/activator/swap_executed_test.go |
| +++ b/internal/activator/swap_executed_test.go |
| @@ -179,6 +179,58 @@ func TestASwapWhoseNestedStartIsUncertainReportsExecution(t *testing.T) { |
| } |
| } |
| +// Astra R3 ruling 206 (zdd-readiness Task 6): a hold's nested start that |
| +// root accepted and whose outcome is uncertain may have run its build, so |
| +// the hold is down (SwapDown: done() holds the build down and halts the |
| +// cohort), never an abandonment with Executed, whose deferral would let the |
| +// scheduler run the build again. That holds for a unit that exited before |
| +// it was observed (errStartedThenExited) and for any other uncertain start, |
| +// whether the backend's kill proved the generation gone or not. Only root's |
| +// refusal of the staged build before it started anything is still an |
| +// abandonment, with nothing executed. Before the fix the kill-proven cases |
| +// were abandoned with Executed true. |
| +func TestAHoldWhoseNestedStartIsUncertainIsDown(t *testing.T) { |
| + unitFailed := errors.New("the unit failed to start") |
| + exitedUnobserved := func(instancectl.StagedExe) (string, error) { return "", nil } |
| + rootFailed := func(instancectl.StagedExe) (string, error) { return "", unitFailed } |
| + rootRefused := func(instancectl.StagedExe) (string, error) { |
| + return "", fmt.Errorf("%w: digest mismatch", instancectl.ErrStagedExe) |
| + } |
| + for _, tc := range []struct { |
| + name string |
| + answer func(instancectl.StagedExe) (string, error) |
| + stopErr error |
| + outcome SwapOutcome |
| + executed bool |
| + }{ |
| + {"the unit exited before it was observed, kill proven", exitedUnobserved, nil, SwapDown, true}, |
| + {"the unit exited before it was observed, kill unproven", exitedUnobserved, errors.New("stop timed out"), SwapDown, true}, |
| + {"root's start failed, kill proven", rootFailed, nil, SwapDown, true}, |
| + {"root's start failed, kill unproven", rootFailed, errors.New("stop timed out"), SwapDown, true}, |
| + {"root refused the staged build", rootRefused, nil, SwapAbandoned, false}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + f := newSwapFixture(t) |
| + b := newLaunchBackend(t, "nested", f.b) |
| + b.ctl.exeAnswer, b.ctl.stopGenErr = tc.answer, tc.stopErr |
| + f.a.backend = b |
| + id := markerScript(t, t.TempDir(), "v2", filepath.Join(t.TempDir(), "v2.ran")) |
| + at := f.attempt(ModeHold) |
| + at.StagedPath, at.Digest = id.StagedPath, id.Digest |
| + res := f.a.Swap(context.Background(), at, f.newest) |
| + if res.Outcome != tc.outcome || res.Executed != tc.executed { |
| + t.Fatalf("Swap = %+v, want %s with Executed %v", res, tc.outcome, tc.executed) |
| + } |
| + b.ctl.mu.Lock() |
| + asked := len(b.ctl.got) |
| + b.ctl.mu.Unlock() |
| + if asked != 1 { |
| + t.Fatalf("root was asked to start %d units, want 1", asked) |
| + } |
| + }) |
| + } |
| +} |
| + |
| // Final review wave 2 (N4): ReplaceWorker learns what its attempt's |
| // respawn did before it answers, even when its caller stops waiting. The |
| // respawn runs on (it is the entry's, on the service lifetime), so it is |
diff --git a/internal/activator/swap_nested_test.go b/internal/activator/swap_nested_test.go| index 74ba77514..2c5891312 100644 |
| --- a/internal/activator/swap_nested_test.go |
| +++ b/internal/activator/swap_nested_test.go |
| @@ -26,8 +26,12 @@ type exeControl struct { |
| tiers []int64 // StartGenExe's memoryMax, per call |
| sidecarTiers []int64 // StartSidecarExe's memoryMax, per call |
| exeErr error // what StartGenExe answers, when set |
| - stopGens int // StopGen calls |
| - stopGenErr error // what StopGen answers |
| + // exeAnswer, when set, is what StartGenExe answers for each staged exe |
| + // (in place of exeErr): an empty invocation with no error is a unit |
| + // that exited before root observed it. |
| + exeAnswer func(instancectl.StagedExe) (string, error) |
| + stopGens int // StopGen calls |
| + stopGenErr error // what StopGen answers |
| } |
| func (c *exeControl) Start(_ context.Context, _ string, role instancectl.Role, _ int64) (string, error) { |
| @@ -68,6 +72,9 @@ func (c *exeControl) StartGenExe(_ context.Context, _ string, _ uint64, memoryMa |
| defer c.mu.Unlock() |
| c.got = append(c.got, x) |
| c.tiers = append(c.tiers, memoryMax) |
| + if c.exeAnswer != nil { |
| + return c.exeAnswer(x) |
| + } |
| if c.exeErr != nil { |
| return "", c.exeErr |
| } |
diff --git a/internal/activator/swaphold.go b/internal/activator/swaphold.go| index e569eeb16..8da573642 100644 |
| --- a/internal/activator/swaphold.go |
| +++ b/internal/activator/swaphold.go |
| @@ -184,8 +184,7 @@ func (a *Service) withdrawOrTake(e *actEntry, h *holdSpec, why error) (holdWoke, |
| // a wake that came up healthy ran the build: SwapResult.Executed (final |
| // review F8). A pinned start handed to root whose outcome is uncertain |
| // (startUncertain) is executed-or-unknown too (final review wave 2, N4), |
| -// though without a process or an unproven kill it is still abandoned |
| -// rather than read as the build's failure. |
| +// and it is the build's failure: the hold is down (Astra R3 ruling 206). |
| func (a *Service) holdResult(sctx context.Context, at Attempt, newest func(string) uint64, hw holdWoke, drain time.Duration) (out SwapResult) { |
| if hw.err != nil { |
| return abandon(hw.err) |
| @@ -196,13 +195,20 @@ func (a *Service) holdResult(sctx context.Context, at Attempt, newest func(strin |
| a.mu.Unlock() |
| defer func() { out.Executed = execed || werr == nil || startMayHaveRun(nil, werr) }() |
| switch { |
| + case werr != nil && proc == nil && startMayHaveRun(nil, werr): |
| + // A nested start root accepted whose outcome is uncertain (it |
| + // exited before it was observed, root's start call failed, or its |
| + // kill is unproven) may have run the build against the database |
| + // (Astra R3 ruling 206, zdd-readiness Task 6). It is a failure of |
| + // the build whatever ended the wake: down, so done() holds the build |
| + // down and halts the cohort. An abandonment with Executed would be |
| + // deferred and could run the build again. |
| + return SwapResult{Outcome: SwapDown, Gen: g, Reason: werr.Error(), Drain: drain} |
| case werr != nil && !execed: |
| // The pinned wake failed before its build's exec: the lease, the |
| // restore, the row, the staged bytes, the claims admission. Nothing |
| // of the build ran, so this is no failure of it and nothing is held |
| - // down (spec §6.4, §8.3; index P5/P6). The attempt is abandoned. (A |
| - // nested start root answered with an uncertain outcome lands here |
| - // too, reported Executed: see above.) |
| + // down (spec §6.4, §8.3; index P5/P6). The attempt is abandoned. |
| return abandon(werr) |
| case werr == nil: |
| if at.Canary { |
diff --git a/internal/activator/wakeplan.go b/internal/activator/wakeplan.go| index 3849e2989..14c8e45d6 100644 |
| --- a/internal/activator/wakeplan.go |
| +++ b/internal/activator/wakeplan.go |
| @@ -391,8 +391,20 @@ func (a *Service) spawnPlanned(ctx context.Context, e *actEntry, rt registry.Rou |
| if err == nil { |
| return proc, ver, withIdentity(g, boot), nil |
| } |
| + // A nested start root accepted whose outcome is uncertain, with no |
| + // process back, may have run B (Astra R3 ruling 206, zdd-readiness |
| + // Task 6). In hold mode that is always B's failure: nothing falls back |
| + // from a hold, and a wake reported as cancelled would leave the build |
| + // free to boot again. In overlap mode only a unit that exited before it |
| + // was observed (errStartedThenExited) is a proven crash at the start and |
| + // fails B; any other uncertain overlap start stays "may have run" |
| + // (afterWake's GenCancelled with Executed), unless its kill is |
| + // unproven, which fails B below as before. |
| + uncertain := proc == nil && startMayHaveRun(nil, err) |
| + crashed := uncertain && errors.Is(err, errStartedThenExited) |
| + uncertainFailure := uncertain && (plan.Mode == ModeHold || crashed) |
| executed := proc != nil || genStartUnproven(err) |
| - if !executed || classifyStep(ctx, err) != stepFailed || errors.Is(err, errPinMismatch) { |
| + if !uncertainFailure && (!executed || classifyStep(ctx, err) != stepFailed || errors.Is(err, errPinMismatch)) { |
| // Nothing of the build ran, or the wake's lifecycle ended it |
| // (maintenance, Shutdown, a stop), or its gate refused, or the |
| // backend ran something else: no failure of the build (spec §6.4, |
| @@ -403,10 +415,21 @@ func (a *Service) spawnPlanned(ctx context.Context, e *actEntry, rt registry.Rou |
| if plan.FirstWake { |
| w.failed, w.failReason = &boot, err.Error() |
| } |
| - if !canFall || proc == nil { |
| + if !canFall { |
| return proc, ver, g, err |
| } |
| - if kerr := a.killFailed(rt, g, proc, false); kerr != nil { |
| + if proc == nil { |
| + // No handle to kill. The good build starts only beside a B whose |
| + // end is proven (plan review finding 9): a crash at the start that |
| + // the backend's own kill proved gone (no errGenUnproven in the |
| + // chain). Every other overlap start that reaches here with no |
| + // process carries errGenUnproven: its end is unproven, so nothing |
| + // more starts, the wake serves nothing, and doWake keeps the lease |
| + // and the generation as an unproven wake. |
| + if !crashed || genStartUnproven(err) { |
| + return proc, ver, g, err |
| + } |
| + } else if kerr := a.killFailed(rt, g, proc, false); kerr != nil { |
| return proc, ver, g, fmt.Errorf("%w; its end was not proven, so the good build was not started: %v", err, kerr) |
| } |
| good := *plan.Fallback |
diff --git a/internal/activator/wakeplan_test.go b/internal/activator/wakeplan_test.go| index 954e57512..99a8a6625 100644 |
| --- a/internal/activator/wakeplan_test.go |
| +++ b/internal/activator/wakeplan_test.go |
| @@ -20,6 +20,7 @@ import ( |
| "github.com/carlosframework/platform/internal/cgroup" |
| "github.com/carlosframework/platform/internal/hostclaim" |
| "github.com/carlosframework/platform/internal/hostwrite" |
| + "github.com/carlosframework/platform/internal/instancectl" |
| "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| "github.com/carlosframework/platform/internal/rolloutwords" |
| @@ -1329,3 +1330,98 @@ func TestAFirstWakeWithTimeForAVerdictStillExecsB(t *testing.T) { |
| t.Fatalf("fallback event = %+v", ev) |
| } |
| } |
| + |
| +// Astra R3 ruling 206 and plan review finding 9 (zdd-readiness Task 6): a |
| +// planned first wake whose nested start root accepted, with an uncertain |
| +// outcome, may have run its build. In hold mode that is always a failure of |
| +// the build (GenFailed, under the outcome fence), never a cancellation the |
| +// scheduler could boot the build again after. In overlap only a unit that |
| +// exited before it was observed (errStartedThenExited) is a proven crash at |
| +// the start, so it fails the build, and the good build boots in its place |
| +// only when the backend's kill proved the generation gone; unproven, the |
| +// wake serves nothing. Any other uncertain overlap start keeps its |
| +// GenCancelled with Executed and boots nothing. Each wake sends its token |
| +// exactly one terminal event. Before the fix both hold cases sent |
| +// GenCancelled and the proven overlap crash served nothing. |
| +func TestAPlannedWakeWhoseNestedStartIsUncertain(t *testing.T) { |
| + unitFailed := errors.New("the unit failed to start") |
| + // B (v2) is answered as the case says; the good build (v1) starts. |
| + bAnswers := func(inv string, err error) func(instancectl.StagedExe) (string, error) { |
| + return func(x instancectl.StagedExe) (string, error) { |
| + if x.Version == "v2" { |
| + return inv, err |
| + } |
| + return "inv-7", nil |
| + } |
| + } |
| + unproven := errors.New("stop timed out") |
| + for _, tc := range []struct { |
| + name string |
| + mode SwapMode |
| + answer func(instancectl.StagedExe) (string, error) |
| + stopErr error |
| + want GenEventKind |
| + executed bool // GenCancelled only |
| + goodAsked bool // root was asked to start the good build, which serves |
| + }{ |
| + {"hold, the unit exited before it was observed", ModeHold, bAnswers("", nil), nil, GenFailed, false, false}, |
| + {"hold, root's start failed", ModeHold, bAnswers("", unitFailed), nil, GenFailed, false, false}, |
| + {"overlap, the unit exited before it was observed, kill proven", ModeOverlap, bAnswers("", nil), nil, GenFellBack, false, true}, |
| + {"overlap, the unit exited before it was observed, kill unproven", ModeOverlap, bAnswers("", nil), unproven, GenFailed, false, false}, |
| + {"overlap, root's start failed, kill proven", ModeOverlap, bAnswers("", unitFailed), nil, GenCancelled, true, false}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + b := newLaunchBackend(t, "nested", &fakeSwapBackend{fakeBackend: &fakeBackend{}}) |
| + b.ctl.exeAnswer, b.ctl.stopGenErr = tc.answer, tc.stopErr |
| + dir := t.TempDir() |
| + next := markerScript(t, dir, "v2", filepath.Join(t.TempDir(), "v2.ran")) |
| + good := markerScript(t, dir, "v1", filepath.Join(t.TempDir(), "v1.ran")) |
| + plan := WakePlan{Boot: next, Mode: tc.mode, FirstWake: true, Token: 7} |
| + if tc.mode == ModeOverlap { |
| + plan.Fallback = &good |
| + } |
| + a, evs := launchActivator(t, b, plan) |
| + done, err := a.beginRequest(context.Background(), testRoute("one.example.com")) |
| + if tc.goodAsked { |
| + if err != nil { |
| + t.Fatalf("wake = %v, want the good build serving", err) |
| + } |
| + done() |
| + } else if err == nil { |
| + done() |
| + t.Fatal("the wake served, want it failed with nothing serving") |
| + } |
| + ev := evs.wait(t, tc.want) |
| + if ev.Token != 7 { |
| + t.Fatalf("%s event = %+v, want the plan's token", tc.want, ev) |
| + } |
| + switch tc.want { |
| + case GenFailed: |
| + if ev.Failed.Version != "v2" { |
| + t.Fatalf("failure event = %+v, want v2 failed", ev) |
| + } |
| + case GenFellBack: |
| + if ev.Failed.Version != "v2" || ev.Gen.Build != "v1" { |
| + t.Fatalf("fallback event = %+v, want v2 failed and v1 serving", ev) |
| + } |
| + case GenCancelled: |
| + if ev.Executed != tc.executed { |
| + t.Fatalf("cancellation = %+v, want Executed %v", ev, tc.executed) |
| + } |
| + } |
| + time.Sleep(50 * time.Millisecond) |
| + if k := evs.kinds(); len(k) != 1 { |
| + t.Fatalf("events %v, want exactly one terminal event for the token", k) |
| + } |
| + b.ctl.mu.Lock() |
| + var asked []string |
| + for _, x := range b.ctl.got { |
| + asked = append(asked, x.Version) |
| + } |
| + b.ctl.mu.Unlock() |
| + if want := map[bool]int{true: 2, false: 1}[tc.goodAsked]; len(asked) != want || asked[0] != "v2" { |
| + t.Fatalf("root was asked to start %v, want v2 first and %d start(s) in all", asked, want) |
| + } |
| + }) |
| + } |
| +} |