fix(topology): exclude unknown-NVLink-domain nodes from segment selection - #2924
Merged
Conversation
terrykong
changed the title
terrykong
added
the
CI:Lfast
Collaborator
Author
|
/ok to test e2b6b2a |
…tion
select_segment_nodes ran over the full topology including nodes with no
NVLink-domain info (e.g. a GPU-less/unprobed head node). Those collapse into a
single NVLINK_DOMAIN_UNKNOWN pseudo-domain with topo_rank -1, so they sort
first and get selected once there are >= segment_size of them — but the
resulting placement-group constraint {"unknown": 0.001} names a Ray resource
ray.sub never registers, so the bundle can never schedule (retry loop / hang).
Skip unknown-domain nodes when building the candidate set so we only ever pin
to real, registered NVLink domains; the skipped nodes fall through to
remaining_node_ids. No-op on a homogeneous all-GPU cluster (no unknown nodes).
Adds regression tests (TestSelectSegmentNodes) covering exclusion of unknown
nodes and the unknown-only -> ResourceInsufficientError case; both fail on the
pre-fix code.
Signed-off-by: Terry Kong <terryk@nvidia.com>
terrykong
force-pushed
the
youngeunk/fix-topology-unknown-domain
branch
from
June 25, 2026 04:05
e2b6b2a to
92c4726
Compare
Collaborator
Author
|
/ok to test 92c4726 |
yuki-97
approved these changes
Jun 26, 2026
ashors1
pushed a commit
that referenced
this pull request
Jun 27, 2026
…tion (#2924) Signed-off-by: Terry Kong <terryk@nvidia.com> Signed-off-by: Anna Shors <ashors@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes a placement bug in
select_segment_nodes(topology-aware NVLink-domain placement, from #2612). Nodes with no NVLink-domain info — e.g. a GPU-less or unprobed Ray head node — collapse into a singleNVLINK_DOMAIN_UNKNOWNpseudo-domain withtopo_rank = -1, so they sort first and get selected once there are >=segment_sizeof them. The resulting placement-group constraint{"unknown": 0.001}names a Ray resource thatray.subnever registers, so the bundle can never schedule (retry loop / hard failure on heterogeneous or partial-probe clusters).The fix skips unknown-domain nodes when building the candidate set, so we only ever pin to real, registered NVLink domains; skipped nodes fall through to
remaining_node_ids. No-op on a homogeneous all-GPU cluster.Testing
Unit tests — added regression tests in
tests/unit/distributed/test_topology_placement.py::TestSelectSegmentNodes:test_unknown_domain_nodes_excluded_from_selectionandtest_unknown_domain_only_raises_instead_of_unschedulable_pg— both fail on the pre-fix code, pass after.e2b6b2a2).Functional
--segmentrun on GB200 — confirms the fix does not regress topology-aware placement on real hardware.grpo-llama3.2-1b-instruct-1n8g-megatron.yaml, 2 nodes × 4 GPU,cluster.segment_size=2, 5 steps:✓ Topology-aware allocation: 2 training nodes in 1 NVLink domains (segment_size=2)---EXIT:0---), with no unschedulable-PG /ResourceInsufficientError.(The unknown-domain case itself cannot occur on a clean homogeneous GB200 rack — it is covered by the unit tests; this functional run verifies no regression to the normal placement path.)
Stack
Bottom of a stack — #2613 (feat: Numa aware binding) stacks on top of this.
Before your PR is "Ready for review"