[Core][Label Scheduling 1/n]Add NodeLabelSchedulingStrategy API in python by larrylian · Pull Request #36418 · ray-project/ray

larrylian · 2023-06-14T15:10:23Z

Why are these changes needed?

After our offline discussion, we have come up with the following plan. This API plan will review and finalization by more people in the future.

Complete form of API:

MyActor.options(
        scheduling_strategy=NodeLabelSchedulingStrategy(
            [
                {
                    "region": IN("us"), 
                    "gpu_type": IN("A100")
                },
                {
                    "region": IN("asia", "europe"), 
                    "gpu_type": IN("T100")
                 },
            ]
        )
).remote()

User also can use more simple api:

 MyActor.options(
        scheduling_strategy=NodeLabelSchedulingStrategy(
                {
                    "region": IN("us"), 
                    "gpu_type": IN("A100")
                }
)).remote()

Related issue number

[Core][Labels Scheduling]Finalize the new node affinity scheduling with node labels API in the Python worker #36419

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 :(

jjyao · 2023-07-10T04:43:58Z

python/ray/util/label_scheduling_strategies.py

Let's start with alpha.

jjyao · 2023-07-10T04:44:40Z

python/ray/util/label_scheduling_strategies.py

Can we move this file into scheduling_strategies.py? All the other strategies are defined there.

python/ray/util/label_scheduling_strategies.py

src/ray/protobuf/common.proto

python/ray/util/scheduling_strategies.py

jjyao · 2023-07-10T16:11:42Z

python/ray/util/scheduling_strategies.py

_convert_map_to_label_math_expressions

I change to _convert_map_to_expressions. more concise.

jjyao · 2023-07-10T16:13:53Z

python/ray/util/scheduling_strategies.py

Suggested change

"The value of label match expression in node label "

"scheduling strategy must be created by "

"`IN` or `NOT_IN` or`EXIST` or `DOES_NOT_EXIST`."

"The value of label match expression in node label "

"scheduling strategy must be "

"`In` or `NotIn` or`Exists` or `DoesNotExist` operator."

python/ray/util/scheduling_strategies.py

python/ray/tests/test_node_label_scheduling_strategy.py

python/ray/util/scheduling_strategies.py

jjyao · 2023-07-11T04:29:19Z

python/ray/util/scheduling_strategies.py

Suggested change

class LabelMatchExpression:

class _LabelMatchExpression:

jjyao · 2023-07-11T04:31:15Z

python/ray/util/scheduling_strategies.py

Suggested change

"""The schedling strategy of node Affinity with labels

Simplified usage:

scheduling_strategy=NodeLabelSchedulingStrategy({

"region": IN("us"),

"gpu_type": EXISTS()

})

"""

"""Label based node affinity scheduling strategy

scheduling_strategy=NodeLabelSchedulingStrategy({

"region": IN("us"),

"gpu_type": EXISTS()

})

"""

pcmoritz · 2023-07-13T17:47:42Z

python/ray/util/scheduling_strategies.py

lable -> label

pcmoritz · 2023-07-13T17:49:00Z

python/ray/util/scheduling_strategies.py

Be a little more specific here like Expected value {value} in position {index} to be of type str

pcmoritz · 2023-07-13T17:52:27Z

Before this can be merged, the error messages should be improved:

First, see my comments
But then, they should also tell the user which key is associated to the value that has the error, and whether it is the "soft" branch

Think about the user experience: They will get the error, and they will need to find out exactly which value caused the error. Currently that is extremely hard at best.

larrylian · 2023-07-14T01:58:27Z

Before this can be merged, the error messages should be improved:

First, see my comments

But then, they should also tell the user which key is associated to the value that has the error, and whether it is the "soft" branch

I have fixed. Can you help me review again?

jjyao · 2023-07-14T04:29:32Z

python/ray/util/scheduling_strategies.py

Suggested change

if values is None or len(values) == 0:

if not values:

jjyao · 2023-07-14T04:30:23Z

python/ray/util/scheduling_strategies.py

The variadic parameter of the In operator must be a non-empty tuple: e.g. In("value1", "value2").

python/ray/util/scheduling_strategies.py

…thon Signed-off-by: LarryLian <554538252@qq.com>

pcmoritz

@jjyao The ideal error message would also include which label the error message is referring (e.g. "gpu_type" in the example) :)

jjyao · 2023-07-18T03:57:53Z

The ideal error message would also include which label the error message is referring (e.g. "gpu_type" in the example) :)

Discussed offline: currently the validation is done inside the operator constructor and we don't have the label key information. User should be able to rely on the stack trace to locate which operator is invalid without knowing the label key.

