[core] v1 autoscaler consistency fix #40488

vitsai · 2023-10-19T09:12:01Z

A less invasive way to do it.

Why are these changes needed?

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

rickyyx · 2023-10-23T16:45:14Z

src/ray/gcs/gcs_server/gcs_resource_manager.cc

@@ -188,15 +190,15 @@ void GcsResourceManager::HandleGetAllResourceUsage(
    rpc::GetAllResourceUsageRequest request,
    rpc::GetAllResourceUsageReply *reply,
    rpc::SendReplyCallback send_reply_callback) {
-  if (!node_resource_usages_.empty()) {
+  if (!gcs_autoscaler_state_manager_.GetNodeResourceInfo().empty()) {


I wonder if we should just pull the GcsPlacementGroupManager for the current pg load? Instead of relying on the periodic runner?

That's a bigger refactor beyond the scope of this change

If GcsAutoscalerStateManager exposes a GetPlacementGroupLoad function (which just gets the load from the PG manager) for the below:

ray/src/ray/gcs/gcs_server/gcs_autoscaler_state_manager.cc

Line 125 in 17e6cc2

auto placement_group_load = gcs_placement_group_manager_.GetPlacementGroupLoad();

Then we could remove the dependency between GcsResourceManager and GcsPlacementGroupManager? So we don't need changes here: https://github.com/ray-project/ray/pull/40488/files#diff-f33835748a6d386dd44a3449d9299117799bd9b552655054f901e42c5fbb59a1R253-R262 ?

This also unifies v1/v2 for how PG load is populated with a potentially smaller change?

The periodic function also schedules pending placement groups, it doesn't just update the info

Also, we already removed the dependency in the current change

jjyao · 2023-10-24T18:46:57Z

src/ray/gcs/gcs_server/gcs_server.cc

+        gcs_resource_manager_->UpdatePlacementGroupLoad(
+            gcs_placement_group_manager_->GetPlacementGroupLoad());


I think we can remove placement_group_load in gcs_resource_manager since it will get all loads from gcs autoscaler manager now?

We can also do that this way: #40254

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

stale · 2023-12-15T04:00:04Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

DmitriGekhtman · 2024-10-03T21:45:47Z

Is this a viable approach to resolving the issue?

vitsai added 13 commits October 11, 2023 08:48

prototype

a1c5901

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

test

1d9d079

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

some comments, autoscaler v1

3985c42

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

lagging comment and undo earlier change

084b353

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

add lag to mock node provider

1a4f74e

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

comments

c989540

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

fix test

cc39b4b

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

comments

a2b7bd8

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

fix bug

c384499

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

comment

990b332

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

comment

7bbd5a8

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

bug

4a3098d

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

initial

ba8e266

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

vitsai changed the base branch from report-usage-v2 to master October 19, 2023 09:20

vitsai changed the base branch from master to report-usage-v2 October 19, 2023 09:20

vitsai added 4 commits October 19, 2023 10:04

fixes

570f5c9

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

another fix

2a89f5d

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

Merge branch 'report-usage-v2' into new-usage-v1

44263a0

remove circular dependency

dd5b900

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

vitsai assigned jjyao and rickyyx Oct 19, 2023

vitsai added 3 commits October 19, 2023 21:31

fix tests

c19d6a6

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

more test fixes

5b4e0c9

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

mock

03bbc31

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

rickyyx reviewed Oct 24, 2023

View reviewed changes

jjyao reviewed Oct 24, 2023

View reviewed changes

vitsai mentioned this pull request Oct 24, 2023

[core] Autoscaler consistency fix [2/2] #40254

Closed

8 tasks

vitsai added 3 commits October 24, 2023 20:07

comments

295d49d

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

oops

0857476

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

Merge branch 'master' into new-usage-v1

8e9f7fa

vitsai added 2 commits October 26, 2023 22:19

remove dead code

8360f68

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

wip

c157614

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

vitsai changed the base branch from report-usage-v2 to master October 30, 2023 16:45

fix client test

783bae3

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

vitsai mentioned this pull request Oct 30, 2023

[core] Remove unused GCS RPCs #40808

Merged

8 tasks

vitsai added 10 commits October 30, 2023 21:01

remove mock

aa7e724

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

fix accessor test

d80ebf6

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

Merge branch 'experimental-remove' into new-usage-v1

58c0a32

fix merge

eed2fa0

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

fix failing tests

1632340

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

fix resource manager test

b672005

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

wip

8228655

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

monitor server

7017ee9

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

Merge branch 'master' into new-usage-v1

3828a13

fix merge and test

e7542db

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

vitsai mentioned this pull request Nov 8, 2023

[Autoscaler] starts extra workers than necessary #36926

Open

stale bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Dec 15, 2023

anyscalesam closed this Apr 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] v1 autoscaler consistency fix #40488

[core] v1 autoscaler consistency fix #40488

vitsai commented Oct 19, 2023

rickyyx Oct 23, 2023

vitsai Oct 24, 2023

rickyyx Oct 24, 2023

vitsai Oct 24, 2023

vitsai Oct 24, 2023

jjyao Oct 24, 2023

vitsai Oct 24, 2023

stale bot commented Dec 15, 2023

DmitriGekhtman commented Oct 3, 2024

		gcs_resource_manager_->UpdatePlacementGroupLoad(
		gcs_placement_group_manager_->GetPlacementGroupLoad());

[core] v1 autoscaler consistency fix #40488

[core] v1 autoscaler consistency fix #40488

Conversation

vitsai commented Oct 19, 2023

Why are these changes needed?

Related issue number

Checks

rickyyx Oct 23, 2023

Choose a reason for hiding this comment

vitsai Oct 24, 2023

Choose a reason for hiding this comment

rickyyx Oct 24, 2023

Choose a reason for hiding this comment

vitsai Oct 24, 2023

Choose a reason for hiding this comment

vitsai Oct 24, 2023

Choose a reason for hiding this comment

jjyao Oct 24, 2023

Choose a reason for hiding this comment

vitsai Oct 24, 2023

Choose a reason for hiding this comment

stale bot commented Dec 15, 2023

DmitriGekhtman commented Oct 3, 2024