fix(gpu): reuse the AFD world's store for subgroup rendezvous - #328
Conversation
Signed-off-by: swjeong9 <swjeong25@gmail.com>
hsliuustc0106
left a comment
There was a problem hiding this comment.
Reviewed at ea4b6dc. No blocking findings from my review — this closes the #327 root cause with proportionate evidence.
Verified against the pinned stack (vLLM 0.26.0 / torch 2.11.0):
StatelessProcessGroupis a dataclass in v0.26.0 andcreate()only builds the TCPStore before constructing it, so constructing the subgroup group over a provided store is behaviorally equivalent for the pin.ProcessGroup.get_group_store()exists on torch 2.11.0 (vLLM 0.26.0's pin). It is absent on older torch, so theTARGET_VLLM_VERSIONgate is what makes the call safe — on an unsupported stack it fails loudly rather than silently.- Per-subgroup
PrefixStoreprefixes keep the PyNccl unique-id bootstrap keys disjoint; the new unit test proves the separation through a real store, and follows the CPU-safe import convention.
The 3-host EKS A/B (placement C fails on the base image, passes here) plus the EFA repeat covers this regression better than the single-node gate would. Stating the GPUDirect RDMA path as untested rather than approximating it is the right call.
Non-blocking notes:
- No stale
port + subgroup_indexreferences remain — guide, recipes, and tests are all consistent with the new rendezvous scheme. - At review time only DCO had reported; as a fork PR this likely will not get the standard single-node GPU E2E gate. The EKS evidence substitutes well for this fix, but a same-repo
test-readyrun post-approval wouldn't hurt. - Follow-up worth an issue: the GPUDirect RDMA rendezvous hang path from the #327 design discussion is still unexercised. Tracking it separately keeps this PR mergeable without waiting on hardware with GPUDirect support.
specture724
left a comment
There was a problem hiding this comment.
Thank @swjeong9 's work. I only have some nits
| store=PrefixStore( | ||
| f"afd_subgroup_{self.mapping.subgroup_index}", | ||
| afd_pg.get_group_store(), | ||
| ), |
There was a problem hiding this comment.
explicitly set timeout here, since the previous behavior is 300s timeout. After the modification, the timeout is set to120s by default ( init_afd_process_group set the store timeout to store.set_timeout(timedelta(minutes=2)))
Please add:
subgroup_store.set_timeout(timedelta(seconds=300))There was a problem hiding this comment.
Thank you for pointing this out — applied.
|
|
||
| monkeypatch.setattr(module, "init_afd_process_group", lambda **kwargs: afd_pg) | ||
| monkeypatch.setattr(module, "_get_default_group", lambda: None) | ||
| monkeypatch.setattr(module, "DefaultProcessGroupSwitcher", _NullSwitcher) |
There was a problem hiding this comment.
monkeypatch.setattr(module, "DefaultProcessGroupSwitcher", lambda *a, **k: nullcontext())and remove _NullSwitcher
There was a problem hiding this comment.
Thank you for the better approach — applied as suggested.
There was a problem hiding this comment.
Modify this line since the multi-node run is verified
Signed-off-by: swjeong9 <swjeong25@gmail.com>
|
Thanks, LGTM |
|
Fix precommit |
Signed-off-by: swjeong9 <swjeong25@gmail.com>
|
Fixed. Pre-commit passes locally. |
|
@jiangkuaixue123 Thank you for re-running the pipeline. Unfortunately, it looks like the Buildkite E2E job hit a runner disk issue: |
Purpose
When the FFN ranks span multiple hosts, FFN worker initialization fails with
OSError: [Errno 99] Cannot assign requested address(#327). Each subgroup brings up a store server of its own and takesafd_config.hostas its bind address, so any FFN rank other than F0 attempts to bind an address that is not local to its host.Following the design @specture724 outlined, a subgroup no longer brings up a store of its own; it reuses the store the AFD world has already rendezvoused on.
Issue
Scope
In scope
Out of scope
p2pcontrol-plane groupImplementation Notes
The subgroup store is the AFD world process group's store, obtained through
ProcessGroup.get_group_store()and wrapped in a per-subgroupPrefixStore, so no additional rendezvous endpoint is created.Test Plan
2A2Finitialization on EKS. One g6.12xlarge (4x L4) and two g6.xlarge, running placement B (Attention split) and placement C (FFN split) against the images built before and after the change.2A2Fstartup and serving over EFA. Four g6.8xlarge with one rank each, followed by 256 requests.Test Result
pytest tests/unit: 830 passed, 2 failed, 62 skipped. Both failures originate intest_e2e_process_utils.py, which requires the Linux-onlyos.pidfd_open; that attribute is unavailable on the macOS machine used for local CPU testing.2A2Fon EKS across three hosts, run against the image built before the change and an image carrying this PR.Before the change, FFN rank 1 terminates with
OSError: [Errno 99] Cannot assign requested address; afterwards, initialization completes correctly.The verification above was carried out over TCP (ENA). We repeated the check on EFA, with
2A2Fon fourg6.8xlarge. Under the same condition as placement C, initialization succeeded on EFA as well, and serving proceeded correctly undervllm bench. The log below identifies the network interface that was selected.Docs Impact
docs/gpu/NCCL_P2P_CONNECTOR_USER_GUIDE.md, together with the module and class docstrings inafd_plugin/connectors/gpu/p2p.py. The wording that required a derived port (port + subgroup_index + 1) has been removed.