[Serve] Downstream deployments over-provision when receiving Deployme…#60747
Conversation
…ntResponse arguments from slow upstream Signed-off-by: abrar <abrar@anyscale.com>
![gemini-code-assist[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an over-provisioning issue in downstream deployments by resolving request arguments before they are counted as queued. The logic change in router.py is direct and well-commented, and the new test case in test_autoscaling_policy.py effectively validates the fix. I have one suggestion to make the test even more robust against potential timing issues.
|
|
||
| # Wait for all 5 requests to be blocked at SlowUpstream (waiting on signal) | ||
| wait_for_condition(lambda: ray.get(signal.cur_num_waiters.remote()) == 5) | ||
|
|
There was a problem hiding this comment.
To make this test more robust against timing-related flakiness, it would be beneficial to add a short time.sleep() after waiting for the requests to be blocked and before asserting the number of replicas. This ensures that the autoscaler has had sufficient time to make a (potentially incorrect) scaling decision. Given upscale_delay_s is 0.2s, a sleep of 0.5s should be adequate.
| # Give the autoscaler time to potentially make a wrong decision. | |
| # A sleep duration longer than upscale_delay_s (0.2s) ensures that | |
| # we would have seen an upscale event if the fix was not effective. | |
| time.sleep(0.5) | |
ray-project#60747) fixes ray-project#60624 Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: tiennguyentony <46289799+tiennguyentony@users.noreply.github.com>
ray-project#60747) fixes ray-project#60624 Signed-off-by: abrar <abrar@anyscale.com>
ray-project#60747) fixes ray-project#60624 Signed-off-by: abrar <abrar@anyscale.com> Signed-off-by: Adel Nour <ans9868@nyu.edu>
ray-project#60747) fixes ray-project#60624 Signed-off-by: abrar <abrar@anyscale.com>
2 participants
fixes #60624