fix(vllm): apply the TCPStore port offset on the branch vLLM actually takes

[terrykong]

What does this PR do ?

Fixes RL-1104: on vLLM 0.25.1, any engine whose model-parallel size spans more than one node dies at startup with EADDRINUSE on the engine's VLLM_PORT. This targets terryk/bump-vllm-0.25.1 so the fix can be reviewed against the bump.

Why the patch on this branch is currently inert

ce28ac042 added the right offset but put it in a branch vLLM never takes:

if local_dp_rank is None:      # <- NeMo-RL's fix went in here
    return get_open_port()
# Offset past the DP master port reserved range, one window per rank.
window = 32
start_port = master_port + 100 + local_dp_rank * window
try:
    return _get_open_port(start_port=start_port, max_attempts=window)
except RuntimeError:
    return get_open_port()     # <- what actually runs

ParallelConfig.__post_init__ takes its "offline SPMD" path whenever vLLM's own DP coordinator is not in use — always, here — and assigns data_parallel_rank_local = envs.VLLM_DP_RANK_LOCAL and data_parallel_master_port = envs.VLLM_DP_MASTER_PORT. Both default to 0. So a plain non-DP engine arrives with local_dp_rank=0, not None:

1. the None branch is dead — the patch never executes;
2. the DP branch searches from 0 + 100 + 0*32 = port 100, fails all 32 attempts on the privileged range, and
3. falls through to get_open_port() → straight back to VLLM_PORT.

That is the port the broadcast MessageQueue binds and holds a few lines later, so rank 0's TCPStore gets EADDRINUSE.

Instrumented on hardware:

ENTRY pid=2797290 local_dp_rank=0 master_port=0 VLLM_PORT=7000

The Port 100 is already in use, trying port 101 ... lines present in every vLLM worker log are the fingerprint of step 2.

The fix

Run the VLLM_PORT-anchored search before the local_dp_rank test, so it applies on whichever branch vLLM would otherwise take, and fall through to vLLM's own logic when VLLM_PORT is unset or the window is full. Ports stay inside the engine's reserved 100-port band and therefore below the OS ephemeral floor — dropping VLLM_PORT instead would reintroduce the TOCTOU contention that band exists to prevent (#2380, #3103).

Confirmation

Reproduced and fixed on 2 nodes x 1 GPU, generation TP=2 — the cheapest shape that puts one engine across two nodes (previously the smallest repro in the repo was 64 GPUs):

vLLM 0.25.1 state	2-node hardware (H100)	CPU-only repro
pristine	EADDRINUSE port 7000 in RayWorkerProc.initialize_worker()	EADDRINUSE 7000
ce28ac042 (this branch today)	EADDRINUSE, unchanged	EADDRINUSE 7000
this PR	distributed_init_method=tcp://<rank0>:7032, engine starts and generates	TCPStore 7032, no collision

The hardware signature matches the DeepSeek-V3 64-GPU CI failure exactly.

Blast radius

17 recipes across performance*.txt and release*.txt have tp*pp > gpus_per_node and should be assumed broken at engine startup on 0.25 without this. Zero nightly tests do, which is why the bump's nightly parity comparison is unaffected.

Changelog

• Move the RayExecutorV2._select_tcpstore_port offset ahead of the local_dp_rank test so it runs on the branch vLLM actually takes.
• Add tests/unit/models/generation/test_vllm_tcpstore_port.py: a GPU-free, single-node regression test that pins the port arithmetic. It fails against ce28ac042 and passes against this PR, and guards the anchor snippet against upstream drift.

Usage

N/A — no user-facing API change.

Before your PR is "Ready for review"

• Make sure you read and followed Contributor guidelines
• Did you write any new necessary tests?
• Did you run the unit tests and functional tests locally? (new unit test run locally: 6 passed in 6s; 2-node functional reproduction run by hand — see table above)
• Did you add or update any necessary documentation? (patch docstring explains the dead-branch trap)

Additional Information

• Still open (tracked in RL-1104): wiring the 2-node reproduction in as a permanent functional test. The unit test above covers the port arithmetic at zero cost, but there is still no CI recipe with tp*pp > gpus_per_node, so the node-spanning engine-startup path itself remains uncovered. The 2-node driver script used here is attached to RL-1104.
• Attempt 2 on this branch's earlier history (1d634dfb3, "use ephemeral vLLM ports for engines that span nodes") was also a no-op: its guard was model_parallel_size > len(bundle_indices), and bundle_indices is the engine's local_bundle_indices, so len(...) == model_parallel_size and the condition is always false. That explains the pre-patch H100 CI datapoint independently.

🤖 Generated with Claude Code

[copy-pr-bot]

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.

[terrykong]

Closing as superseded — the fix is already on terryk/bump-vllm-0.25.1 and so is part of #3280.

Its content was applied to the branch directly rather than by merging this PR, so GitHub never auto-closed it (unlike #3355, which did). The branch carries the start_port=envs.VLLM_PORT + 32 offset plus its regression suite, now rebased onto main.

Two follow-ups landed on top of what was reviewed here, both worth knowing about:

• The pristine_source fixture in test_vllm_tcpstore_port.py was copying an already-patched vLLM. _apply_vllm_patches rewrites site-packages in place, so once any earlier test in the vLLM lane builds a generation worker, the "unpatched" fixture is patched. It now reverses the patch, with the snippets read from patches.py via ast so they cannot drift.
• test_unpatched_vllm_hands_the_tcpstore_the_messagequeue_port asserted the unpatched port equals VLLM_PORT, which only holds where privileged ports cannot be bound. CI runs as root, where unpatched vLLM instead returns master_port + 100 + local_dp_rank * 32 = 100. Both outcomes are broken, so the test now asserts the invariant the patch restores — a port inside the engine's reserved band that the MessageQueue will not also take — rather than either environment's number.

Tracking issue RL-1104 stays as the record.