Skip to content

feat(opensandbox): add run-scoped cleanup job - #2559

Merged
bxyu-nvidia merged 10 commits into
mainfrom
hemild/feat-opensandbox-cleanup
Aug 26, 2026
Merged

feat(opensandbox): add run-scoped cleanup job#2559
bxyu-nvidia merged 10 commits into
mainfrom
hemild/feat-opensandbox-cleanup

Conversation

@hemildesai

@hemildesai hemildesai commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the benchmark-local, user-wide cleanup helper with a standalone OpenSandbox cleanup CLI that requires an exact run and user scope
  • attach the Slurm run and user attribution to every sandbox created by the evaluation
  • submit one CPU-only afterany cleanup job immediately after the evaluation job, giving cleanup its own allocation and time budget after success, failure, timeout, or cancellation
  • paginate the full sandbox inventory, delete matches concurrently through one bounded connection pool, and repeat the sweep to handle teardown races
  • keep credentials in the existing connection config; the dependent job receives only its path and the exact cleanup scope

Test plan

  • focused cleanup and launcher tests: 37 passed locally, cleanup module at 100% coverage
  • Bash 5 launcher behavior checks for workload failure, TERM cancellation, and server-first exit
  • scoped pre-commit hooks, shell syntax check, and git diff --check
  • live Slurm dependency probe: plain parent cancellation released the afterany CPU job and inherited GPU requests were overridden
  • live cancellation E2E with real attributed rollouts plus controlled isolation fixtures: matching target deleted with HTTP 2xx, same-user wrong-run decoy preserved, cleanup completed successfully, and the final exact-scope audit found zero survivors
  • fixture teardown verified no test fixtures remained

@copy-pr-bot

copy-pr-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@hemildesai

Copy link
Copy Markdown
Contributor Author

Real Slurm cancellation smoke test: done (the unchecked test-plan box)

Two rounds against a live 500-rollout eval, scancel issued with the full sandbox population present. Net: the epilogue works, but trap placement is load-bearing.

Round 1 — trap inside the containerized eval command: FAILS. When the eval runs under ray symmetric-run (this launcher family's topology), Ray's teardown kills its managed children the instant the step is cancelled — the trap never fires, no epilogue output, all ~350 sandboxes orphaned. Any launcher that wraps the eval in Ray (or any supervisor that hard-kills children) will silently lose the epilogue.

Round 2 — trap in the batch shell (host side, no exec so the shell survives): WORKS. scancel with 498 live sandboxes → Slurm TERMs the batch shell → trap fires within the kill grace → CLEANUP-EPILOGUE497 × Deleted … HTTP 204CLEANUP-EPILOGUE-DONE, population 498 → 1 in ~95 s end-to-end (most of that was the workload shutting down; the 32-way concurrent reap itself took seconds).

CLI validation at scale: separately ran the CLI against round 1's 349 orphans — all reaped, zero remaining, correctly scoped to exact run+user.

Two suggestions:

  1. Document (or assert in the script) that the trap must live in a shell Slurm signals directly — not inside ray symmetric-run or similar child-killing supervisors. The batch shell also needs a host python with aiohttp, which is worth a preflight check.
  2. One sandbox out of 498 survived round 2's reap (created mid-wave, still listed at reap time) — likely list-pagination skew while the shutting-down workload's own deletes were mutating the set. A cheap fix: loop list→delete until the list comes back empty instead of a single pass.

🤖 Generated with Claude Code

@hemildesai
hemildesai force-pushed the hemild/feat-opensandbox-cleanup branch 2 times, most recently from 287d018 to cf384cc Compare August 19, 2026 23:12
@hemildesai
hemildesai marked this pull request as ready for review August 25, 2026 00:56
@hemildesai
hemildesai force-pushed the hemild/feat-opensandbox-cleanup branch 2 times, most recently from 1221a73 to 8242ba9 Compare August 25, 2026 21:57
@hemildesai hemildesai changed the title feat(opensandbox): add run-scoped cleanup epilogue feat(opensandbox): add run-scoped cleanup job Aug 25, 2026
hemildesai and others added 10 commits August 25, 2026 15:52
Replace the benchmark-specific shell cleanup with a standalone tool that
requires exact run and user attribution before deleting sandboxes.

Run it from the Slurm batch exit handler so leaked sandboxes are reaped
without changing the evaluation's original exit status.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
A cancellation smoke test at ~500-sandbox scale surfaced two gaps. One
sandbox out of 498 survived a single-pass reap: deletes race the
cancelled workload's own teardown, and a list taken while the set
mutates can skip entries across page boundaries. The reap now re-lists
after each sweep and repeats (bounded) until the list comes back empty
or a sweep stops progressing, which also lets transient delete failures
recover instead of failing the epilogue outright.

The same test showed the trap-based epilogue never fires when the eval
command runs under ray symmetric-run: cancellation tears such shells
down before their traps run. The wiring here is safe because the eval
is its own srun task; codify that requirement in a comment and preflight
the aiohttp dependency at job start instead of discovering it during
cancellation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
Signed-off-by: Hemil Desai <hemild@nvidia.com>
@hemildesai
hemildesai force-pushed the hemild/feat-opensandbox-cleanup branch from e95556a to b5038e2 Compare August 25, 2026 22:52
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Aug 26, 2026
@bxyu-nvidia

Copy link
Copy Markdown
Contributor

/ok to test b5038e2

@bxyu-nvidia
bxyu-nvidia merged commit cae7594 into main Aug 26, 2026
35 checks passed
@bxyu-nvidia
bxyu-nvidia deleted the hemild/feat-opensandbox-cleanup branch August 26, 2026 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sla:triage-overdue Review assignment is over the one-business-day SLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants