[core][experimental] Avoid false positives in deadlock detection #47912

kevin85421 · 2024-10-06T00:39:48Z

Why are these changes needed?

Initially, the deadlock detection mechanism was designed based on a DAGNode-based execution schedule, meaning the read, compute, and write operations belonging to the same DAGNode will be executed consecutively.

Then, #46911 changes the execution schedule from DAGNode-based to operation-based to avoid deadlocks caused by NCCL operations. Therefore, there are some false positives for the old deadlock detection mechanism.

With this PR, the deadlock detection focuses only on NCCL operations where both the reader and writer belong to the same actor.

Future works: Avoid false negatives

actor1.f1 ---> actor2.f1
           |
actor1.f2 --

In this case, actor1.f1 and actor1.f2 have control dependencies between them. If actor2.f1 reads actor1.f2 first and then actor1.f1, there will be a deadlock. However, this deadlock is not detectable until we have a more granular execution schedule.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Kai-Hsun Chen <kaihsun@anyscale.com>

kevin85421 · 2024-10-06T00:49:40Z

python/ray/dag/tests/experimental/test_torch_tensor_dag.py

@@ -746,28 +745,6 @@ def test_torch_tensor_nccl_nested_dynamic(ray_start_regular):
    compiled_dag.teardown()


-@pytest.mark.parametrize("ray_start_regular", [{"num_cpus": 4}], indirect=True)


This has already been covered by tests in test_detect_deadlock_dag.py.

python/ray/dag/compiled_dag_node.py

Signed-off-by: Kai-Hsun Chen <kaihsun@anyscale.com>

…-project#47912) Signed-off-by: Kai-Hsun Chen <kaihsun@anyscale.com> Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>

update

c234cd7

Signed-off-by: Kai-Hsun Chen <kaihsun@anyscale.com>

kevin85421 commented Oct 6, 2024

View reviewed changes

kevin85421 marked this pull request as ready for review October 6, 2024 00:59

kevin85421 assigned rkooo567 and ruisearch42 Oct 6, 2024

ruisearch42 approved these changes Oct 7, 2024

View reviewed changes

python/ray/dag/compiled_dag_node.py Outdated Show resolved Hide resolved

update

4941d2c

Signed-off-by: Kai-Hsun Chen <kaihsun@anyscale.com>

kevin85421 added the go add ONLY when ready to merge, run all tests label Oct 7, 2024

kevin85421 assigned kevin85421 and unassigned rkooo567 and ruisearch42 Oct 7, 2024

jjyao merged commit 02d65eb into ray-project:master Oct 8, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core][experimental] Avoid false positives in deadlock detection #47912

[core][experimental] Avoid false positives in deadlock detection #47912

kevin85421 commented Oct 6, 2024 •

edited

Loading

kevin85421 Oct 6, 2024

		@@ -746,28 +745,6 @@ def test_torch_tensor_nccl_nested_dynamic(ray_start_regular):
		compiled_dag.teardown()


		@pytest.mark.parametrize("ray_start_regular", [{"num_cpus": 4}], indirect=True)

[core][experimental] Avoid false positives in deadlock detection #47912

[core][experimental] Avoid false positives in deadlock detection #47912

Conversation

kevin85421 commented Oct 6, 2024 • edited Loading

Why are these changes needed?

Future works: Avoid false negatives

Related issue number

Checks

kevin85421 Oct 6, 2024

Choose a reason for hiding this comment

kevin85421 commented Oct 6, 2024 •

edited

Loading