[Serve] Remove replicas from scheduler when receiving an ActorDiedError
#44772
Conversation
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
…ctor_error_router
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
|
|
||
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize("pow_2_scheduler", [{}], indirect=True) | ||
| async def test_replicas_actor_unavailable_error( |
There was a problem hiding this comment.
This test fails on master.
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
| # We do not catch RayActorError here because that error can be | ||
| # raised even when a replica is temporarily unavailable. | ||
| # See https://github.com/ray-project/ray/issues/44185 for details. | ||
| if isinstance(t.exception(), ActorDiedError): |
There was a problem hiding this comment.
let's also add an else block for the temporarily unavailable exception type with a more informative error message here!
There was a problem hiding this comment.
I added a more informative error message.
| # The scheduler should keep r1 since it may recover. | ||
| assert set(pow_2_scheduler.curr_replicas.values()) == {r1, r2} |
There was a problem hiding this comment.
let's test the e2e behavior instead of verifying what are basically implementation details -- swap r1 to stop raising the exception and check it can be scheduled to
There was a problem hiding this comment.
Sounds good, I checked that r1 gets scheduling requests after it recovers.
| # The scheduler should remove r1 since it died. | ||
| assert set(pow_2_scheduler.curr_replicas.values()) == {r2} |
There was a problem hiding this comment.
similar comment to below -- instead, you can schedule a bunch requests of and check that the get_queue_len call is never made to the replica that returned the actor died error
There was a problem hiding this comment.
The test now checks whether get_queue_len gets called.
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Why are these changes needed?
This change makes the
PowerOfTwoChoicesReplicaSchedulerstop routing to a replica when it receives anActorDiedError.Context: Currently, the scheduler stops routing to a replica when it receives a
RayActorError. However,RayActorErrorcan also be raised when there's network instability in the cluster, which leads to issues like #44185. #44212 recently introduced theActorDiedErrorsubclass, which only gets raised if the actor has died.Related issue number
Closes #44185.
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.test_pow_2_replica_scheduler.py.