Daytona pool self-healing: replace slots whose env server died instead of retrying forever#1609
Conversation
…d of retrying forever
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to detect connection errors and replace broken Daytona slots in the background. The review feedback highlights critical issues with this implementation: first, the newly spawned slot is not added to self._slots and the old one is not removed, leading to resource leaks during teardown; second, background replacement tasks are not cancelled or awaited during teardown, which can also leak containers; and third, the _is_connection_error check should include the exception class name in the string check to ensure exceptions like ConnectionClosedError are correctly matched.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…down _replace now removes the broken slot from _slots and appends the new one, so the replacement sandbox is stopped at teardown instead of leaking. teardown cancels and awaits in-flight _replace tasks before iterating _slots, preventing a replacement from spawning a sandbox after teardown and from mutating _slots mid-iteration.
…ubstrings EnvClient.connect normalizes every handshake failure (refused, timeout, HTTP 502 / rejected websocket) to a builtin ConnectionError, and a mid-stream drop surfaces as websockets ConnectionClosedError. Matching those two types is exact and drops the false-positive risk of the substring check (e.g. any error text containing "502").
Builds on #1487 (targets its branch). Fixes a failure mode found running the tbench2 + Daytona pool adapter in fully-async disaggregated training.
Problem
When in-flight episodes are killed mid-run (e.g. every weight update aborts current generations), their env-server processes inside the Daytona sandboxes can crash on the client disconnect and never recover (subsequent connects get 502 / rejected websocket). The slot goes back into the pool anyway, so every episode that later acquires it fails and retries forever.
Observed at scale: one weight update killed ~30 in-flight episodes, all their sandboxes' env servers 502-crashed, and the rollout worker spun on thousands of doomed retries — the run was permanently stalled while all sandboxes were "alive" from Daytona's point of view.
Fix
Classify connection-level failures (
_is_connection_error) in_with_env's pool branch and, instead of releasing the broken slot back into the queue, replace it in the background: stop the dead sandbox, provision a fresh one through the existing_spawn_onepath (inherits the throttle/backoff handling), and enqueue the replacement. The episode's own exception still propagates unchanged, so episode-level retry/reward semantics are untouched.Notes:
asyncio.create_taskwith a held reference) so neither the failing episode nor other episodes block on the ~30-60s provisioning.Validation
GLM-5.2 744B fully-async run (8 train + 8 inference nodes, 64-episode batches): at a weight-update window one slot's env server died, was detected on next acquire, replaced in background, and the episode stream continued without stalling — vs. the pre-fix run which permanently stalled at the same point.
🤖 Generated with Claude Code