Repository navigation
[release/2.14][c10d][nccl2] Fix ROCm timeout-test hangs by splitting the subgroup from the default group - #3946
Open
albmalamd wants to merge 1 commit into
Conversation
…the subgroup from the default group On release/2.14, test_wait_times_out_and_process_survives and test_timeout_without_process_abort hang on 8x MI300X (ROCm 10.2) until CI kills them after 30 minutes, three times each. In ROCm/rockrel run 41069793579, test_c10d_nccl2 took 181 minutes and pushed the distributed shard past its 6 h limit. Root cause: the tests time out an all_reduce on a throwaway subgroup and then abort it. release/2.14 initializes nccl2 lazily, so the default group has no bound device, and new_group creates the subgroup as a standalone communicator instead of splitting the default one. For a standalone single-node 8-rank communicator, RCCL in ROCm 10.2 runs small all_reduces with its DDA-IPC kernel, which never polls abortFlag. ncclCommAbort then never returns: commReclaim joins the RCCL proxy thread, which is blocked in hipDeviceSynchronize behind that kernel. Split communicators never get DDA resources, so their kernels exit on abort. release/2.15 passes because nccl2 binds the default group eagerly there (pytorch#192895), so the subgroup is split. Forcing a standalone subgroup on the 2.15 wheel (lazy_init=True) hangs the same way. Fix: bind the default group to a device in _ProcessGroupNCCL2SubgroupTest._init_pg with init_process_group(device_id=...), as ProcessGroupNCCL2EagerNewGroupTest already does. _new_subgroup then splits it, as on release/2.15. The tests still cover the same nccl2 timeout, error and abort paths. The RCCL defect remains for standalone 8-rank communicators until ROCm/rocm-systems#11245 (merged to develop, not in ROCm 10.2) ships. Alternatives considered: skipping the tests on ROCm, as #3700 did for test_timeout_with_communicator_cleanup, drops their coverage. Backporting eager nccl2 initialization (pytorch#192854, pytorch#192855, pytorch#192895, pytorch#193237) applies cleanly, but it is a feature backport with C++ changes that makes every nccl2 group create its communicator eagerly. Test Plan: 8x MI300X, ROCm 10.2.0rc1 wheels (RCCL 2.32.3), torch 2.14.0+rocm10.2.0rc1 (386e979, the CI wheel): ``` python -m pytest test/distributed/test_c10d_nccl2.py -v -k "test_wait_times_out_and_process_survives or test_timeout_without_process_abort" ``` Before: 4/4 runs hang, with rank 0 in ncclCommAbort and GPU 0 at 100%. After: 12/12 pass, about 82 s for the pair. ``` python -m pytest test/distributed/test_c10d_nccl2.py -v -k "not ProcessGroupNCCLLegacyNonblockingTest" ``` After: 42 passed, 3 skipped (238 s). ProcessGroupNCCLLegacyNonblockingTest is deselected because of a separate known race (pytorch#203369). ``` lintrunner -a ``` This PR was authored with the help of an AI assistant. Co-authored-by: Cursor <cursoragent@cursor.com>
This branch has not been deployed
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
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.
Linked issue or supporting maintainer
Supported by @pragupta
BC-breaking?
No
Summary (human written only)
This change fixes hangs of the tests test_wait_times_out_and_process_survives and test_timeout_without_process_abort in test_c10d_nccl2.py on release/2.14 with ROCm 10.2. The hang comes from RCCL, and 2.14 hits it because of nccl2's lazy initialization: new_group creates the subgroup as a standalone 8-rank communicator, and for such a communicator RCCL in ROCm 10.2 runs small all_reduces with its DDA kernel, which doesn't check abortFlag. As a result, after the deliberate timeout ncclCommAbort never returns, because RCCL's proxy thread waits in hipDeviceSynchronize for that kernel to finish. The change in _ProcessGroupNCCL2SubgroupTest._init_pg binds the default group to a device (init_process_group(device_id=...)), so the subgroup is created with ncclCommSplit, as on 2.15, and RCCL doesn't use DDA for split communicators, so the abort works. This works around the RCCL bug rather than fixing it; the bug remains for standalone 8-rank communicators until ROCm/rocm-systems#11245 ships.
Agent details (if applicable)