[Serve][Cherry-Pick] Fix the max_concurrent_queries issue #32974
Conversation
sihanwang41
force-pushed
the
cherry_pick_autoscaling_fix
branch
from
8000529
to
744a94e
Compare
sihanwang41
force-pushed
the
cherry_pick_autoscaling_fix
branch
from
744a94e
to
ed31d1a
Compare
There was a problem hiding this comment.
@sihanwang41 one nit and another high level comment: we don't seem to have a test for the actual behavior reported in the github issue, can we add that? The unit test for the hash value here is useful but not comprehensive.
Please address this and my nit below on master, otherwise the change LGTM.
I am going to add a test to cover it on the master. |
|
Sorry |
|
Okay, the cherry pick is completed. Ready to be reviewed and merged |
When the longpoll client timeout happens, the all internal objects will be cleaned up. The longpoll client will try to poll again. When this happens, longpoll client will receive another object which having same information but different object id (ActorHandle) from the controller. Then after compute_iterable_delta is called, it will replace the same replica in in_flight_queries, and cause router clean up ongoing request_ref, and keep assigning new request to the replica, which will break the max_concurrent_queries parameter. Related issue number Closes ray-project#32652 Signed-off-by: Sihan Wang <[email protected]>
For the `hashable` object, __eq__ and __hash__ both need to be provided for correctness. https://docs.python.org/3.9/glossary.html#term-hashable And add tests to make sure the long poll timeout issue won't happen.
sihanwang41
force-pushed
the
cherry_pick_autoscaling_fix
branch
from
6aff3c7
to
d8efee2
Compare
sihanwang41
changed the title
sihanwang41
changed the title
cherry pick: #32772
cherry pick: #33022
Related issue number
Closes #32652
Why are these changes needed?
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.