[RESUBMIT] Ensure ncclCommAbort can abort stuck ncclCommInitRank #103925

pritamdamania87 · 2023-06-20T23:10:00Z

#95715 added the functionality to abort ncclCommInitRankConfig by specifying blocking=0 to enable non-blocking behavior.

However, calling the pg._abort() didn't recover from a stuck ncclCommInitRankConfig since the _abort method only looked through devNCCLCommMap_ map and aborted those communicators. Since ncclCommInitRankConfig was stuck, the communicator itself wasn't added to the map and the host thread was stuck on this line: https://github.com/pytorch/pytorch/blob/main/torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp#L1171. As a result, _abort was a no-op.

To resolve this issue, I added the communicators to inProgressCommMap_ as soon as they were created and then removed them once added to devNCCLCommMap_.

I also added a unit test that was failing without the changes to ProcessGroupNCCL.cpp

pytorch-bot · 2023-06-20T23:10:03Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/103925

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 5e367b5:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

pritamdamania87 · 2023-06-21T02:29:45Z

@osalpekar This is a resubmit of #103264 and CI is green. Is there some specific Sandcastle configuration that was causing issues?

facebook-github-bot · 2023-06-21T02:33:30Z

@osalpekar has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

pritamdamania87 · 2023-06-21T20:28:31Z

@osalpekar Wondering if you found any issues after importing the PR?

osalpekar · 2023-06-21T22:11:49Z

@pritamdamania87 The flakiness is on our end internally. I'll stamp this and we can merge.

pritamdamania87 · 2023-06-22T18:40:45Z

@pytorchbot merge

pytorchmergebot · 2023-06-22T18:42:24Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2023-06-22T20:03:28Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / win-vs2019-cpu-py3 / test (default, 1, 3, windows.4xlarge.nonephemeral)

Details for Dev Infra team

Raised by workflow job

pritamdamania87 · 2023-06-23T04:50:13Z

@pytorchbot rebase

pritamdamania87 · 2023-06-23T04:50:23Z

@pytorchbot merge

pytorchmergebot · 2023-06-23T04:52:16Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2023-06-23T04:52:21Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / win-vs2019-cpu-py3 / test (default, 1, 3, windows.4xlarge.nonephemeral)

Details for Dev Infra team

Raised by workflow job

pytorchmergebot · 2023-06-23T04:52:21Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorch#95715 added the functionality to abort `ncclCommInitRankConfig` by specifying `blocking=0` to enable non-blocking behavior. However, calling the `pg._abort()` didn't recover from a stuck `ncclCommInitRankConfig` since the `_abort` method only looked through `devNCCLCommMap_` map and aborted those communicators. Since `ncclCommInitRankConfig` was stuck, the communicator itself wasn't added to the map and the host thread was stuck on this line: https://github.com/pytorch/pytorch/blob/main/torch/csrc/distributed/c10d/ProcessGroupNCCL.cpp#L1171. As a result, `_abort` was a no-op. To resolve this issue, I added the communicators to `inProgressCommMap_` as soon as they were created and then removed them once added to `devNCCLCommMap_`. I also added a unit test that was failing without the changes to ProcessGroupNCCL.cpp

pytorchmergebot · 2023-06-23T04:52:27Z

Successfully rebased user/pdamania/nccl_abort onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout user/pdamania/nccl_abort && git pull --rebase)

pritamdamania87 · 2023-06-23T18:34:58Z

@pytorchbot merge

pritamdamania87 · 2023-06-23T18:35:18Z

@osalpekar Do we need to import again? :)

This Pull Request has changes that have not been imported to the Diff, please import the Pull Request

pytorchmergebot · 2023-06-23T18:36:50Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2023-06-23T18:36:54Z

Merge failed

Reason: 1 jobs have failed, first few of them are: Meta Internal-Only Changes Check

Details for Dev Infra team

Raised by workflow job

facebook-github-bot · 2023-06-27T04:18:41Z

@osalpekar has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

pytorchmergebot · 2023-06-27T04:19:06Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2023-06-27T04:19:08Z

Merge failed

