fix issue 3264

[woodlgz]

Checklist before submitting

• Did you read the contributor guide?
• Did you update the docs?
• Did you write any tests to validate this change?
• Did you update the CHANGELOG, if this change affects users?

Description

This is a PR from JIZHI Team & Taiji AI platform in Tencent.
Fixes #3264.
avoid enqueuing more collective ops while horovod is shutting down.

Review process to land

1. All tests and other checks must succeed.
2. At least one member of the technical steering committee must review and approve.
3. If any member of the technical steering committee requests changes, they must be addressed.

[github-actions]

Unit Test Results

     718 files   -   28       718 suites   - 28   5h 56m 57s ⏱️ + 9m 40s
     591 tests ±    0       552 ✔️ ±    0       34 💤 ±    0    5 ❌ ±0 
15 375 runs   - 662  10 705 ✔️  - 512  4 652 💤  - 150  18 ❌ ±0 

For more details on these failures, see this check.

Results for commit 280a716. ± Comparison against base commit 28eaf2b.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Results (with flaky tests)

     942 files  ±0       942 suites  ±0   6h 44m 51s ⏱️ + 15m 35s
     591 tests ±0       551 ✔️  - 1       34 💤 ±0    6 ❌ +1 
20 671 runs  ±0  14 214 ✔️  - 5  6 402 💤 +4  55 ❌ +1 

For more details on these failures, see this check.

Results for commit 280a716. ± Comparison against base commit 28eaf2b.

♻️ This comment has been updated with latest results.

[tgaddair]

Thanks for the PR @woodlgz. Can you rebase off master to pick up any test fixes?

@romerojosh can you double check the changes to checking the shut down state here? It looks like it should be fine, but wanted to get a second opinion.

[romerojosh]

The changes look fine to me, but I'm not exactly sure what we expect to happen in an elastic scenario, which seems to be what this issue is about.

In an elastic case, do we expect these workers to resubmit these collective requests? If so, then this change looks good to me.

[ashahab]

@romerojosh I have a clarifying question: is horovod_global.shut_down a worker-only state or a shared state among all workers? If it's the former then yes, the remaining elastic workers should resubmit the collective ops(after forming a new ring).

[woodlgz]

@ashahab yes, horovod_global.shut_down is a worker only state, but somehow all workers remaining will reach a common shutdown state and in elastic training they will form a new ring in next loop.
@romerojosh current training loop is wrapped in a so-called elastic loop in https://github.com/horovod/horovod/blob/master/horovod/common/elastic.py#L151. so in next loop,user training function will try to redo the calculation and resubmit the ops.

[tgaddair]

LGTM! Looks like the failing tests are head versions, which have some known issues being addressed. So we can go ahead and land this.