[serve] Add request_routing_timeout_s to bound replica assignment

[codechrl]

Description

_choose_replica_for_request awaits the pending request's future with no bound. A request waits with no error while every replica is at capacity. It also waits while the deployment has no replicas, for example when a scaled-to-zero deployment can't schedule a new replica. HTTPOptions.request_timeout_s applies to the HTTP proxy only and also counts processing time. A DeploymentHandle caller can bound assignment only per call, by wrapping _to_object_ref() in asyncio.wait_for and cancelling the response on timeout. _to_object_ref() returns on assignment since #65243.

This adds request_routing_timeout_s to RequestRouterConfig. It follows the same path as the backoff fields: serve.proto, AsyncioRouter, then RequestRouter. When set, a request that isn't assigned a replica within the timeout raises TimeoutError. The default None keeps the current behavior. The unset path still awaits the future directly.

• On timeout, both pending deques drop done entries from their front. The lazy cleanup in _fulfill_pending_requests never runs while there are no replicas, so timed-out requests would otherwise pile up with their args.
• __eq__ and __hash__ include the field. broadcast_deployment_config_if_changed compares configs with ==, so a timeout-only change would otherwise never reach the routers.
• The proto field is optional, so an unset value doesn't come back as 0.0.
• A retry after a replica rejects the request starts a new timeout.
• The experimental CapacityQueueRouter overrides _choose_replica_for_request, so the timeout doesn't apply to it.
• Documented in performance.md next to the router backoff settings.

Related issues

Closes #40995

Additional information

Not a duplicate: no open or closed PR references #40995. #64399 (open) adds max_request_retries for #61017. It bounds the retry loops that run after a replica rejects a request. It doesn't bound the wait inside _choose_replica_for_request. Both PRs add field 9 to RequestRouterConfig in serve.proto, so whichever merges second needs to renumber.

Tests:

• test_pow_2_request_router.py: timeout with no replicas, and with the only replica at capacity. Both fail with the request_router.py hunk reverted.
• test_config.py: proto round trip for None and 1.5, and equality. The None case fails with a non-optional proto field. The equality test fails without the __eq__ change.
• test_router.py: TestAsyncioRouterBackoffConfig also checks that the field reaches the request router. It fails with the router.py hunk reverted.

Ran with this branch's python/ray/serve on top of a Ray 2.54.1 wheel, Python 3.11:

pytest python/ray/serve/tests/unit/test_pow_2_request_router.py python/ray/serve/tests/unit/test_config.py
# 226 passed
pytest python/ray/serve/tests/unit/test_router.py -k TestAsyncioRouterBackoffConfig
# 1 passed

The rest of test_router.py fails the same way on master and on this branch in that setup, because master's Serve needs a matching core build.

Disclaimer: this PR was prepared with the assistance of an AI agent (Claude Code). All code and test changes were reviewed by the author before submission.

[gemini-code-assist]

Code Review

This pull request introduces a new configuration option, request_routing_timeout_s, allowing users to set a timeout for assigning requests to replicas in Ray Serve. Key changes include updates to RequestRouterConfig, the protobuf definition, documentation, and the request routing logic to handle and raise a timeout error when the limit is exceeded. The review feedback highlights a potential memory leak in the queue cleanup logic due to head-of-line blocking and suggests filtering the queues in-place. Additionally, it recommends raising asyncio.TimeoutError instead of the built-in TimeoutError to ensure backward compatibility with Python versions prior to 3.11, along with corresponding updates to the unit tests.

[gemini-code-assist]

The current cleanup logic only removes done/timed-out requests from the front of the queues (while queue and queue[0].future.done(): queue.popleft()). If there is a pending request at the front of the queue that has not timed out (e.g., because it has no timeout set), any timed-out requests behind it will remain in the queue indefinitely. This head-of-line blocking leads to a memory leak of timed-out requests and their associated arguments/metadata.

To prevent this memory leak, we should filter the queues in-place to remove all done/timed-out requests regardless of their position in the queue.

Suggested change

	for queue in (
	self._pending_requests_to_fulfill,
	self._pending_requests_to_route,
	):
	while queue and queue[0].future.done():
	queue.popleft()
	for queue in (
	self._pending_requests_to_fulfill,
	self._pending_requests_to_route,
	):
	active_requests = [r for r in queue if not r.future.done()]
	queue.clear()
	queue.extend(active_requests)

[codechrl]

With a timeout set, requests mostly expire in FIFO order, and each expiry pops the done prefix. 50 staggered timeouts with no replicas leave both deques empty. Entries can sit behind a pending head that has a later deadline: a retry (re-inserted by created_at with a fresh timeout), or a head from before the timeout was lowered. They stay only until that head resolves or expires. They're held indefinitely only if the head was enqueued before any timeout was set, and that head waits forever regardless. Cancelled requests already get this lazy cleanup today. Rebuilding both deques on every timeout would be O(n) per timeout under overload, so I kept the lazy pop.

