[Core] Introduce fail_on_unavailable option for hard NodeAffinitySchedulingStrategy #36718
Conversation
…trategy Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com>
| ) | ||
| ).remote() | ||
|
|
||
| with pytest.raises(ray.exceptions.ActorUnschedulableError): |
There was a problem hiding this comment.
Should we have a better error message (and a test) in this case? I think it'd be great the exception contains a message like the task couldn't be scheduled, and _fail_on_unavailable is set to true?
There was a problem hiding this comment.
Yea, I think we should if we make it public. For now, I think it's fine to not have an error message since it's private and I will just use it in serve and I don't need to know the error message.
| a1 = Actor.remote() | ||
| target_node_id = ray.get(a1.get_node_id.remote()) | ||
|
|
||
| a2 = Actor.options( |
There was a problem hiding this comment.
Is all the combination tested actually? IIUC, the behavior is
spill: True fail: True
-> makes no sense (maybe raise an exception?)
spill:True fail:False
-> spill to other node if other node is available
spill: False fail:True
-> fail if the node is not available
spill:False fail:False
not scheduled until the node is available
can you make sure all these scenarios are tested?
There was a problem hiding this comment.
Currently invalid combinations will check failure since these are private options now and not used by users. Once we make them public, we need to throw proper exceptions. All the valid combinations are tested.
Yea, once we decide to make them public, we will definitely find a better way to structure the API. It's tracked here: #34283. For fail_on_unavailable, it's experimental for serve scheduling support and we may remove it in the future. |
…dulingStrategy (ray-project#36718) Add an experimental fail_on_unavailable option to try out application level scheduling Signed-off-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Why are these changes needed?
Add an experimental fail_on_unavailable option to try out application level scheduling
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.