perf(api): cjson.decode in startTransitionScript Lua script blocks Redis event loop under concurrent evictions

Aperta
#3,602 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
48/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
go, redis
Ambito
backend, databases

Direzione di ricerca

Inizia da packages/api/internal/sandbox/storage/redis/scripts.go e segui startTransitionScript, addSandboxScript e removeSandboxScript attraverso StartRemoving e RemoveOpts.ExpectExecutionID. Esamina come vengono assemblate le chiavi Redis e gli argomenti degli script, quindi verifica che i controlli dell’ID di esecuzione rimangano atomici e che aggiunta, transizione e rimozione mantengano coerente lo stato ausiliario senza una decodifica completa del JSON.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Problem

`startTransitionScript` in `packages/api/internal/sandbox/storage/redis/scripts.go` conditionally calls `cjson.decode` on the full sandbox JSON blob when `ARGV[5]` (the expected `executionID`) is non-empty:

```lua
if ARGV[5] ~= '' then
local current = redis.call('GET', KEYS[1])
local ok, decoded = pcall(cjson.decode, current) -- ← full JSON parse on main thread
if not ok or decoded['executionID'] ~= ARGV[5] then
return 0
end
end
redis.call('SET', KEYS[1], ARGV[1])
redis.call('SET', KEYS[2], ARGV[2], 'EX', ARGV[3])
redis.call('SET', KEYS[3], '', 'EX', ARGV[4])
return 1
```

Redis executes all Lua on its single-threaded event loop. While a script runs, every other client command — including unrelated `GET`, `SET`, `ZADD` — is blocked.

`cjson.decode` is O(N) in the size of the JSON string. A full sandbox record is typically several hundred bytes. Under concurrent evictions (many sandboxes expiring simultaneously, all API Nomad allocations calling `StartRemoving` with a non-empty `ExpectExecutionID`), each Lua invocation serializes JSON parsing on the main thread. Command latency compounds: with 100 concurrent calls each taking ~100 µs of CPU for the decode, that is 10 ms of pure event-loop stall during which no other command makes progress.

This is introduced by `c29ee26` (feat(api): let a removal pin the sandbox incarnation it meant to remove).

Reproduction scenario

  • High sandbox density (400+ sandboxes/node) with concurrent TTL expiration
  • Multiple API Nomad allocations, each running the 50 ms evictor loop
  • All allocations call `StartRemoving` with `RemoveOpts.ExpectExecutionID` set (evictor path)
  • Observed: Redis `latency` spikes, evictor falling behind its own SLA, commands timing out

Root cause

The `executionID` is embedded inside the JSON blob. Validating it atomically requires either JSON-decoding the blob inside Lua or storing the field separately so it can be fetched with a plain `GET`.

The `cjson.decode` path was chosen because `Add` is lockless, making it impossible to guard the check in Go: a resume can install a new incarnation between the Go-side comparison and the Lua write. This reasoning is correct — the fix must preserve that atomicity.

Proposed fix

Store `executionID` in a dedicated auxiliary Redis key alongside the main sandbox key:

```
sandbox key: {team}:sbx:{sandboxID} → full JSON blob (existing)
execution key: {team}:sbx:{sandboxID}:eid → executionID string only (new)
```

Both keys share the same hash tag `{team}`, so they land on the same cluster slot and can be read/written atomically inside a single Lua script.

Updated `startTransitionScript`:

```lua
-- KEYS[4] = execution ID key (new)
-- ARGV[5] = expected executionID, or "" to write unconditionally
-- ARGV[6] = new executionID to write (new)

if ARGV[5] ~= '' then
local eid = redis.call('GET', KEYS[4]) -- O(1) string fetch, no JSON parse
if not eid or eid ~= ARGV[5] then
return 0
end
end
redis.call('SET', KEYS[1], ARGV[1])
redis.call('SET', KEYS[4], ARGV[6]) -- keep eid key in sync
redis.call('SET', KEYS[2], ARGV[2], 'EX', ARGV[3])
redis.call('SET', KEYS[3], '', 'EX', ARGV[4])
return 1
```

`addSandboxScript` also writes the eid key atomically:

```lua
-- KEYS[3] = execution ID key (new)
-- ARGV[3] = executionID (new)
redis.call('SET', KEYS[1], ARGV[1])
redis.call('SET', KEYS[3], ARGV[3]) -- write eid alongside main key
redis.call('SADD', KEYS[2], ARGV[2])
return 1
```

`removeSandboxScript` deletes the eid key together with the sandbox key.

This replaces `cjson.decode` (O(N) CPU on main thread) with `GET` (O(1) hash-table lookup), while preserving the same atomicity guarantee.

Impact

  • No behaviour change: same correctness properties, same Cluster slot constraints
  • Reduces Lua CPU time on the hot path by ~10–100× depending on JSON payload size
  • Introduces one additional Redis key per sandbox (negligible memory overhead)
  • Requires a short migration window for existing deployments: sandboxes created before the deploy lack the eid key; the script should treat a missing eid key as a mismatch when `ARGV[5]` is set (safe: the removal is rejected until the sandbox is next written with the new code path)

Alternatives considered

  • Move the check to Go with a distributed lock — rejected: `Add` is lockless, so the check cannot be made atomic with the write from outside Lua
  • Use Redis Hash instead of String for sandbox data — would allow `HGET key executionID` inside Lua without full decode; viable but requires changing the storage layout everywhere and a data migration
  • Accept the status quo — acceptable at low sandbox counts; becomes a bottleneck at 400+ sandboxes/node with concurrent evictions
Lingua principale
Go
Stelle
1.6k
Fork
438
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di e2b-dev/runtime

Tutte le issue di e2b-dev/runtime

Issue simili

Altre issue su Go

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.