Skip to content

fix(test): cap RL worker GPU memory so test_rl_worker_discovery survives parallel bin-packing - #10602

Merged
nv-tusharma merged 4 commits into
mainfrom
tushar/fix-rl-worker-discovery-gpu-mem-binpack
Jun 11, 2026
Merged

fix(test): cap RL worker GPU memory so test_rl_worker_discovery survives parallel bin-packing#10602
nv-tusharma merged 4 commits into
mainfrom
tushar/fix-rl-worker-discovery-gpu-mem-binpack

Conversation

@nv-tusharma

@nv-tusharma nv-tusharma commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

tests/rl/test_worker_discovery.py::test_rl_worker_discovery_and_engine_admin_routes has been failing 77.9% of the time on main since it was introduced in #9681, blocking post-merge/PR pipelines. Datadog shows 26 failed pipelines and ~265h of CI time lost. It fails with:

RuntimeError: Main server process exited with code 1 while waiting for health check

Root cause

The test launches its vLLM worker through a hardcoded RLVllmWorkerProcess command that omits the GPU-memory budget every other vLLM e2e test injects. The parallel GPU scheduler bin-packs this worker onto a shared GPU and sets _PROFILE_OVERRIDE_VLLM_KV_CACHE_BYTES from the test's requested_vllm_kv_cache_bytes(331_801_000) marker — but the worker never consumed it, so vLLM fell back to its default gpu_memory_utilization=0.9, demanded ~90% of the whole card, and crashed during engine init:

ValueError: Free memory on device cuda:0 (3.51/47.37 GiB) on startup is less than
desired GPU memory utilization (0.92, 43.58 GiB). ...
RuntimeError: Engine core initialization failed.

This crashes the worker (exit 1) only when it's co-located with other workers on the GPU, which is why it passed ~22% of the time (scheduled first/alone). tests/README.md documents honoring _PROFILE_OVERRIDE_VLLM_KV_CACHE_BYTES as a hard requirement for vLLM launch paths.

Fix

Route the worker command through build_gpu_mem_args("build_vllm_gpu_mem_args") — the same helper the standard vLLM workers (tests/mm_router/*, tests/router/test_router_e2e_with_vllm.py, etc.) use. Under parallel runs it emits --kv-cache-memory-bytes 331801000 --gpu-memory-utilization 0.01 (caps the worker to its profiled budget); serially it falls back to --gpu-memory-utilization 0.4.

Verification (local A/B, RTX 6000 Ada, simulated bin-packing: 43 GiB held, ~3.5 GiB free)

  • Unfixed: dies at the vLLM free-memory check — exit_code=1 after 27s, same 503 → process-death pattern as CI.
  • Fixed: passes the memory gate — reserved 0.31 GiB memory for KV Cache ... skipped memory profiling, GPU KV cache size: 2,880 tokens, init engine ... took 0.32 s — engine initializes within its budget and proceeds well past the crash point.

One-file, test-only change that brings this worker in line with the harness's documented GPU-budget contract.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Updated RL worker discovery test to use shared GPU memory configuration helper, improving consistency in GPU memory settings across workers.

@nv-tusharma
nv-tusharma requested review from a team as code owners June 11, 2026 16:03
@github-actions github-actions Bot added the fix label Jun 11, 2026
test_rl_worker_discovery_and_engine_admin_routes launches its vLLM worker via a
hardcoded command that omitted the GPU-memory budget every other vLLM e2e test
injects. Under the parallel GPU scheduler the worker is bin-packed onto a shared
GPU but ignored its _PROFILE_OVERRIDE_VLLM_KV_CACHE_BYTES budget, so vLLM fell
back to gpu_memory_utilization=0.9, demanded ~90% of the whole card, and crashed
during engine init ("Free memory ... less than desired GPU memory utilization")
with exit code 1 — surfacing as "Main server process exited with code 1 while
waiting for health check". 77.9% failure rate on main since #9681.

Route the worker command through build_gpu_mem_args("build_vllm_gpu_mem_args")
like the standard vLLM workers, so it honors the scheduler's
--kv-cache-memory-bytes budget under parallel runs and uses a conservative
--gpu-memory-utilization 0.4 fallback when run serially. The test already
declares requested_vllm_kv_cache_bytes(331_801_000); this makes the worker
actually respect it (tests/README.md documents this as a hard requirement).

Reproduced locally under simulated bin-packing (43 GiB held, ~3.5 GiB free):
the unfixed worker dies at the vLLM free-memory check (exit 1); with the fix the
engine initializes within its 0.31 GiB KV budget and proceeds past the memory gate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nv-tusharma
nv-tusharma force-pushed the tushar/fix-rl-worker-discovery-gpu-mem-binpack branch from 2f4d2ca to d68d60d Compare June 11, 2026 16:05
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2a336bfc-be9d-43c2-9203-fd5f289d65f3

📥 Commits

Reviewing files that changed from the base of the PR and between 78dab60 and 2f4d2ca.

📒 Files selected for processing (1)
  • tests/rl/test_worker_discovery.py

Walkthrough

The PR refactors vLLM GPU memory configuration in the RL worker discovery test by extracting a reusable helper function. It imports a shared build_gpu_mem_args utility, creates _vllm_gpu_mem_args() to conditionally derive GPU memory CLI arguments, and applies the helper to the worker process launch command.

Changes

vLLM GPU Memory Configuration

Layer / File(s) Summary
GPU memory settings helper and worker integration
tests/rl/test_worker_discovery.py
Imports build_gpu_mem_args, introduces _vllm_gpu_mem_args(default_utilization) to build GPU memory CLI arguments from a shared budget utility or fall back to a utilization percentage, and updates the vLLM worker process launch command to expand those arguments into the subprocess call.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description provides comprehensive Problem, Root cause, Fix, and Verification sections. However, the Related Issues section is missing—the description does not indicate whether this PR is linked to or closes an issue. Add a Related Issues section specifying whether this PR closes #9681 or another issue, or explicitly confirm no related issue exists.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: capping RL worker GPU memory to fix a failing test in parallel bin-packing scenarios.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Temporarily switch post_merge -> pre_merge so this PR's pre-merge vLLM GPU job
runs the test under real parallel bin-packing (gpu_parallel, max 24 GiB) and
validates the GPU-memory-budget fix end-to-end. Revert to post_merge before merge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…PU-mem fix"

Validation complete: the test PASSED [78s] in this PR's pre-merge vLLM GPU job
on GPU0 under real parallel bin-packing. Restore post_merge gating so the heavy
900s e2e stays out of the per-PR critical path. Leaves the clean GPU-mem fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@biswapanda biswapanda 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.

lgtm - thanks @nv-tusharma !

@nv-tusharma
nv-tusharma merged commit 9b7a98a into main Jun 11, 2026
87 of 88 checks passed
@nv-tusharma
nv-tusharma deleted the tushar/fix-rl-worker-discovery-gpu-mem-binpack branch June 11, 2026 19:24
yao531441 pushed a commit to yao531441/dynamo that referenced this pull request Jun 24, 2026
…ves parallel bin-packing (ai-dynamo#10602)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Yao, Qing <qing.yao@intel.com>
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.

2 participants