[core] Fix placement groups scheduling when no resources are specified #39946

vitsai · 2023-09-28T18:39:10Z

Before, actors scheduled with no resources and placement groups would not be placed with the placement group or bundle. Fix this by using the phantom bundle resource that is also used for the Ready() call as an additional placement group constraint. (See #16819 for context)

Why are these changes needed?

Related issue number

#34866

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: vitsai <vitsai@cs.stanford.edu>

vitsai · 2023-10-03T03:30:06Z

Bug fix: #40038

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

rkooo567

Can you post the ray status output when there's a pending task that requires a placement gruop (just to see the output does not regress due to this change we always add bundles resources 0.0001)

python/ray/tests/test_placement_group.py

src/ray/common/bundle_spec.cc

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

vitsai · 2023-10-05T07:15:19Z

Here is output with both pending and running pg tasks:

======== Autoscaler status: 2023-10-05 07:13:09.740863 ========
GCS request time: 0.000660s

Node status
---------------------------------------------------------------
Active:
 1 node_7dedc912c22c07df0fd49dc5b6e8450472310ad4041ddd2540e519dc
Idle:
 (no idle nodes)
Pending:
 (no pending nodes)
Recent failures:
 (no failures)

Resources
---------------------------------------------------------------
Total Usage:
 4.0/36.0 CPU (4.0 used of 8.0 reserved in placement groups)
 0B/35.23GiB memory
 0B/17.61GiB object_store_memory

Total Demands:
 {'CPU': 1.0}: 6+ pending tasks/actors (6+ using placement groups)

Node: 7dedc912c22c07df0fd49dc5b6e8450472310ad4041ddd2540e519dc
 Usage:
  4.0/36.0 CPU (4.0 used of 8.0 reserved in placement groups)
  0B/35.23GiB memory
  0B/17.61GiB object_store_memory
 Activity:
  Resource: CPU currently in use.
  Busy workers on node.
  Resource: CPU_group_0_38c7049e01961594ddbd48ae247806000000 currently in use.
  Resource: CPU_group_38c7049e01961594ddbd48ae247806000000 currently in use.

I think the only potential change needed is in node activity and that can be a follow up as it is not super noisy.

rkooo567

LGTM. I think we should merge this after we merge #40038.

rkooo567 · 2023-10-10T13:42:54Z

ping me after ^

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

…rset of Resources (#40038) Looking at the code, we use RequiredResources and RequiredPlacementResources in various parts of the codebase for scheduling. This is generally fine, but since AllocateTaskResources in LocalTaskManager uses RequiredResources to allocate, and spillback/scheduling (GetBestSchedulableNode) uses RequiredPlacementResources, there is a potential infinite loop if we don't maintain the invariant of PlacementResources being a superset of Resources. Fix it so that we do. Note the surface area of the bug was actually quite small because this invariant is maintained when num_cpus=1. For testing: This is already tested by test_scheduling.py in #39946

vitsai · 2023-10-19T20:35:29Z

This looks ready to merge; same failures as the stacked PR

jjyao · 2023-10-19T21:05:41Z

src/ray/core_worker/common.h

@@ -108,7 +108,8 @@ struct ActorCreationOptions {
        max_task_retries(max_task_retries),
        max_concurrency(max_concurrency),
        resources(resources),
-        placement_resources(placement_resources),
+        placement_resources(placement_resources.empty() ? resources


This change is already merged?

…specified (#39946)" This reverts commit af332f4.

…specified (#39946)" (#40506) This reverts commit af332f4.

…ces are specified (#39946)" (#40506)" This reverts commit 66cfec9. Signed-off-by: vitsai <vitsai@cs.stanford.edu>

…#39946 (#43269) Signed-off-by: vitsai <vitsai@cs.stanford.edu> Co-authored-by: vitsai <vitsai@cs.stanford.edu> Co-authored-by: SangBin Cho <sangcho@sangcho-LT93GQWG9C.local>

fix

b76330d

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

vitsai assigned jjyao and rkooo567 Sep 28, 2023

fix

de370b8

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

vitsai mentioned this pull request Oct 3, 2023

[core] Fix placement group invariant of PlacementResources being superset of Resources #40038

Merged

8 tasks

bug fix

755678b

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

vitsai requested a review from a team as a code owner October 3, 2023 16:45

rkooo567 reviewed Oct 4, 2023

View reviewed changes

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 4, 2023

comments

2ff25a6

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

vitsai removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 5, 2023

rkooo567 approved these changes Oct 5, 2023

View reviewed changes

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 10, 2023

vitsai added 2 commits October 12, 2023 19:18

frontend-independent fix

3b463fe

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

Merge branch 'resource-fix' into pg-zero

a61933a

vitsai removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 12, 2023

vitsai added 10 commits October 12, 2023 22:26

remove python frontend fix for testing

32a73ab

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

Merge branch 'master' into resource-fix

0517211

comments

34c41d3

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

Merge branch 'resource-fix' into pg-zero

e5c74ed

add back deleted line

1e39515

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

Merge branch 'resource-fix' into pg-zero

d1484fb

fix bug

358d4a4

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

Merge branch 'resource-fix' into pg-zero

cb0780f

comment

8874cf5

Signed-off-by: vitsai <vitsai@cs.stanford.edu>

Merge branch 'resource-fix' into pg-zero

d3eeebe

jjyao reviewed Oct 19, 2023

View reviewed changes

rkooo567 merged commit af332f4 into ray-project:master Oct 19, 2023

vitsai added a commit that referenced this pull request Oct 19, 2023

Revert "[core] Fix placement groups scheduling when no resources are …

46e22dc

…specified (#39946)" This reverts commit af332f4.

vitsai mentioned this pull request Oct 19, 2023

Revert "[core] Fix placement groups scheduling when no resources are specified" #40506

Merged

aslonnie pushed a commit that referenced this pull request Oct 20, 2023

Revert "[core] Fix placement groups scheduling when no resources are …

66cfec9

…specified (#39946)" (#40506) This reverts commit af332f4.

vitsai added a commit that referenced this pull request Oct 20, 2023

Revert "Revert "[core] Fix placement groups scheduling when no resour…

c75082d

…ces are specified (#39946)" (#40506)" This reverts commit 66cfec9. Signed-off-by: vitsai <vitsai@cs.stanford.edu>

rickyyx mentioned this pull request Oct 26, 2023

Release test long_running_many_actor_tasks failed #40568

Closed

rkooo567 mentioned this pull request Feb 20, 2024

[core] Fix placement groups scheduling when no resources are specified #39946 #43269

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Fix placement groups scheduling when no resources are specified #39946

[core] Fix placement groups scheduling when no resources are specified #39946

vitsai commented Sep 28, 2023

vitsai commented Oct 3, 2023

rkooo567 left a comment

vitsai commented Oct 5, 2023

rkooo567 left a comment

rkooo567 commented Oct 10, 2023

vitsai commented Oct 19, 2023

jjyao Oct 19, 2023

[core] Fix placement groups scheduling when no resources are specified #39946

[core] Fix placement groups scheduling when no resources are specified #39946

Conversation

vitsai commented Sep 28, 2023

Why are these changes needed?

Related issue number

Checks

vitsai commented Oct 3, 2023

rkooo567 left a comment

Choose a reason for hiding this comment

vitsai commented Oct 5, 2023

rkooo567 left a comment

Choose a reason for hiding this comment

rkooo567 commented Oct 10, 2023

vitsai commented Oct 19, 2023

jjyao Oct 19, 2023

Choose a reason for hiding this comment