[Serve] feat: scale down non matching primary labels replica first by manhld0206 · Pull Request #61488 · ray-project/ray

manhld0206 · 2026-03-04T12:59:12Z

gemini-code-assist

Code Review

This pull request introduces a change to the deployment scale-down logic to prioritize stopping replicas on nodes that do not match the deployment's primary label_selector. This is achieved by modifying the sorting key in _get_replicas_to_stop. While the overall approach is correct, I found a critical issue in the implementation of the sorting logic that inverts the intended priority. My review includes a specific comment with a suggested fix for this issue.

abrarsheikh

like the approach overall, it's reasonable. Please add e2e tests.

abrarsheikh · 2026-03-04T13:42:15Z

function has grown a bit complicated, please update the docstring explain the algorithm

abrarsheikh · 2026-03-04T13:43:02Z

+            match_primary_labels = int(
+                node_labels_match_selector(node_labels, primary_labels)
            )


maybe this should also take into account placement group label selector.

+1 I think we can check node_labels against self._deployments[deployment_id].placement_group_bundle_label_selector if it's set, label_selector is only for the deployment Actor while placement_group_bundle_label_selector is applied to each of the bundles that dictate where the replicas are placed. If the node labels don't match any bundles/bundle_label_selector in placement_group_bundle_label_selector, then it's infeasible for scheduling and should be scaled down first.

abrarsheikh · 2026-03-04T13:44:26Z

please also fix DCO

ryanaoleary · 2026-03-05T03:42:29Z

