[Bugfix][Router] loadaware: score bursts against live load, not the arrival snapshot - #1108
Open
ibrahimnd2000 wants to merge 1 commit into
Open
ibrahimnd2000 wants to merge 1 commit into
ibrahimnd2000 wants to merge 1 commit into
Conversation
…rrival snapshot route_general_request snapshots request_stats before awaiting route_request, and LoadAwareRouter.route_request awaits the controller lookup (and the instance-map refresh). A request only counts as in flight once process_request calls on_new_request. So every request of a burst was scored against the same pre-burst load, and a small shared-prefix match (e.g. a common system prompt cached on one endpoint) won every tie: a 128-request burst over three endpoints was placed 128/0/0. Re-read request_stats from the RequestStatsMonitor after the awaits, on the select path and the no-cache fallback path. Nothing awaits between the placement decision and on_new_request, so each decision now sees every earlier one; the same burst is placed 43/43/42. Without a reachable monitor the caller's snapshot is used as before. Signed-off-by: Muhammad Ibrahim <ibrahimnd2000@gmail.com>
ibrahimnd2000
requested review from
ApostaC,
Shaoting-Feng,
YuhanLiu11 and
ruizhang0101
as code owners
September 28, 2026 07:32
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a live_request_stats method to fetch up-to-date request statistics from the monitor during routing, rather than relying on stale snapshots taken at the request's arrival. This prevents concurrent request bursts from being routed to the same endpoint due to outdated load information. Corresponding unit tests have been added to verify burst routing behavior and live stats retrieval. There are no review comments, so no further feedback is provided.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Under a burst,
loadawareplaces almost every request on one endpoint.route_general_requestsnapshotsrequest_statsbefore it awaitsroute_request.LoadAwareRouter.route_requestthen awaits the controller lookup (and the instance-map refresh). A request only counts as in flight onceprocess_requestcallson_new_request. So when many requests arrive together, every one of them is scored against the same pre-burst load. The load term is equal for all endpoints, and whichever endpoint holds even a short shared prefix (for example a common system prompt) wins every placement.Impact: a GLM-5.x deployment with 3 vLLM replicas ran a benchmark cell of 128 concurrent ~32k-token prompts.
Fix
route_request, re-readrequest_statsfromrequest.app.state.request_stats_monitor(new helperLoadAwareRouter.live_request_stats). This applies to the select path and to the no-cache fallback path.on_new_request. Afterroute_requestreturns,route_general_requestruns only synchronous code untilawait anext(stream_generator), which runsprocess_requestsynchronously up toon_new_request. So each decision sees every request placed before it.Tests
New tests in
src/tests/test_loadaware_router.pyroute 128 concurrent requests throughroute_requestwith a realRequestStatsMonitor. The controller round-trip is mocked with 1–20 ms of latency, and a 512-of-32,000-token prefix is cached on one of three endpoints. As in the real request path, each placement is followed byon_new_request.Also added: unit tests for
live_request_stats, covering the live read and the fallback to the snapshot without a monitor.Four of the new tests fail on
main.pre-commit(black, isort, ruff, codespell) passes.Validation
In the deployment above, with the fix, a live 30-request burst with a shared cached system prompt was placed 11 / 10 / 9. The router's per-endpoint in-flight counts matched the engines' running requests.
Notes
route_request. Once both are merged, that fallback should also uselive_request_stats. Both PRs add a test section at the same place intest_loadaware_router.py, so whichever lands second needs a trivial rebase.