amadan

carlos / platform Public

zdd-5 final review: restore a superseded worker only if its build never ran (F2)

The attempt's respawn now reports its execution evidence (attemptExec:
a process of the attempt's build was started, installed or shed), next
to attemptVeto. ReplaceWorker restores the replaced build on a
supersession only when that evidence is absent. A hold attempt whose
build was exec'd and then superseded before its install (the install
check shed it) may have migrated the database: nothing is restored,
the entry stops (beginStopLocked) and the attempt is abandoned, not a
failure of the build. An overlap attempt superseded after its exec
restores as before, and so does a hold superseded at its exec gate.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 35772c9aa4206385d7a97c9f938d57876af473da parent 607da3e
3 files changed, +110 −10
  • internal/activator/activator.go +10 −1
  • internal/activator/workerswap.go +25 −9
  • internal/activator/workerswap_test.go +75 −0
diff --git a/internal/activator/activator.go b/internal/activator/activator.go
index ee15e54c2..04c073897 100644
--- a/internal/activator/activator.go
+++ b/internal/activator/activator.go
@@ -582,7 +582,11 @@ type actEntry struct {
// it is called, so a later respawn (a restore, a fallback, a crash
// restart) never inherits them, and a ReplaceWorker that stops waiting
// cannot disarm a respawn still running. attemptVeto is that respawn's
- // answer when the gate refused, read and cleared by ReplaceWorker.
+ // answer when the gate refused, read and cleared by ReplaceWorker, and
+ // attemptExec its execution evidence (final review F2): the respawn
+ // started a process of the attempt's build, whether or not it was then
+ // installed or shed. A supersession restores the replaced build only
+ // when it is false.
// heldServing is the publication holdServing kept back, delivered by
// PublishHeldServing. workerInc is the worker child's incarnation,
// bumped at every install (a wake, a respawn): an attempt counts only if
@@ -592,6 +596,7 @@ type actEntry struct {
retireTook time.Duration
attemptGate func() error
attemptVeto error
+ attemptExec bool
holdServing bool
heldServing *heldServing
workerInc uint64
@@ -2783,6 +2788,10 @@ func (a *Service) respawnWorkerLocked(e *actEntry, stopOld Proc, why string, ont
case refused == nil && verdictOf(err) != gateProceed:
e.attemptVeto = err
}
+ // The execution evidence (final review F2): a process of the
+ // attempt's build was started, installed below or shed. Without
+ // it, nothing of the build ran.
+ e.attemptExec = proc != nil
}
switch {
case owned && !retired && live:
diff --git a/internal/activator/workerswap.go b/internal/activator/workerswap.go
index ba3e30f27..7a646c1a3 100644
--- a/internal/activator/workerswap.go
+++ b/internal/activator/workerswap.go
@@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
+ "log"
"github.com/carlosframework/platform/internal/release"
)
@@ -25,8 +26,12 @@ import (
// its termination proof (respawnWorkerLocked). The attempt's own respawn
// also asks the attempt's §7.4 checks there, at all three of its points
// (before the preparation, at the exec, before the install). A refusal
-// that is a supersession brings back the build the attempt replaced; any
-// other refusal (no ownership) spawns nothing at all (spec §7.4).
+// that is a supersession brings back the build the attempt replaced, unless
+// the attempt is a hold whose build was exec'd before the refusal (its
+// install check shed it): that build may have migrated the database, so
+// nothing is restored and the entry stops (final review F2; spec §6.4,
+// §8.3). Any other refusal (no ownership) spawns nothing at all (spec
+// §7.4).
//
// The scheduler runs the cohort-failure transition from the result, then
// calls PublishHeldServing for a fallback's identity. newest is called with
@@ -68,7 +73,7 @@ func (a *Service) ReplaceWorker(ctx context.Context, at Attempt, fallback *relea
// its preparation, at the exec and before the install, after the old
// child has retired (spec §6.3, §7.4, §8.1).
e.attemptGate = func() error { return a.swapChecksLocked(e, at, newest) }
- e.attemptVeto = nil
+ e.attemptVeto, e.attemptExec = nil, false
// Spec §9.4: the bounce event names the attempt's cause.
emitEvent(a.Logs, e.route, "info", bounceText(true, at.Cause))
// The incarnation before the respawn: the attempt's child counts only if
@@ -92,19 +97,30 @@ func (a *Service) ReplaceWorker(ctx context.Context, at Attempt, fallback *relea
return abandon(errSwapStopped)
}
a.mu.Lock()
- veto := e.attemptVeto
- e.attemptVeto = nil
+ veto, executed := e.attemptVeto, e.attemptExec
+ e.attemptVeto, e.attemptExec = nil, false
if a.entries[at.Host] != e || e.state != actAwake {
a.mu.Unlock()
return abandon(errSwapStopped)
}
if veto != nil {
- // The respawn's re-check refused the attempt's build: nothing of it
- // runs. Only a supersession brings back the build it replaced (index
- // P5 #1); without ownership (a lost lease, a fence, shutdown) nothing
+ // The respawn's re-check refused the attempt's build: before its
+ // exec nothing of it ran; at the install check its child was shed.
+ // Only a supersession brings back the build it replaced (index P5
+ // #1); without ownership (a lost lease, a fence, shutdown) nothing
// spawns, a restore included, and the entry stops through the
// sweep's lease-loss path.
- if verdictOf(veto) == gateSuperseded {
+ switch {
+ case verdictOf(veto) != gateSuperseded:
+ case executed && at.Mode == ModeHold:
+ // A hold build that was exec'd may have migrated the database,
+ // and nothing falls back from a hold (spec §6.4, §8.3; final
+ // review F2): no restore of the replaced build. The entry stops
+ // and stays down until its next attempt or wake. Abandoned, not
+ // a failure of the build: it was superseded, not seen to fail.
+ log.Printf("activator: worker %s: hold attempt onto %s was superseded after its exec; the build it replaced (%s) is not restored, and the worker stops", at.Host, onto.Version, prev.Version)
+ a.beginStopLocked(e)
+ default:
a.restoreLocked(e, prev)
}
a.mu.Unlock()
diff --git a/internal/activator/workerswap_test.go b/internal/activator/workerswap_test.go
index 6849b5c6e..ae76a37b2 100644
--- a/internal/activator/workerswap_test.go
+++ b/internal/activator/workerswap_test.go
@@ -694,3 +694,78 @@ func TestAFallbackWakeRecordsTheFailureBeforeItPublishes(t *testing.T) {
t.Fatal("the fallback's identity was published before its failure transition")
}
}
+
+// Final review F2 (spec §§6.4/8.3, P5's hold-everywhere invariant): "not
+// installed" is not evidence that a build never ran. A hold attempt whose
+// build was exec'd, and then superseded before its install (the install
+// check sheds the child), may have migrated the database: the build it
+// replaced is NOT restored, the entry stops, and the attempt is abandoned,
+// not a failure of the build. Superseded at its exec gate instead, the hold
+// build provably never ran, so the replaced build comes back as before. An
+// overlap attempt superseded after its exec still restores, as before.
+func TestASupersededWorkerAttemptRestoresOnlyWhatProvablyNeverRan(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ mode SwapMode
+ executed bool // superseded after the exec (true) or at the exec gate
+ restores bool
+ }{
+ {"hold, superseded after its exec", ModeHold, true, false},
+ {"hold, superseded at its exec gate", ModeHold, false, true},
+ {"overlap, superseded after its exec", ModeOverlap, true, true},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ shrinkFirstWake(t, 55*time.Second, 25*time.Second, 100*time.Millisecond)
+ a, b, _, _, _ := stagedActivator(t)
+ host := "bot.x.internal"
+ if err := a.WakeWorker(alwaysOnWorkerRoute(host)); err != nil {
+ t.Fatal(err)
+ }
+ var rev atomic.Uint64
+ rev.Store(1)
+ var hooked atomic.Bool
+ b.mu.Lock()
+ if tc.executed {
+ b.workerStartHook = func(registry.Route) {
+ if hooked.CompareAndSwap(false, true) {
+ rev.Store(2) // after the exec, before the install check
+ }
+ }
+ } else {
+ b.admitHook = func() {
+ if hooked.CompareAndSwap(false, true) {
+ rev.Store(2) // during the preparation, before the exec gate
+ }
+ }
+ }
+ b.mu.Unlock()
+ res := a.ReplaceWorker(context.Background(), workerAttempt(CauseRelease, tc.mode, fakeIdentity("v2")), nil,
+ func(string) uint64 { return rev.Load() })
+ if res.Outcome != SwapAbandoned || !strings.Contains(res.Reason, errSwapSuperseded.Error()) {
+ t.Fatalf("ReplaceWorker = %+v, want abandoned: superseded (never a failure of the build)", res)
+ }
+ wantStarts := 1 // the wake
+ if tc.executed {
+ wantStarts++ // the attempt's child, shed
+ }
+ if tc.restores {
+ wantStarts++ // the replaced build, back
+ waitFor(t, "the restore", func() bool { return b.workerStarts() == wantStarts && !a.entryRespawning(host) })
+ if got := a.entryServing(host); got != fakeIdentity("v1") {
+ t.Fatalf("serving %+v, want the build the attempt replaced", got)
+ }
+ return
+ }
+ waitFor(t, "the entry to stop", func() bool { return !a.Owns(host) })
+ time.Sleep(100 * time.Millisecond)
+ if n := b.workerStarts(); n != wantStarts {
+ t.Fatalf("%d worker children, want %d: the replaced build was restored after the hold build ran", n, wantStarts)
+ }
+ for _, c := range b.calls()[wantStarts-1:] {
+ if c != nil && *c == fakeIdentity("v1") {
+ t.Fatalf("spawns %v: the replaced build was respawned after the hold build ran", b.calls())
+ }
+ }
+ })
+ }
+}