Skip to content

fix(sandbox-cache): broadcast state transition event from StartRemoving - #3596

Open
AdaAibaby wants to merge 2 commits into
e2b-dev:mainfrom
AdaAibaby:fix/sandbox-cache-start-removing-broadcast
Open

fix(sandbox-cache): broadcast state transition event from StartRemoving#3596
AdaAibaby wants to merge 2 commits into
e2b-dev:mainfrom
AdaAibaby:fix/sandbox-cache-start-removing-broadcast

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Summary

StartRemoving atomically writes a new sandbox state to Redis via a Lua script (e.g. Running → Killing), but never called publishSandboxEvent. Every allocation's in-process sandbox cache (introduced in #3593) continued to serve the old Running state until the sandbox was eventually deleted and a remove event arrived.

Symptoms:

  • TeamItems(states: [Running]) returned sandboxes already mid-removal
  • Metrics and user-facing sandbox list showed inflated Running counts during high-eviction periods

Root Cause

Three code locations form the gap:

1. StartRemoving — Lua write succeeds, no event published

// state_change.go
written, err := startTransitionScript.Run(ctx, s.redisClient, ...)
// Redis now has State=Killing — but no publishSandboxEvent call here
return updated, false, s.createCallback(...), nil

2. createCallback — publishes routing key, not a sandboxEvent

s.publisher.Publish(cbCtx, getTransitionRoutingKey(...))
// payload: "lock:sandbox:storage:...:transition:<uuid>"

3. dispatch — routing key never reaches cache.apply

if isSandboxEvent(payload) { // strings.HasPrefix("{") → false for routing keys
    m.cache.apply(evt)        // never reached
}

Fix

After startTransitionScript.Run() succeeds, publish a sandboxEvent{update}:

s.publisher.publishSandboxEvent(ctx, sandboxEvent{Op: sandboxEventOpUpdate, Sandbox: &updated})

dispatch() routes it to cache.apply, so all allocations immediately reflect the intermediate state.

createCallback needs no extra publish:

  • Terminal transitions: Remove() broadcasts a remove event
  • Transient transitions: restoreToRunning calls Update() which broadcasts the restored Running state

Tests

Two new integration tests (real Redis via testcontainers):

  • TestStartRemoving_CacheBroadcastsTransitionState — Kill transition is visible as Killing in cache before Remove is called
  • TestStartRemoving_CacheBroadcastsTransientTransition — Snapshot transition visible as Snapshotting, restored to Running after callback

Related

…ET bandwidth

TeamItems issues SMEMBERS + MGET on every call. With teams of 9 000+
sandboxes and multiple API allocations the read bandwidth scales as
O(allocations × team_size), saturating the API allocation NIC.

This PR introduces a per-allocation in-process cache for sandbox state
backed by the existing pub/sub infrastructure (publisher +
subscriptionManager, introduced in e2b-dev#2099 / e2b-dev#2668). The cache eliminates
the O(allocations × team_size) multiplier: each allocation maintains a
local snapshot and TeamItems reads from memory after the first cold-fetch.

Design
- sandbox_event.go: sandboxEvent JSON type published alongside existing
  plain routing-key strings on globalStorageNotifyChannel. JSON prefix
  '{' is an unambiguous discriminator from routing keys ('sandbox:...',
  'lock:...').
- sandbox_cache.go: sandboxCache keyed by sandbox ID, indexed by team,
  with warm/cold state per team. Thread-safe via sync.RWMutex.
- publisher.go: publishSandboxEvent marshals and enqueues events on the
  existing 32-worker publish pool.
- subscription_manager.go: dispatch detects JSON events and applies them
  to the embedded sandboxCache before routing-key fan-out.
- operations.go: Add/Update/Remove broadcast events after each Redis
  write; TeamItems checks the cache (warm-path) or falls back to
  SMEMBERS+MGET and warms the team on cold-start. Gated by
  SandboxTeamItemsCacheFlag (default false).
- featureflags/flags.go: SandboxTeamItemsCacheFlag for safe rollout.

Tests
- sandbox_cache_test.go: unit tests for apply/evict/warmTeam/getTeam,
  state filtering, team isolation, stale-entry eviction, event
  marshal/unmarshal, routing-key disambiguation.
- team_items_test.go: extended with 9 new integration tests (real Redis
  via testcontainers) covering cold-start warming, empty-team warming,
  event-driven add/remove reflection, end-to-end Add/Remove/Update
  broadcast, team isolation, and flag-off behaviour.

Closes e2b-dev#3593
Before this change, StartRemoving atomically wrote the new sandbox state
(e.g. Running→Killing) to Redis via startTransitionScript, but never
called publishSandboxEvent. Every allocation continued to see the old
Running state in its local TeamItems cache until the sandbox was
eventually deleted and a remove event arrived.

Fix: publish a sandboxEvent{update} immediately after the Lua script
succeeds. dispatch() applies it via cache.apply, so all allocations
reflect the intermediate state (Killing, Pausing, Snapshotting) without
any Redis read.

The createCallback path needs no separate publish:
- Terminal transitions: the subsequent Remove() already broadcasts remove.
- Transient transitions: restoreToRunning calls Update() which already
  broadcasts the restored Running state.

Adds two integration tests:
- TestStartRemoving_CacheBroadcastsTransitionState: Kill transition
  visible as Killing in cache before Remove is called.
- TestStartRemoving_CacheBroadcastsTransientTransition: Snapshot
  transition visible as Snapshotting, then restored to Running.

Fixes e2b-dev#3595
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sandbox cache: StartRemoving state transition not broadcast, all allocations see stale Running state

2 participants