…thon (ray-project#36418) Signed-off-by: LarryLian <554538252@qq.com>

…thon (ray-project#36418) Signed-off-by: LarryLian <554538252@qq.com> Signed-off-by: NripeshN <nn2012@hw.ac.uk>

…thon (ray-project#36418) Signed-off-by: LarryLian <554538252@qq.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>

…thon (ray-project#36418) Signed-off-by: LarryLian <554538252@qq.com> Signed-off-by: Victor <vctr.y.m@example.com>

larrylian requested review from SongGuyang, jjyao, pcmoritz, scv119 and wumuzi520 June 14, 2023 15:35

larrylian self-assigned this Jun 14, 2023

larrylian force-pushed the labels_scheduling_1 branch from e96d370 to 8e53f5c Compare June 25, 2023 09:36

larrylian requested review from a team, AmeerHajAli, ericl, raulchen, robertnishihara and wuisawesome as code owners June 25, 2023 09:36

larrylian changed the title ~~[WIP][Core][Labels Scheduling]Add node affinity with labels API in python~~ [Core][Label Scheduling 1/n]Add NodeLabelSchedulingStrategy API in python Jun 25, 2023

larrylian assigned jjyao and unassigned larrylian Jun 25, 2023

larrylian force-pushed the labels_scheduling_1 branch from 8e53f5c to b2b6d13 Compare June 25, 2023 11:35

larrylian force-pushed the labels_scheduling_1 branch from b2b6d13 to 8c79eda Compare July 9, 2023 15:02

jjyao reviewed Jul 10, 2023

View reviewed changes

larrylian force-pushed the labels_scheduling_1 branch from 8c79eda to 134e094 Compare July 10, 2023 11:55

larrylian requested a review from jjyao July 10, 2023 11:55

larrylian force-pushed the labels_scheduling_1 branch 2 times, most recently from a3625e3 to db02d91 Compare July 10, 2023 11:58

jjyao reviewed Jul 10, 2023

View reviewed changes

larrylian force-pushed the labels_scheduling_1 branch from db02d91 to 0aa8344 Compare July 11, 2023 03:04

jjyao approved these changes Jul 11, 2023

View reviewed changes

larrylian force-pushed the labels_scheduling_1 branch from 0aa8344 to 93e0a61 Compare July 11, 2023 07:11

jjyao assigned pcmoritz Jul 11, 2023

pcmoritz reviewed Jul 13, 2023

View reviewed changes

python/ray/util/scheduling_strategies.py Outdated

Copy link
Copy Markdown

Contributor

pcmoritz Jul 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lable -> label

pcmoritz reviewed Jul 13, 2023

View reviewed changes

larrylian force-pushed the labels_scheduling_1 branch from 93e0a61 to 9b850dd Compare July 14, 2023 01:51

larrylian requested review from jjyao and pcmoritz July 14, 2023 01:57

jjyao reviewed Jul 14, 2023

View reviewed changes

larrylian force-pushed the labels_scheduling_1 branch 2 times, most recently from 3edceb9 to 8bb6b7c Compare July 14, 2023 08:21

jjyao reviewed Jul 14, 2023

View reviewed changes

[Core][Label Scheduling 1/n]Add NodeLabelSchedulingStrategy API in py…

6071810

…thon Signed-off-by: LarryLian <554538252@qq.com>

larrylian force-pushed the labels_scheduling_1 branch from 8bb6b7c to 6071810 Compare July 14, 2023 15:46

pcmoritz approved these changes Jul 17, 2023

View reviewed changes

jjyao merged commit 13a5391 into ray-project:master Jul 18, 2023

Bhav00 pushed a commit to Bhav00/ray that referenced this pull request Jul 28, 2023

[Core][Label Scheduling 1/n]Add NodeLabelSchedulingStrategy API in py…

e09c090

…thon (ray-project#36418) Signed-off-by: LarryLian <554538252@qq.com>

NripeshN pushed a commit to NripeshN/ray that referenced this pull request Aug 15, 2023

[Core][Label Scheduling 1/n]Add NodeLabelSchedulingStrategy API in py…

a643466

…thon (ray-project#36418) Signed-off-by: LarryLian <554538252@qq.com> Signed-off-by: NripeshN <nn2012@hw.ac.uk>

larrylian mentioned this pull request Sep 4, 2023

[Core] Enhancing node affinity scheduling feature through node labels #34894

Open

28 tasks

vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023

[Core][Label Scheduling 1/n]Add NodeLabelSchedulingStrategy API in py…

95ad4a4

…thon (ray-project#36418) Signed-off-by: LarryLian <554538252@qq.com> Signed-off-by: Victor <vctr.y.m@example.com>

-                "The value of label match expression in node label "
-                "scheduling strategy must be created by "
-                "`IN` or `NOT_IN` or`EXIST` or `DOES_NOT_EXIST`."
+                "The value of label match expression in node label "
+                "scheduling strategy must be "
+                "`In` or `NotIn` or`Exists` or `DoesNotExist` operator."

-    """The schedling strategy of node Affinity with labels
-    Simplified usage:
-    scheduling_strategy=NodeLabelSchedulingStrategy({
-          "region": IN("us"),
-          "gpu_type": EXISTS()
-    })
-    """
+    """Label based node affinity scheduling strategy
+    scheduling_strategy=NodeLabelSchedulingStrategy({
+          "region": IN("us"),
+          "gpu_type": EXISTS()
+    })
+    """

Conversation

larrylian commented Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pcmoritz commented Jul 13, 2023

Uh oh!

larrylian commented Jul 14, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pcmoritz left a comment

Choose a reason for hiding this comment

Uh oh!

jjyao commented Jul 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

larrylian commented Jun 14, 2023 •

edited

Loading