@@ -1089,6 +1090,8 @@ def _get_replicas_to_stop(
            if len(replicas_to_stop) == max_num_to_stop:
                return replicas_to_stop

+        actor_options = self._deployments[deployment_id].actor_options or {}
+        primary_labels: Dict[str, str] = actor_options.get("label_selector", {})


Can we do primary_label_selector or similar, I think an Actor can also have labels so it might get confusing.

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

…caling_down_fallback_labels_replica

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

manhld0206 · 2026-03-16T07:08:33Z

@abrarsheikh The PR is ready for review. Could you take a look?

abrarsheikh · 2026-03-18T03:31:01Z

+        assert dep_id in deployment_to_replicas_to_stop
+        return deployment_to_replicas_to_stop[dep_id]
+
+    def test_downscale_prioritizes_non_running_replicas(self, ray_cluster):


the current tests are wriiten as unit tests, if you see the rest of the tests in the file they are all written as integration tests. Let's maintain consistency with other tests.

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

…caling_down_fallback_labels_replica

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

jeffreywang88

great contribution! let's add some more tests:

no tests for the bundle_label_selector path. both integ tests use label_selector.
test for head node interaction: verify that the head node replicas are still deprioritized even when they're on a fallback node

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

manhld0206 · 2026-03-20T05:45:04Z

test for head node interaction: verify that the head node replicas are still deprioritized even when they're on a fallback node

@jeffreywang-anyscale We should prioritize fallback node condition over head node shouldn't we? I think it makes more sense to always scale down node which doesn't match the primary label selector regradless it's the head node or not

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

…caling_down_fallback_labels_replica

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

jeffreywang88 · 2026-03-20T19:35:50Z

test for head node interaction: verify that the head node replicas are still deprioritized even when they're on a fallback node

@jeffreywang-anyscale We should prioritize fallback node condition over head node shouldn't we? I think it makes more sense to always scale down node which doesn't match the primary label selector regardless it's the head node or not

The main goal of this PR is to free up spot instance resources. Head node hosts critical components like GCS, autoscaler, and Serve controller, so even though we evict replicas, we can't delete the underlying spot instance. Because of this, I think replicas on the head node should be deprioritized for eviction during scale-down. They are last in line, since removing them provides no benefit in terms of draining nodes.

While Ray offers GCS fault tolerance via external Redis, allowing workers to continue serving traffic during head node recovery, the control plane is unavailable during this period (e.g. no deployments can be updated). Avoiding head node loss remains important for cluster stability.

We can avoid scheduling head node on spot instances by configuring nodeSelector in the KubeRay manifest. But even if we do schedule head node on spot instances, I think we should keep the head node, and therefore there isn't much benefit to evict replicas on the head node.

Let's get a second opinion, @abrarsheikh WDYT?

jeffreywang88 · 2026-03-20T19:36:11Z

Could you please fix premerge test failures?

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

abrarsheikh · 2026-03-26T22:36:39Z

test for head node interaction: verify that the head node replicas are still deprioritized even when they're on a fallback node

@jeffreywang-anyscale We should prioritize fallback node condition over head node shouldn't we? I think it makes more sense to always scale down node which doesn't match the primary label selector regardless it's the head node or not

The main goal of this PR is to free up spot instance resources. Head node hosts critical components like GCS, autoscaler, and Serve controller, so even though we evict replicas, we can't delete the underlying spot instance. Because of this, I think replicas on the head node should be deprioritized for eviction during scale-down. They are last in line, since removing them provides no benefit in terms of draining nodes.

While Ray offers GCS fault tolerance via external Redis, allowing workers to continue serving traffic during head node recovery, the control plane is unavailable during this period (e.g. no deployments can be updated). Avoiding head node loss remains important for cluster stability.

We can avoid scheduling head node on spot instances by configuring nodeSelector in the KubeRay manifest. But even if we do schedule head node on spot instances, I think we should keep the head node, and therefore there isn't much benefit to evict replicas on the head node.

Let's get a second opinion, @abrarsheikh WDYT?

I agree with @jeffreywang-anyscale. The head node should be the last to go, behavior is consistent with what currently exists. Also, no serious user will run the head node of a spot instance, so this decision is moot.

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

manhld0206 · 2026-03-31T06:03:36Z

@abrarsheikh @jeffreywang-anyscale Could you check again? I have updated code to make replicas on head node go down last

Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>

jeffreywang88

thanks for the work! pushed a minor fix for the tests to ensure we're testing the intended priorities.

…ay-project#61488) > 💡 Tip: Mark as draft if you want early feedback, or ready for review when it's complete. ## Description > Briefly describe what this PR accomplishes and why it's needed. - Prioritze sclaing down replica which doesn't fit label_selector (fallback_strategy enabled) ## Related issues > Link related issues: "Fixes ray-project#1234", "Closes ray-project#1234", or "Related to ray-project#1234". - Related to ray-project#58959 ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com> Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com> Co-authored-by: Jeffrey Wang <jeffreywang@anyscale.com>

gemini-code-assist Bot reviewed Mar 4, 2026

View reviewed changes

Comment thread python/ray/serve/_private/deployment_scheduler.py Outdated

manhld0206 mentioned this pull request Mar 4, 2026

[Serve] Allow customize Ray Serve auto scaler scale down logic #58959

Closed

abrarsheikh reviewed Mar 4, 2026

View reviewed changes

ryanaoleary reviewed Mar 5, 2026

View reviewed changes

manhld0206 changed the title ~~feat: scale down non matching primary labels first~~ Mar 5, 2026

manhld0206 force-pushed the feat/prioritize_scaling_down_fallback_labels_replica branch from 722d7af to aae4e32 Compare March 5, 2026 10:14

manhld0206 added 2 commits March 5, 2026 19:15

feat: scale down non matching primary labels first

46f24fc

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

fix: fix logic

62cab53

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

manhld0206 force-pushed the feat/prioritize_scaling_down_fallback_labels_replica branch from aae4e32 to 62cab53 Compare March 5, 2026 10:15

manhld0206 added 3 commits March 5, 2026 20:55

feat: consider bundle label selector during scale down

90489d1

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

feat: add e2d test

d929f12

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

Merge remote-tracking branch 'upstream/master' into feat/prioritize_s��

591d7f6

…caling_down_fallback_labels_replica

manhld0206 marked this pull request as ready for review March 12, 2026 14:22

manhld0206 requested a review from a team as a code owner March 12, 2026 14:22

manhld0206 requested a review from abrarsheikh March 12, 2026 14:25

cursor Bot reviewed Mar 12, 2026

View reviewed changes

Comment thread python/ray/serve/tests/test_deployment_scheduler.py Outdated

feat: fix test

5e28684

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

cursor Bot reviewed Mar 12, 2026

View reviewed changes

Comment thread python/ray/serve/_private/deployment_scheduler.py Outdated

Comment thread python/ray/serve/tests/test_deployment_scheduler.py Outdated

ray-gardener Bot added serve Ray Serve Related Issue community-contribution Contributed by the community labels Mar 12, 2026

manhld0206 added 2 commits March 15, 2026 20:11

feat: improve test

0c20909

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

fix: fix test

9e0126d

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

abrarsheikh added the go add ONLY when ready to merge, run all tests label Mar 18, 2026

abrarsheikh reviewed Mar 18, 2026

View reviewed changes

manhld0206 added 4 commits March 19, 2026 11:52

fix: fix var name and test

3fa43d9

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

fix: fix test

7425263

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

Merge remote-tracking branch 'upstream/master' into feat/prioritize_s…

60aaddf

…caling_down_fallback_labels_replica

fix: fix test

15d81af

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

cursor Bot reviewed Mar 19, 2026

View reviewed changes

Comment thread python/ray/serve/tests/test_deployment_scheduler.py Outdated

fix: remove unreachable

6456cb7

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

jeffreywang88 reviewed Mar 19, 2026

View reviewed changes

manhld0206 added 2 commits March 20, 2026 14:35

feat: refactor and improve test

3468641

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

feat: use placement group bundle in test

169cd2d

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

manhld0206 added 5 commits March 20, 2026 14:48

fix: small change

b1ead80

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

fix: change scaling config for more stable test

5c944e0

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

fix: change how to wait for upscale to be more stable

45304d4

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

fix: wait for head node

0e3b56e

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

Merge remote-tracking branch 'upstream/master' into feat/prioritize_s…

0ddd885

…caling_down_fallback_labels_replica

cursor Bot reviewed Mar 20, 2026

View reviewed changes

Comment thread python/ray/serve/_private/deployment_scheduler.py

manhld0206 added 2 commits March 21, 2026 12:49

fix: move test to another file to avoid conflict

915e900

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

fix: add new test file to bazel build

92f04f5

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

feat: prioritize replicas not on head node

b910196

Signed-off-by: Mạnh Lê Đức <naruto12308@gmail.com>

manhld0206 requested review from abrarsheikh and jeffreywang88 March 27, 2026 13:29

update tests

15b9690

Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>

jeffreywang88 approved these changes Apr 1, 2026

View reviewed changes

abrarsheikh approved these changes Apr 3, 2026

View reviewed changes

abrarsheikh merged commit b610044 into ray-project:master Apr 3, 2026
6 checks passed

Uh oh!

Conversation

manhld0206 commented Mar 4, 2026 • edited by abrarsheikh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

abrarsheikh left a comment

Choose a reason for hiding this comment

abrarsheikh Mar 4, 2026

Choose a reason for hiding this comment

abrarsheikh Mar 4, 2026

Choose a reason for hiding this comment

ryanaoleary Mar 5, 2026

Choose a reason for hiding this comment

abrarsheikh commented Mar 4, 2026

ryanaoleary Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

manhld0206 commented Mar 16, 2026

Uh oh!

abrarsheikh Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

jeffreywang88 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

manhld0206 commented Mar 20, 2026

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jeffreywang88 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

jeffreywang88 commented Mar 20, 2026

abrarsheikh commented Mar 26, 2026

manhld0206 commented Mar 31, 2026

jeffreywang88 left a comment

Choose a reason for hiding this comment

Uh oh!

Labels

4 participants

manhld0206 commented Mar 4, 2026 •

edited by abrarsheikh

Loading

jeffreywang88 commented Mar 20, 2026 •

edited

Loading