Skip to content

Fix vLLM server hangs after worker errors - #6628

Closed
DaoyuanLi2816 wants to merge 2 commits into
huggingface:mainfrom
DaoyuanLi2816:agent/propagate-vllm-worker-errors
Closed

DaoyuanLi2816 wants to merge 2 commits into
huggingface:mainfrom
DaoyuanLi2816:agent/propagate-vllm-worker-errors

Conversation

@DaoyuanLi2816

@DaoyuanLi2816 DaoyuanLi2816 commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

A vLLM data-parallel worker currently exits when an engine call raises. The server then waits indefinitely on its pipe, which leaves the HTTP request hanging and can surface as an NCCL timeout on the training ranks.

This change:

  • returns worker-call failures through the existing pipe and raises them promptly in the request path;
  • wraps successful call results in the same response protocol;
  • closes each unused child pipe endpoint in the parent after starting the worker;
  • adds a CPU multiprocessing regression covering the failure path and clean worker shutdown.

No user-facing API changes or new dependencies are introduced.

Fixes #3433

Before submitting

Validation:

  • python -m pytest tests/test_vllm_client_server.py -m "not slow" -q (11 passed, 42 deselected)
  • ruff check trl/scripts/vllm_serve.py tests/test_vllm_client_server.py
  • ruff format --check trl/scripts/vllm_serve.py tests/test_vllm_client_server.py

AI writing disclosure

We welcome the use of AI tools to help with contributions. For transparency and to help us improve our review process, please indicate the level of AI involvement in this PR.

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag members/contributors who may be interested in your PR.


Note

Medium Risk
Touches all DP parent?worker pipe paths for generation and related endpoints; behavior on failure changes from hang to RuntimeError, which is intended but affects production error handling during RL training.

Overview
Fixes indefinite hangs on the vLLM data-parallel serve path when a worker?s engine call fails (e.g. prompt too long) instead of returning a response.

Worker IPC now uses a small protocol: successful call replies are {"result": ...} and failures are {"error": {"type", "message"}} so the worker stays alive. The parent uses _recv_worker_results everywhere it used to block on raw recv() (generate, chat, logprobs dispatch, reset prefix cache), raising RuntimeError with the worker error when any DP rank fails. The parent also closes the child pipe end after spawning each worker.

CPU multiprocessing tests cover error round-trip through _run_llm_worker and that one worker?s error still drains paired results before the next receive.

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

@DaoyuanLi2816
DaoyuanLi2816 marked this pull request as ready for review August 2, 2026 20:55

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8c59574. Configure here.

Comment thread trl/scripts/vllm_serve.py Resolved
@verma8076

Copy link
Copy Markdown
Contributor

This PR's target code was removed by #6765 ("Use vLLM's own server instead of TRL's", merged 2026-08-26). That PR took trl vllm-serve from 1218 to ~130 lines: the custom FastAPI app, its data-parallel worker pool, and the pipe-based IPC protocol (llm_worker, connection.recv(), the generate/chat/reset_prefix_cache endpoints — everything this diff touches) are gone. trl vllm-serve is now a deprecated wrapper that execves vLLM's own vllm serve directly.

Confirmed on current main: trl/scripts/vllm_serve.py is 307 lines, and neither llm_worker nor any connection.recv()-based endpoint exists in it anymore.

The underlying failure mode (issue #3433, a worker error hanging the HTTP request) may still be worth checking against vLLM's own DP server now that TRL delegates to it — but that would be a vLLM-side fix, not a TRL one, so this PR's diff has no file left to apply to. Might be worth closing with a pointer to #6765, and re-filing against vllm-project/vllm if the hang still reproduces there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants