Summary
When StartRemoving atomically writes a state transition to Redis (e.g. Running → Killing), it never calls publishSandboxEvent. The per-allocation in-process sandbox cache (introduced in #3593) therefore never learns about the new state, and every allocation continues to serve the old Running state from memory until the sandbox is eventually deleted and a remove event arrives.
Code Path
Step 1 — Lua script writes new state to Redis, but no event is published
// state_change.go
updated := sbx
updated.State = newState // e.g. Killing
written, err := startTransitionScript.Run(ctx, s.redisClient,
[]string{key, transitionKey, resultKey},
newData, transitionID, ...) // Redis now has State=Killing
// StartRemoving returns here — no publishSandboxEvent call
return updated, false, s.createCallback(...), nil
Step 2 — createCallback publishes only a routing key, not a sandboxEvent
// state_change.go — createCallback
s.publisher.Publish(cbCtx, getTransitionRoutingKey(teamID.String(), sandboxID, transitionID))
// payload: "lock:sandbox:storage:...:transition:<uuid>"
Step 3 — dispatch routes the payload to waiters, never to cache.apply
// subscription_manager.go
func (m *subscriptionManager) dispatch(payload string) {
if isSandboxEvent(payload) { // strings.HasPrefix(payload, "{") → false for routing keys
m.cache.apply(evt) // never reached
return
}
// falls through to waiter fan-out
}
Impact
Between startTransitionScript.Run() and the final Remove() call (which does publish a remove event), every allocation returns State = Running from TeamItems for a sandbox that Redis already has as Killing or Pausing. For transient transitions, restoreToRunning calls Update() which publishes correctly, but the first half of the round-trip remains invisible to the cache.
Concretely:
TeamItems(states: [Running]) returns sandboxes that are mid-removal — callers may act on them incorrectly.
- Metrics and user-facing sandbox list show inflated
Running counts during high-eviction periods.
Fix
In StartRemoving, after startTransitionScript.Run() succeeds, broadcast the updated sandbox:
s.publisher.publishSandboxEvent(ctx, sandboxEvent{
Op: sandboxEventOpUpdate,
Sandbox: &updated,
})
This mirrors exactly what Update() does in operations.go. dispatch() will call cache.apply({update, updated}), replacing the cached entry with the correct state.
The createCallback path does not need a separate publish because:
- For permanent removals (
TransitionExpires), the subsequent Remove() call already publishes a remove event.
- For transient transitions,
restoreToRunning calls Update() which already publishes an update event.
Related
Summary
When
StartRemovingatomically writes a state transition to Redis (e.g.Running → Killing), it never callspublishSandboxEvent. The per-allocation in-process sandbox cache (introduced in #3593) therefore never learns about the new state, and every allocation continues to serve the oldRunningstate from memory until the sandbox is eventually deleted and aremoveevent arrives.Code Path
Step 1 — Lua script writes new state to Redis, but no event is published
Step 2 —
createCallbackpublishes only a routing key, not a sandboxEventStep 3 —
dispatchroutes the payload to waiters, never to cache.applyImpact
Between
startTransitionScript.Run()and the finalRemove()call (which does publish aremoveevent), every allocation returnsState = RunningfromTeamItemsfor a sandbox that Redis already has asKillingorPausing. For transient transitions,restoreToRunningcallsUpdate()which publishes correctly, but the first half of the round-trip remains invisible to the cache.Concretely:
TeamItems(states: [Running])returns sandboxes that are mid-removal — callers may act on them incorrectly.Runningcounts during high-eviction periods.Fix
In
StartRemoving, afterstartTransitionScript.Run()succeeds, broadcast the updated sandbox:This mirrors exactly what
Update()does inoperations.go.dispatch()will callcache.apply({update, updated}), replacing the cached entry with the correct state.The
createCallbackpath does not need a separate publish because:TransitionExpires), the subsequentRemove()call already publishes aremoveevent.restoreToRunningcallsUpdate()which already publishes anupdateevent.Related
packages/api/internal/sandbox/storage/redis/state_change.gopackages/api/internal/sandbox/storage/redis/operations.go(reference: correct publish pattern inUpdate())