Reason: Comment with id 1608768805 not found

Details for Dev Infra team

Raised by workflow job

osalpekar · 2023-06-27T04:19:12Z

@pytorchbot merge -f "all checks passed, no internal-only changes"

pytorchmergebot · 2023-06-27T04:21:58Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

osalpekar · 2023-06-27T04:29:12Z

@pritamdamania87 Yeah looks like this needed a re-import since the PR was updated, but it's merged now :)

pritamdamania87 requested review from H-Huang, awgu, d4l3k, fduwjj, fegin, kiukchung, kwen2501, mrshenli, rohan-varma, wanchaol and zhaojuanmao as code owners June 20, 2023 23:10

pytorch-bot bot added the release notes: distributed (c10d) release notes category label Jun 20, 2023

pytorchbot added the open source label Jun 20, 2023

janeyx99 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jun 21, 2023

osalpekar approved these changes Jun 21, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 22, 2023

pytorchmergebot added the merging label Jun 22, 2023

pytorchmergebot removed the merging label Jun 22, 2023

pytorchmergebot added the merging label Jun 23, 2023

pytorchmergebot removed the merging label Jun 23, 2023

pritamdamania87 added 2 commits June 23, 2023 04:52

Fix device

5e367b5

pytorchmergebot force-pushed the user/pdamania/nccl_abort branch from 35663c6 to 5e367b5 Compare June 23, 2023 04:52

pytorchmergebot added the merging label Jun 23, 2023

pytorchmergebot removed the merging label Jun 23, 2023

pytorchmergebot added the merging label Jun 27, 2023

pytorchmergebot removed the merging label Jun 27, 2023

pytorchmergebot added the merging label Jun 27, 2023

pytorchmergebot added Merged and removed merging labels Jun 27, 2023

pytorchmergebot closed this in 572ff27 Jun 27, 2023

pritamdamania87 mentioned this pull request Aug 16, 2023

TImeout in NCCL doesn't work #104506

Open

[RESUBMIT] Ensure ncclCommAbort can abort stuck ncclCommInitRank #103925

[RESUBMIT] Ensure ncclCommAbort can abort stuck ncclCommInitRank #103925

Uh oh!

Conversation

pritamdamania87 commented Jun 20, 2023

Uh oh!

pytorch-bot bot commented Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/103925

✅ No Failures

Uh oh!

pritamdamania87 commented Jun 21, 2023

Uh oh!

facebook-github-bot commented Jun 21, 2023

Uh oh!

pritamdamania87 commented Jun 21, 2023

Uh oh!

osalpekar commented Jun 21, 2023

Uh oh!

pritamdamania87 commented Jun 22, 2023

Uh oh!

pytorchmergebot commented Jun 22, 2023

Merge started

Uh oh!

pytorchmergebot commented Jun 22, 2023

Merge failed

Uh oh!

pritamdamania87 commented Jun 23, 2023

Uh oh!

pritamdamania87 commented Jun 23, 2023

Uh oh!

pytorchmergebot commented Jun 23, 2023

Merge started

Uh oh!

pytorchmergebot commented Jun 23, 2023

Merge failed

Uh oh!

pytorchmergebot commented Jun 23, 2023

Uh oh!

pytorchmergebot commented Jun 23, 2023

Uh oh!

pritamdamania87 commented Jun 23, 2023

Uh oh!

pritamdamania87 commented Jun 23, 2023

Uh oh!

pytorchmergebot commented Jun 23, 2023

Merge started

Uh oh!

pytorchmergebot commented Jun 23, 2023

Merge failed

Uh oh!

facebook-github-bot commented Jun 27, 2023

Uh oh!

pytorchmergebot commented Jun 27, 2023

Merge started

Uh oh!

pytorchmergebot commented Jun 27, 2023

Merge failed

Uh oh!

osalpekar commented Jun 27, 2023

Uh oh!

pytorchmergebot commented Jun 27, 2023

Merge started

Uh oh!

osalpekar commented Jun 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pytorch-bot bot commented Jun 20, 2023 •

edited

Loading