Repository navigation
[serve] Prevent routing tasks from abandoning pending requests - #66325
Conversation
Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
There was a problem hiding this comment.
Code Review
This pull request improves the request router in Ray Serve to prevent pending requests from being stranded when routing tasks are retired or replicas are removed. It ensures that a routing task finishes its owned request before being retired, cleans up completed or cancelled entries from the fulfillment queue, and triggers replacement tasks if pending requests remain. Unit tests are also added to cover these scenarios. The review feedback suggests two key improvements: simplifying the loop-break condition to trigger whenever a request is done, and cleaning up both the fulfillment and routing queues in the finally block to avoid spawning redundant routing tasks.
| # Remove completed or cancelled entries from the fulfillment queue. | ||
| while ( | ||
| self._pending_requests_to_fulfill | ||
| and self._pending_requests_to_fulfill[0].future.done() | ||
| ): | ||
| self._pending_requests_to_fulfill.popleft() |
There was a problem hiding this comment.
We should also clean up completed or cancelled entries from self._pending_requests_to_route in the finally block. If we don't, self._pending_requests_to_route might remain truthy even if all its requests are done, causing _maybe_start_routing_tasks() to spawn redundant routing tasks that will immediately exit.
We can clean up both queues in a single loop to keep the code DRY and efficient.
# Remove completed or cancelled entries from the queues.
for queue in (self._pending_requests_to_fulfill, self._pending_requests_to_route):
while queue and queue[0].future.done():
queue.popleft()There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2c0071e. Configure here.
Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
|
9f61afa makes routing tasks follow whether their request is done, rather than whether they assigned any request or exceeded the task limit. Examples:
The outer loop still enforces task retirement before taking new work. New tests:
That said, I think there are abstraction leakages between |
abrarsheikh
left a comment
There was a problem hiding this comment.
This makes two existing tests flaky. test_pow_2_request_router.py teardown asserts curr_num_routing_tasks == 0 synchronously, right after the test body returns.
Measured on this branch merged onto master, 20 runs each of test_prefer_replica_in_same_az + test_prefer_replica_in_same_az_without_prefer_node:
this PR: 8/20 runs failed teardown
master: 0/20 runs failed teardown
Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
…ests Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
| @@ -442,6 +444,7 @@ def _fulfill_next_pending_request( | |||
| while len(self._pending_requests_to_fulfill) > 0: | |||
| pr = self._pending_requests_to_fulfill.popleft() | |||
| if not pr.future.done(): | |||
| self._cancel_routing_task_for_pending_request(pr) | |||
There was a problem hiding this comment.
- is it possible for the routing task that's being canceled to be actively routing other pending requests?
- what happens to those pending requests when their routing task gets canceled?
There was a problem hiding this comment.
consider this example:
- Two requests A and B arrive
- Two tasks TA and TB which take A and B respectively
Case 1: FIFO routers (power-of-two, round-robin)
- assume there are two replicas and both are full -> TA and TB sleep in backoff.
- now, a replica frees up, and assume that TB wakes up first and finds that replica.
- FIFO gives that replica to A since it arrives first.
- A is done, so TA is cancelled mid-sleep.
- B is still pending, so TB keeps routing and serves B next.
is it possible for the routing task that's being canceled to be actively routing other pending requests?
no, because a routing task is only responsible for one request at a time. it's cancelled only when that request is fulfilled by another task or cancelled by its caller.
what happens to those pending requests when their routing task gets canceled?
their routing task won't get cancelled (consider B in this case which is handled by TB). let's say now another request C arrives. if we're below the routing task cap (2 × replicas), C gets its own task right away. if we're at the cap, C sits in _pending_requests_to_route, and TA's finally block starts a replacement task to take care of C.
Case 2: Exact-match routers (KV, prefix-aware)
- assume TA is waiting for replica selection
- A's client disconnects, so A is cancelled, and so is TA.
- TB is unaffected and serves B.
this is the right behavior. TA should only serve A because exact matching assigns a replica only to the request with the matching request ID. when TA is cancelled and requests are still waiting in _pending_requests_to_route, its finally block starts a replacement task.
…roject#66325) ## Description The target number of routing tasks decreases as requests receive assignments or replicas disappear. The bug was **treating that lower target as permission to retire a task even when it still owned an unfinished request**. This can leave requests waiting indefinitely. ### Background Serve uses background **routing tasks** to assign incoming requests to replicas. Picking up a request removes it from the routing queue; the request then waits for that task to complete its assignment. Previously, Serve could decide there were too many routing tasks and stop one **before it assigned its request**. That request was no longer queued for another task to pick up: https://github.com/ray-project/ray/blob/3f7bcbef92c55ee50f5b21638bf9af0f34fcfbe5/python/ray/serve/_private/request_router/request_router.py#L1253-L1268 For routers that match assignments to specific request IDs, another task cannot automatically finish the abandoned request: https://github.com/ray-project/ray/blob/3f7bcbef92c55ee50f5b21638bf9af0f34fcfbe5/python/ray/serve/_private/request_router/request_router.py#L1137-L1147 ### Why the default power-of-two router avoids this particular failure The power-of-two router uses FIFO matching: a routing task can assign its selected replica to the next unfinished request. For example, if task B exits before assigning request B, task A can still fulfill B, even though it originally picked up request A. https://github.com/ray-project/ray/blob/3f7bcbef92c55ee50f5b21638bf9af0f34fcfbe5/python/ray/serve/_private/request_router/request_router.py#L440-L448 ### Solution The fix keeps tasks alive until their assigned requests are fulfilled, stops tasks from selecting without a request, and ensures queued work continues. ### How we find the bug We discovered this while testing concurrent requests during a KV selection service failure. Some requests remained unassigned despite healthy replicas being available because their routing tasks had exited prematurely. Unlike power-of-two routing, KV-aware selection depends on the specific request’s tokens and cache overlap. A replica selected for request A cannot simply be reassigned to request B without invalidating that decision and its associated reservation. `KVAwareRouter` therefore uses **exact-request matching** rather than FIFO, **exposing the scheduler’s assumption that another task could fulfill an abandoned request**. ## Tests 1. `test_inflight_selection_completes`: Two tasks pick up requests A and B. A completes while B’s selection is paused. This reduces the pending-request count. The test verifies that B still receives a replica, rather than its task retired by the following because another task remains alive without an unassigned request. https://github.com/ray-project/ray/blob/3f7bcbef92c55ee50f5b21638bf9af0f34fcfbe5/python/ray/serve/_private/request_router/request_router.py#L1259-L1260 2. `test_inflight_selections_complete_after_replica_removal`: The routing-task limit decreases. 4 requests start selection with two replicas available. While their selections are paused, one replica is removed, reducing the target task count from 4 to 2. The test verifies that **all four already-owned requests** receive the surviving replica instead of excess tasks abandoning their requests. Both assert completed replica assignments, without requiring KV routing or a selection-service failure. ## Additional information > Optional: Add implementation details, API changes, usage examples, screenshots, etc. --------- Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
## Problem A routing task can assign an older FIFO request while still owning another request. After #66325, these successful assignments continued exponential backoff, slowing model composition. ## Solution Reset backoff after each successful assignment while preserving request ownership and cancellation protections. Signed-off-by: Jeffrey Wang <jeffreywang@anyscale.com>
…s when their routing task is cancelled (#66632) ## Description `test_pow_2_request_router` has been flaky on every Linux serve shard since #66325 landed. The cause is a real router bug that the test exposes. Routing tasks assign pending requests to replicas. Since #66325, a routing task is cancelled when another task fulfills its request or when the request itself is cancelled. That task is usually waiting in `_probe_queue_lens` for a queue length reply from a replica, and `asyncio.wait` does not cancel the tasks it waits on when its caller is cancelled. The probe keeps running and never calls `ray.cancel` on its `get_num_ongoing_requests` RPC. In `test_tasks_routed_fifo`, the fake replica hands out exactly one queue length reply per request the test expects to be routed. The orphaned probe consumes one of those replies, so the last request is never routed and the test hangs. This PR cancels the probe tasks when `_probe_queue_lens` is cancelled, so cancelling a routing task cancels its in-flight RPCs again. It adds `test_get_queue_len_cancelled_with_routing_task`, which cancels a request while its routing task is mid-probe and checks that the probe is cancelled. On a 4-core machine with 2 CPU stressors, `test_tasks_routed_fifo` hung in 8 of 10 full-file runs on master, 0 of 10 with the #66325 router change reverted, and 0 of 20 with this fix. --------- Signed-off-by: john.taylor <john.taylor@anyscale.com> Co-authored-by: Claude <noreply@anthropic.com>

Description
The target number of routing tasks decreases as requests receive assignments or replicas disappear. The bug was treating that lower target as permission to retire a task even when it still owned an unfinished request. This can leave requests waiting indefinitely.
Background
Serve uses background routing tasks to assign incoming requests to replicas. Picking up a request removes it from the routing queue; the request then waits for that task to complete its assignment.
Previously, Serve could decide there were too many routing tasks and stop one before it assigned its request. That request was no longer queued for another task to pick up:
ray/python/ray/serve/_private/request_router/request_router.py
Lines 1253 to 1268 in 3f7bcbe
For routers that match assignments to specific request IDs, another task cannot automatically finish the abandoned request:
ray/python/ray/serve/_private/request_router/request_router.py
Lines 1137 to 1147 in 3f7bcbe
Why the default power-of-two router avoids this particular failure
The power-of-two router uses FIFO matching: a routing task can assign its selected replica to the next unfinished request. For example, if task B exits before assigning request B, task A can still fulfill B, even though it originally picked up request A.
ray/python/ray/serve/_private/request_router/request_router.py
Lines 440 to 448 in 3f7bcbe
Solution
The fix keeps tasks alive until their assigned requests are fulfilled, stops tasks from selecting without a request, and ensures queued work continues.
How we find the bug
We discovered this while testing concurrent requests during a KV selection service failure. Some requests remained unassigned despite healthy replicas being available because their routing tasks had exited prematurely.
Unlike power-of-two routing, KV-aware selection depends on the specific request’s tokens and cache overlap. A replica selected for request A cannot simply be reassigned to request B without invalidating that decision and its associated reservation.
KVAwareRoutertherefore uses exact-request matching rather than FIFO, exposing the scheduler’s assumption that another task could fulfill an abandoned request.Tests
test_inflight_selection_completes: Two tasks pick up requests A and B. A completes while B’s selection is paused. This reduces the pending-request count. The test verifies that B still receives a replica, rather than its task retired by the following because another task remains alive without an unassigned request.ray/python/ray/serve/_private/request_router/request_router.py
Lines 1259 to 1260 in 3f7bcbe
test_inflight_selections_complete_after_replica_removal: The routing-task limit decreases. 4 requests start selection with two replicas available. While their selections are paused, one replica is removed, reducing the target task count from 4 to 2. The test verifies that all four already-owned requests receive the surviving replica instead of excess tasks abandoning their requests.Both assert completed replica assignments, without requiring KV routing or a selection-service failure.
Additional information