[gemini-code-assist]

In Python versions prior to 3.11, asyncio.TimeoutError and the built-in TimeoutError are distinct exception classes (with different inheritance hierarchies). Since this is an asynchronous API and callers typically expect asyncio.TimeoutError when awaiting async operations, raising asyncio.TimeoutError is more idiomatic and ensures backward compatibility for callers catching asyncio.TimeoutError on Python 3.8 - 3.10.

Suggested change

	raise TimeoutError(
	f"Failed to route request to a replica of {self._deployment_id} "
	f"within {self.request_routing_timeout_s}s."
	) from None
	raise asyncio.TimeoutError(
	f"Failed to route request to a replica of {self._deployment_id} "
	f"within {self.request_routing_timeout_s}s."
	) from None

[codechrl]

Builtin TimeoutError is intentional. The HTTP/gRPC proxies map isinstance(exc, TimeoutError) to 408/DEADLINE_EXCEEDED (http_util.py, grpc_util.py), and handle.py re-raises asyncio.TimeoutError as the builtin for the same reason. On 3.10, asyncio.TimeoutError isn't a subclass of the builtin, so switching would turn the 408 into a 500.

[gemini-code-assist]

Update the test to expect asyncio.TimeoutError to match the exception raised by the request router.

Suggested change

	with pytest.raises(TimeoutError, match="Failed to route request to a replica"):
	with pytest.raises(asyncio.TimeoutError, match="Failed to route request to a replica"):

[codechrl]

The router raises the builtin TimeoutError on purpose (see the thread on request_router.py), so the test stays as is.

[gemini-code-assist]

Update the test to expect asyncio.TimeoutError to match the exception raised by the request router.

Suggested change

	with pytest.raises(TimeoutError, match="Failed to route request to a replica"):
	with pytest.raises(asyncio.TimeoutError, match="Failed to route request to a replica"):

[codechrl]

Same as above: the builtin TimeoutError is intentional, so the test stays as is.

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Timeout path leaves pending request future

High Severity

The TimeoutError handler never cancels pending_request.future, unlike the CancelledError path. Queue cleanup only drops entries whose futures are already done, so a still-pending timed-out request stays in _pending_requests_to_fulfill and _pending_requests_to_route. A later routing task can still assign a replica to that abandoned request, and the request args remain queued. On Python 3.11+, asyncio.wait_for no longer directly cancels a bare Future; it cancels the calling task via asyncio.timeout, so this is not guaranteed.

 

^{Reviewed by Cursor Bugbot for commit ecff197. Configure here.}

[codechrl]

wait_for already cancels the awaited future on timeout. 3.10/3.11 call fut.cancel(). On 3.12+ asyncio.timeout cancels the task, and Task.cancel() cancels its _fut_waiter, which is this future. Checked on 3.10-3.13: future.cancelled() is True after the timeout. Routing tasks skip done futures, so a later replica goes to a live request. test_request_routing_timeout_no_replicas relies on this when it asserts the route queue is empty.

[cursor]

None timeout breaks proto serialization

High Severity

DeploymentConfig.to_proto dumps request_routing_timeout_s as None by default and passes it into RequestRouterConfigProto. The same function already pops retry_after_s when it is None so an optional proto field is left unset. Without that, default configs can raise TypeError during proto construction, which breaks deployment config broadcast for every deployment that leaves the timeout unset.

 

^{Reviewed by Cursor Bugbot for commit ecff197. Configure here.}

[codechrl]

The proto field is optional, and the protobuf constructor treats None as unset. HasField is False on protobuf 3.20.3 (Ray's floor, cpp and python) and on 6.31.1. It doesn't raise. test_request_routing_timeout_proto_round_trip[None] covers this path.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

^{Reviewed by Cursor Bugbot for commit 0d9909b. Configure here.}

[cursor]

Proto change needs fault-tolerance review

Low Severity

⚠️ This PR modifies one or more .proto files.
Please review the RPC fault-tolerance & idempotency standards guide here:
https://github.com/ray-project/ray/tree/master/doc/source/ray-core/internals/rpc-fault-tolerance.rst

This is required by the RPC Fault Tolerance Standards Guide rule because serve.proto is in the change set.

 

^{Triggered by project rule: Bugbot Rules}

^{Reviewed by Cursor Bugbot for commit 0d9909b. Configure here.}

[codechrl]

No RPC is added or changed. The PR only adds an optional config field to the RequestRouterConfig message. An unset field reads back as None, and older readers ignore it.