[ROCm][Bugfix] Use TCP store when AITER custom all-reduce is enabled - #51635
Conversation
…eStore Signed-off-by: vllmellm <vllm.ellm@embeddedllm.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
| distributed_init_method = get_file_store_init_method() | ||
| if aiter_requires_tcp_store(): | ||
| distributed_init_method = get_distributed_init_method( | ||
| get_ip(), get_open_port() |
There was a problem hiding this comment.
⚪ Severity: LOW
The TCP store is bound to the machine's external network IP (get_ip()) instead of the loopback address. For a single-process executor no remote workers need to connect, yet this exposes an unauthenticated, unencrypted PyTorch TCPStore on the network. The multiproc_executor.py correctly uses get_loopback_ip() for the same purpose. Per the project's own docs/usage/security.md, PyTorch distributed features accept connections from anywhere without authorization.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Replace get_ip() with get_loopback_ip() on line 81 to bind the TCP store to the loopback address instead of the external network IP, matching the pattern used in multiproc_executor.py. Additionally, update the import statement at line 19: change get_ip to get_loopback_ip in the import from vllm.utils.network_utils.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| get_ip(), get_open_port() | |
| get_loopback_ip(), get_open_port() |
| ``FileStore`` and trips that assertion. Prefer the TCP rendezvous | ||
| (pre-#50999) for ROCm + AITER custom AR until AITER accepts FileStore. | ||
| """ | ||
| import vllm.envs as envs |
There was a problem hiding this comment.
def aiter_requires_tcp_store() -> bool:
from vllm._aiter_ops import rocm_aiter_ops
return rocm_aiter_ops.is_custom_all_reduce_enabled()
Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com>
|
✅ @vllmellm, CI is now available for this PR.
|
|
/ci run |
|
✅ Triggered Buildkite CI #83119 for commit |
|
/ci run |
|
✅ Triggered Buildkite CI #83123 for commit |
|
Filed the upstream ask on the AITER side so this special case can eventually be removed: |
Post-merge validation: DeepSeek-V4-Pro, MI355X TP=8, 8k/1kRan an end-to-end sweep with this change on a DSv4-Pro serving workload, since it exercises the exact path the fix targets — Setup
The nightly did not yet contain this commit, so it was applied on top (clean, 3 files, no rejects). Results
Throughput scales 1.00x / 1.70x / 2.49x against 2x / 4x concurrency, with TPOT degrading gracefully (24.7 → 40.8 ms median) — the expected shape for a decode-bound 8k/1k workload. Startup / stability No Two notes, neither attributable to this change:
Raw |
vLLM vllm-project#51635 restores TCPStore when AITER custom all-reduce is enabled, so the workaround is no longer needed on nightly (validated in spur job 4357). Co-authored-by: Cursor <cursoragent@cursor.com>
vLLM vllm-project#51635 restores TCPStore when AITER custom all-reduce is enabled, so the workaround is no longer needed on nightly (validated in spur job 4357). Co-authored-by: Cursor <cursoragent@cursor.com>
…llm-project#51635) Signed-off-by: vllmellm <vllm.ellm@embeddedllm.com> Signed-off-by: tjtanaa <tunjian.tan@embeddedllm.com> Co-authored-by: tjtanaa <tunjian.tan@embeddedllm.com>
4 participants
Purpose
#50999 switched single-node executors from TCP to
file://rendezvous to eliminate startup port races. On ROCm with AITER custom all-reduce enabled, that broke every server start at worker init:AITER's custom all-reduce asserts the default store is a
TCPStore(aiter/dist/device_communicators/custom_all_reduce.py);file://rendezvous produces aFileStore. AITER hasn't acceptedFileStoreupstream, so until it does, keep the pre-#50999 TCP rendezvous for exactly that configuration.So, as a stopgap, add a check if it's ROCM and AITER custom all_reduce is opted. It will revert the behavior to using the TCP store instead of the FileStore
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.