Skip to content

[fix][ci] Lower gpu_memory_utilization for colocated tinker E2E; dump infra log on failure - #2051

Merged
avigyabb merged 1 commit into
NovaSky-AI:mainfrom
avigyabb:fix/tinker-e2e-colocated-oom
Aug 18, 2026
Merged

[fix][ci] Lower gpu_memory_utilization for colocated tinker E2E; dump infra log on failure#2051
avigyabb merged 1 commit into
NovaSky-AI:mainfrom
avigyabb:fix/tinker-e2e-colocated-oom

Conversation

@avigyabb

@avigyabb avigyabb commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Fix colocated tinker E2E nightly: vLLM startup OOM on L4

The SkyRL-GPU-E2E-CI-Tinker nightly has failed on every run since 2026-08-14 (the first nightly after the vLLM 0.26.0 bump in #1854) with:

RuntimeError: Engine core initialization failed. See root cause above. Failed core proc(s): {}

raised from VLLMServerActor.start() at the first save_weights_for_sampler. (Failures on 08-08..08-13 were the separate unpinned tinker SDK break, fixed by #2022.)

Root cause

Reproduced on a 1x L4 workspace with a scaled-down (1-GPU) version of the CI backend config. The real error never reaches the job log (see "Log visibility" below); it is a CUDA OOM during vLLM engine startup, at the flashinfer sampler warmup that runs after CUDA graph capture:

torch.OutOfMemoryError: CUDA out of memory. Tried to allocate 594.00 MiB.
GPU 0 has a total capacity of 22.03 GiB of which 582.00 MiB is free.
Process 29529 has 796.00 MiB memory in use. ... this process has 20.67 GiB in use.

Two factors combine:

  1. Engine startup ordering. The main trainer entrypoint starts inference engines on empty GPUs before build_models(). The tinker SkyRLTrainBackend does the reverse: FSDP workers initialize first, and even after offload_to_cpu() each leaves ~800 MiB of unreclaimable CUDA context on the GPU that vLLM later profiles on. gpu_memory_utilization budgets a fraction of total (not free) memory, so vLLM still sizes its KV cache as if it had the whole card.
  2. vLLM 0.26.0's larger startup peak. FULL_AND_PIECEWISE cudagraph capture plus the flashinfer sampler warmup transiently allocate ~3 GiB beyond the utilization budget. At 0.8 on a 22 GiB L4 with the FSDP context resident, this misses by ~12 MiB.

This explains why only this nightly regressed: the fully-async tinker E2E is non-colocated (engines get empty GPUs), and the main colocated E2E starts engines before building models.

Changes

  • gsm8k_tinker.sh: gpu_memory_utilization 0.8 → 0.7. Costs ~2 GiB of KV cache, irrelevant for 512-token GSM8K rollouts.
  • Both tinker E2E scripts: on any non-zero exit, the cleanup trap now dumps the tails of server.log and the newest /tmp/skyrl-logs/infra-*.log.

Log visibility

VLLMServerActor calls redirect_actor_output_to_file(), which sends actor output — including the vLLM engine's real traceback — to /tmp/skyrl-logs/infra-*.log on the cluster. The scripts previously only tailed server.log, and only when the server failed to boot, so a client-visible failure like this one showed nothing but the opaque re-raised RayTaskError (the empty Failed core proc(s): {} is a vLLM race where the engine-core proc dies before its exit code is captured).

Verification

On a 1x L4 with the 1-GPU equivalent of the CI backend config (colocate_all=true, 1 engine):

  • at gpu_memory_utilization=0.8, save_weights_and_get_sampling_client() fails with the exact CI error (OOM in flashinfer sampler warmup);
  • at 0.7, the same call succeeds: engine starts, weights sync, KV cache wakes at 18.5/22 GiB.

Note

Any tinker + colocate_all user on ~24 GB GPUs will hit this same OOM at the default 0.8 utilization, since the lazy engine startup ordering is inherent to the backend. A follow-up could account for the training workers' CUDA context when sizing the KV budget, or start engines eagerly once the LoRA config is known.

🤖 Generated with Claude Code


Note

Low Risk
CI-only shell script changes; no production trainer or auth paths touched.

Overview
Fixes the colocated tinker GSM8K nightly (gsm8k_tinker.sh) by lowering vLLM gpu_memory_utilization from 0.8 to 0.7, with comments explaining lazy engine startup after FSDP leaves CUDA context and vLLM 0.26’s larger startup peak OOMing 22 GiB L4s. The fully-async script keeps 0.8 (non-colocated engines).

Both tinker E2E scripts replace the simple EXIT trap with a cleanup() that on any non-zero exit tails server.log and the newest /tmp/skyrl-logs/infra-*.log (Ray/vLLM errors) before tearing down the server process group—so CI logs show real OOM/tracebacks instead of opaque RayTaskError.

Reviewed by Cursor Bugbot for commit 77ff96c. Bugbot is set up for automated code reviews on this repo. Configure here.

… infra log on failure

The colocated tinker nightly has OOMed at vLLM engine startup on every run
since the vLLM 0.26.0 bump (NovaSky-AI#1854). Unlike the main trainer entrypoint,
where engines profile on empty GPUs before models are built, the tinker
backend starts engines lazily at the first save_weights_for_sampler, after
the FSDP workers have left ~800MiB of unreclaimable CUDA context per GPU.
vLLM 0.26's startup peak (cudagraph capture + flashinfer sampler warmup) on
top of a 0.8 KV budget then exceeds a 22GiB L4 by ~12MiB. Lower the budget
to 0.7 for the colocated script.

Also dump the newest /tmp/skyrl-logs/infra-*.log tail on any failure in
both tinker E2E scripts: Ray actor output (including the vLLM engine's real
traceback) is redirected there and never reaches server.log, so the job log
only showed the opaque "Failed core proc(s): {}" re-raise.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Avi Basnet <avigyabb@stanford.edu>
@avigyabb

Copy link
Copy Markdown
Collaborator Author

Verified on real CI: dispatched SkyRL-GPU-E2E-CI-Tinker on this branch — https://github.com/NovaSky-AI/SkyRL/actions/runs/32075396192 (attempt 2, success). Engines start cleanly at 0.7 and the full 14-step convergence run + wandb reward assertion pass. (Attempt 1 was cancelled after an Anyscale capacity failure, unrelated to the change.)

@avigyabb
avigyabb marked this pull request as ready for review August 18, 2026 01:09

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request lowers the GPU memory utilization in the tinker backend configuration to prevent OOM errors and adds a detailed cleanup function to dump server and infrastructure logs upon failure. The review feedback recommends wrapping the process termination commands in a check to ensure SERVER_PID is defined, preventing potential syntax errors or accidental termination of the entire process group if the server fails to start.

Comment on lines +46 to +48
kill -TERM -- -$SERVER_PID 2>/dev/null || true
sleep 5
kill -KILL -- -$SERVER_PID 2>/dev/null || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the script exits or fails before SERVER_PID is initialized (for example, if setsid or uv run fails to start), SERVER_PID will be empty. In that case, executing kill -TERM -- - or kill -KILL -- - can result in syntax errors or potentially attempt to kill the entire current process group. Wrapping the kill commands in a check to ensure SERVER_PID is set and non-empty prevents this issue.

Suggested change
kill -TERM -- -$SERVER_PID 2>/dev/null || true
sleep 5
kill -KILL -- -$SERVER_PID 2>/dev/null || true
if [ -n "${SERVER_PID:-}" ]; then
kill -TERM -- -$SERVER_PID 2>/dev/null || true
sleep 5
kill -KILL -- -$SERVER_PID 2>/dev/null || true
fi

Comment on lines +44 to +46
kill -TERM -- -$SERVER_PID 2>/dev/null || true
sleep 5
kill -KILL -- -$SERVER_PID 2>/dev/null || true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the script exits or fails before SERVER_PID is initialized (for example, if setsid or uv run fails to start), SERVER_PID will be empty. In that case, executing kill -TERM -- - or kill -KILL -- - can result in syntax errors or potentially attempt to kill the entire current process group. Wrapping the kill commands in a check to ensure SERVER_PID is set and non-empty prevents this issue.

Suggested change
kill -TERM -- -$SERVER_PID 2>/dev/null || true
sleep 5
kill -KILL -- -$SERVER_PID 2>/dev/null || true
if [ -n "${SERVER_PID:-}" ]; then
kill -TERM -- -$SERVER_PID 2>/dev/null || true
sleep 5
kill -KILL -- -$SERVER_PID 2>/dev/null || true
fi

@avigyabb
avigyabb merged commit 02b4319 into NovaSky-AI:main Aug 18, 2026
6 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants