amadan
activator: RetireExcept re-validates a probe's open under its final lock (Plan 3 Task 7 fix round 1, finding 2)
A probe entry opened after the held-probe snapshot was taken is kept for this reload: its registry observation is older than the open. A seam between the snapshot and the registry read makes the interleaving deterministic in the test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+52
−5
internal/activator/activator.go+6 −1internal/activator/probewake.go+13 −4internal/activator/probewake_test.go+33 −0
diff --git a/internal/activator/activator.go b/internal/activator/activator.go| index 8439c71cf..d1fac2429 100644 |
| --- a/internal/activator/activator.go |
| +++ b/internal/activator/activator.go |
| @@ -2956,7 +2956,7 @@ func (a *Service) sweepOnce() { |
| func (a *Service) RetireExcept(keep map[string]bool) { |
| // A probe's entry (ProbeWake) is held for its move while the registry |
| // row is: the edge manages ready rows only, so it is never in keep. |
| - seen, held := a.heldProbes(keep) |
| + seen, held, openedAt := a.heldProbes(keep) |
| if retireBetweenLocks != nil { |
| retireBetweenLocks() |
| } |
| @@ -2971,6 +2971,11 @@ func (a *Service) RetireExcept(keep map[string]bool) { |
| if keep[e.host] || held[e] || (e.probe != nil && !seen[e]) { |
| continue |
| } |
| + if e.probe != nil && e.probe.opened && !openedAt[e] { |
| + // Opened after the snapshot: the registry read is older than the |
| + // open, so it cannot judge the entry. Kept for this reload. |
| + continue |
| + } |
| // Workers are outside this reconcile's world entirely. `keep` is the |
| // EDGE's managed set, built by edge.Reload — which refuses a sidecar |
| // row on kind as the first act of its loop, so a worker's host can |
diff --git a/internal/activator/probewake.go b/internal/activator/probewake.go| index 80448dc70..a36690069 100644 |
| --- a/internal/activator/probewake.go |
| +++ b/internal/activator/probewake.go |
| @@ -96,7 +96,7 @@ func probeResultOf(o *hostclaim.Observed) ProbeResult { |
| // Test seams: retireBetweenLocks runs in RetireExcept between the probe |
| // snapshot and the retirement pass; probeWakePublished runs in ProbeWake |
| // after the entry is published and before its wake starts. |
| -var retireBetweenLocks, probeWakePublished func() |
| +var retireBetweenLocks, probeWakePublished, retireBeforeRegistryRead func() |
| // probeEntry marks an actEntry ProbeWake made. |
| type probeEntry struct { |
| @@ -204,7 +204,13 @@ func probeRefused(e *actEntry, err error) error { |
| // a.mu, so RetireExcept retires a probe only when this snapshot covered it |
| // (fix round 1, finding 6): one published after it is left for the next |
| // reload. A row that cannot be read keeps its entry for this reload. |
| -func (a *Service) heldProbes(keep map[string]bool) (seen, held map[*actEntry]bool) { |
| +// |
| +// openedAt is the open state each snapshotted entry had when the snapshot |
| +// was taken. An entry opened since then was opened after its registry |
| +// observation could have been made, so that observation is stale for it: |
| +// RetireExcept re-validates the open under its final lock and keeps the |
| +// entry for this reload (fix round 1, finding 2). |
| +func (a *Service) heldProbes(keep map[string]bool) (seen, held, openedAt map[*actEntry]bool) { |
| a.mu.Lock() |
| var probes []*actEntry |
| opened := map[*actEntry]bool{} |
| @@ -219,8 +225,11 @@ func (a *Service) heldProbes(keep map[string]bool) (seen, held map[*actEntry]boo |
| for _, e := range probes { |
| seen[e] = true |
| } |
| + if retireBeforeRegistryRead != nil { |
| + retireBeforeRegistryRead() |
| + } |
| if a.Routes == nil { |
| - return seen, held |
| + return seen, held, opened |
| } |
| for _, e := range probes { |
| rt, err := a.Routes.Get(e.host) |
| @@ -237,5 +246,5 @@ func (a *Service) heldProbes(keep map[string]bool) (seen, held map[*actEntry]boo |
| held[e] = true |
| } |
| } |
| - return seen, held |
| + return seen, held, opened |
| } |
diff --git a/internal/activator/probewake_test.go b/internal/activator/probewake_test.go| index d3192602a..34e44b6fc 100644 |
| --- a/internal/activator/probewake_test.go |
| +++ b/internal/activator/probewake_test.go |
| @@ -491,3 +491,36 @@ func TestIdleSweepSparesAHeldProbe(t *testing.T) { |
| a.sweepOnce() |
| waitFor(t, "the opened probe to idle out", func() bool { return !a.Owns("one.example.com") }) |
| } |
| + |
| +// Task 7 fix round 1, finding 2: the open lands between RetireExcept's probe |
| +// snapshot (not opened) and its registry read (ready, unheld). The |
| +// registry's answer is older than the entry's open, so the entry is kept, |
| +// not retired as a probe that never opened. |
| +func TestRetireExceptRevalidatesTheOpenUnderTheFinalLock(t *testing.T) { |
| + a, _, _, reg := testActivator(t, time.Hour) |
| + a.Routes = reg |
| + if err := reg.Upsert(testRoute("one.example.com")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.SetHold("one.example.com", "landing", pwOp); err != nil { |
| + t.Fatal(err) |
| + } |
| + e := pwHeldEntry(t, a) |
| + retireBeforeRegistryRead = func() { |
| + if !a.OpenProbeEntry("one.example.com", pwOp) { |
| + t.Error("not marked") |
| + } |
| + if ok, err := reg.ReleaseHold("one.example.com", pwOp); err != nil || !ok { |
| + t.Errorf("release: %v %v", ok, err) |
| + } |
| + } |
| + t.Cleanup(func() { retireBeforeRegistryRead = nil }) |
| + a.RetireExcept(map[string]bool{}) |
| + retireBeforeRegistryRead = nil |
| + a.mu.Lock() |
| + state := e.state |
| + a.mu.Unlock() |
| + if state != actAwake || !a.Owns("one.example.com") { |
| + t.Fatal("the probe opened during the retirement was retired") |
| + } |
| +} |