Repository navigation
fix(least-busy): reserve the picked deployment so a burst of requests spreads out - #47484
Conversation
… spreads out The in-flight count only went up in log_pre_api_call, which runs after selection and the pre-call hooks. Requests selected in that window all saw the same counts and landed on the same deployment (16 simultaneous requests over 4 deployments went as lopsided as 12/4/0/0 in the proxy). When the router selects for a request it is about to send, least-busy now records a reservation for the chosen deployment in the same synchronous step that reads the reservations, and log_pre_api_call turns it into the normal in-flight count. Selections without request kwargs (token counting, probes) do not reserve. A reservation whose request never reaches the API call expires after 30 s. The shared and per-worker counts and their success/failure decrements are unchanged.
|
|
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
Merging this PR will not alter performance1 warning
Comparing 
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The reservation map kept a key per model group forever, so a wildcard route with caller-chosen model names grew it without bound. Drop expired and empty entries on every write. Signed-off-by: Oliver Yu <[email protected]>
ptimizeroracle
left a comment
ptimizeroracle
left a comment
There was a problem hiding this comment.
tested e2b8bc0 with 4 local servers: 64 simultaneous requests now split 16/16/16/16 (main: all 64 on d0), unequal-load burst spreads too
 [Medium impact] The changes appear safe to merge, with no actionable new finding SummaryLeast-busy routing now reserves deployments when real calls select them and frees picks that never start. Lookup-only calls do not reserve deployments The latest change excludes No actionable new issue was established. The earlier tier-count finding was withdrawn after mateo-berri explained that its count keys match existing behavior. The timing-test finding was also withdrawn after mateo-berri explained the event handshake and mutation results Tests were inspected, not executed
Reviews (20) · Last reviewed commit: "fix(least-busy): keep realtime sessions ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Selectors that share a router cache dedupe to one registered callback, so only that instance saw log_pre_api_call. Reservations held on the other instances were never released and skewed picks for the full TTL. Storing them in the router cache's in-memory layer lets any selector release them, and the per-key TTL replaces the full-map rebuild on every write. Tests now hold the burst with a barrier instead of a sleep and use a manual clock object.
InMemoryCache keeps the TTL a live key was first given, so a reservation key that kept being rewritten still expired 30 s after its first pick and took the newer reservations with it. Drop the key before each write so the TTL counts from the latest one, and leave it deleted once it is empty. Signed-off-by: Oliver Yu <[email protected]>
…s on cache hits Least-busy now reads the model group from litellm_metadata before metadata, so /v1/decisions calls that carry caller metadata are counted in flight and release their reservation at pre-call. A response cache hit releases its reservation instead of decrementing another request's in-flight count, and Responses WebSocket turn admission selects without reserving. The reservation store is reached through a typed protocol.
… count requests that start during a Redis read Least-busy now reads a call's deployment from the routing metadata the router sets, so /v1/decisions logging params stay as on main and failed decisions calls no longer start cooldowns.
# Conflicts: # litellm/router.py
…c calls keep each other's picks Router.completion from a thread pool and sync success callbacks can read and rewrite a deployment's reservations at the same time, so one thread's pick or release could be lost.
…rovider A call that fails or returns before the provider handoff (a 400 for an unsupported param, token counting, a proxy-gate rejection without a start log, batch create) now drops its reservation instead of decrementing a count it never added. Router.select picks and batch cancel no longer reserve, since neither makes a call that logs.
…so a busy deployment never looks free
|
bugbot run |
…rst_reservation # Conflicts: # tests/unit/decisions/test_main.py
|
bugbot run |
…rst_reservation # Conflicts: # litellm/router.py # litellm/router_deprecated.py # tests/unit/test_router/test_router.py
2 similar comments
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5860094. Configure here.
mateo-berri
left a comment
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
Under least-busy routing, requests arriving together read the same in-flight counts and pile onto one deployment. This PR reserves each pick at once, so the next request sees it as busy
Details
TLDR
Problem this solves:
log_pre_api_call, so calls picked before then saw the same countsHow it solves it:
/v1/decisions,/v1/responses,/v1/messagesand/v1/ocrcalls now count like chat. Tier-router picks reserve, but their in-flight count stays under the router's name as on mainUser Flow
Before: four decisions calls sent at once all go to one deployment
routing_strategy: least-busyx-litellm-model-id, in every burstAfter: the same burst is shared across both deployments
routing_strategy: least-busyRelevant issues
Refs #39322, whose last comment reports this burst case
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or moreDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Last updated: 5860094. QA ran on 694cb4f, and the realtime case on 5860094. Changes since QA: realtime sessions no longer add in-flight counts, and one unit test waits for its helper thread before its timed join. Proof remains valid because no other case sends realtime traffic, and the realtime case ran again on both sides at 5860094. Verification:
tests/unit/router_strategy/test_least_busy.pypasses 56 of 56, and its new realtime test fails without the fixBefore is main at the merge base 97aae6c, after is 694cb4f. Every case runs real Typesafe and OpenAI calls on fresh proxies with 2 workers each, its own Redis and no database, and reads each pick from
x-litellm-model-id. Full logs, configs and scriptsRouter.completionCases that answer the same on both sides
Router.responses, 8 threads/v1/messagesbeside a long callinput_tokensdecisions, one proxy with 2 workers
python -m litellm.proxy.proxy_cli --config config.yaml --port $PORT --num_workers 2 --timeout_worker_healthcheck 60burst.sh, which sends 3 rounds of 4 parallel/v1/decisionscalls and then 4 parallel/v1/chat/completionscalls, printing each response'sx-litellm-model-idconfig.yaml and burst.sh
Raw output, before and after
Before (97aae6c):
After (694cb4f):
decisions and chat, two processes on one Redis
Two proxies on one Redis with
config-redis.yaml, odd calls to the first and even calls to the secondBefore (97aae6c):
After (694cb4f):
16 calls in one curl --parallel while 3 long calls hold wide-a, wide-b and wide-c, one proxy
u2.shstarts three long chat calls 3 s apart, then sends 16 short calls at once withcurl -ZBefore (97aae6c):
After (694cb4f):
8 SDK threads on Router.completion
threads_sdk.pybuilds a least-busyRouterover twogpt-5.4-nanodeployments and runs 8 threads at once, three timesBefore (97aae6c):
After (694cb4f):
sync Router.responses, streaming and a failed call
responses_sdk.pycallsRouter.responsesone at a time, streams one, fails one before the provider, then runs 8 threads at onceBefore (97aae6c):
After (694cb4f):
/v1/messages, uneven load and cache hits, two processes on one Redis
legs3.shsends short/v1/messagescalls beside a long one, 16 short chat calls while three long ones run, and calls after a cache hitBefore (97aae6c):
After (694cb4f):
calls that end before the provider, then calls one at a time
early400.shsends 4 calls the proxy rejects with 400, then 4 calls one at a time.counttok.shcalls/v1/responses/input_tokens, then 2 chat callsBefore (97aae6c):
After (694cb4f):
complexity router and a fallback rejected before the provider
r4legs.shcalls a complexity router one at a time, then a dead group whose fallback is rejected, then 4 chat calls one at a timeBefore (97aae6c):
After (694cb4f):
realtime, two processes, at 5860094
Steps and raw output
openai/gpt-realtime-2.1-minideployments, rt-a and rt-b, inrt-groupwithrouting_strategy: least-busy. Two proxy processes with 2 workers each share one Redis and one Postgresws://<proxy>/v1/realtime?model=rt-groupone after another, 5 s apart and alternating proxies. The first sends 10 items and the others 3GET /spend/logsMain and 5860094 serve all 4 sessions from rt-a. 6643fc2, the tip before the fix, sends sessions 2 to 4 to rt-b, because the first session's 12 client events stay counted as in flight for up to an hour. Config and scripts are the
realtime-*files in the gistBefore (97aae6c)
After (5860094)
Before the fix (6643fc2)
Observations from the run:
Type
🐛 Bug Fix
Caveats (if any)
No fresh review has read the fixes from 5cdcb89 on, because the review loop stops at four rounds
Medium
Low
x-litellm-call-idcan free each other's picksFixed since the fourth review round
get_available_deploymentand the deprecatedasync_get_available_deploymentheld a 30 s reservation with no call, and areserveargument madeRouter.selectand batch cancel raise TypeError under a custom routing strategy or a Router subclassRouter.responsesthat failed before its provider kept its pick 30 s, so eight-thread bursts split 5/3, and without a call id another call's start could free its pick early/live-pr-risk at 694cb4f, and its realtime fix at 5860094
The full walk ran at 694cb4f, after the merge with main's router refactor, and found one regression. 5860094 fixes it, and the walk of that fix plus its realtime cell ran at 5860094. Nothing else changed in product code since 694cb4f, so every other cell stands
Both sides run two proxy processes with 2 workers each, sharing one Postgres and one Redis, with requests alternating across the processes. Base is the merge base 97aae6c, head runs from a clean detached worktree. Forwarding recorders sit in front of OpenAI, Anthropic, Typesafe and Mistral on both sides, and every deployment is a real provider. Logs, configs and scripts are the
lpr-*andrealtime-*files in the gistBreaking
None at 5860094. At 694cb4f, realtime sessions on a least-busy group added one in-flight count per client event and never gave it back, so new sessions avoided that deployment for up to an hour (
lpr-c12_realtime.log,lpr-c12b_realtime_scaling.log). Realtime callslogging_obj.pre_callfor every client message, and this PR made least-busy read the deployment fromlitellm_metadata, which realtime fills, while success fires once when the session closes. 5860094 keeps_arealtimeout of least-busy counting as main does, the same rule the router's TPM tracking and budget limiter already apply. Its walk found no other producer of that call type. With 4 sessions 5 s apart, main and 5860094 used rt-a for all 4, and 6643fc2 sent sessions 2 to 4 to rt-b (realtime-*.txt)These also showed up, but main does the same, so they are not counted. The Responses WebSocket leaves one count per session (
lpr-c13_responses_ws.log). A fallback group withnum_retries: 2leaves counts behind (lpr-c15_fallback_retries.log). A synctext_completionon a group name fails with "LLM Provider NOT provided" (lpr-sdk_*_noredis_r2.log). A/vllmpass-through with a badlogit_biasanswers 500 (lpr-c14_vllm_mod_ocr.log)Backward incompatible
LeastBusyLoggingHandler.get_available_deploymentsandasync_get_available_deploymentstake newreserveandcall_idkeywords. Both default to off, so existing callers keep working. Async picks reserve by default, while sync picks,select, pass-through lookups and the deprecatedasync_get_available_deploymentdo not. One-at-a-time SDK picks match main after each of these (lpr-sdk_*_noredis_r2.log)/v1/responses,/v1/messagesand/v1/ocrare now counted too, as the TLDR says. Bursts of 4 on/v1/responseswent 2/2 three times on the head and 4/0 three times on main, and/v1/messageswent 2/2 three times against 4/0, 2/2, 4/0 (lpr-c3_c6_routes.log). OCR counts go back to 0 after successes and upstream 400s (lpr-c14_vllm_mod_ocr.log). A realtime pick holds its reservation until its session closes or 30 s pass, so a session opened while the last one is still closing goes to the other deployment, where main always picks the first. In a debugging run of 5 sessions on one single-worker proxy, the 2 that started right after the last one ended went to rt-b, and the 2 that started 5 s later went to rt-a (realtime-diag-58600945d3.txt). The recorded upstream requests for c1 to c6 match on method, path, status, header keys and count (lpr-wire_diff_c1_c6.log)With Redis, a pick uses the larger of the shared count and this worker's count, so a count leaked on one worker outweighs a shared count back at 0. This was traced in the code, not observed alone
Regression risk
These dependents sit outside an
owning_picksscope or can bypass it, and only moderation was driven. Prompt-management selection reserves with no owner on a failure before its call, in a group of exactly one deployment. Moderation has the same shape, and its bursts matched main. The Anthropic mid-stream retry picks after its scope has closed. Comma-separated models throughabatch_completion, the silent experiment and the auto-router encoder share one owner, so the first call to finish frees its siblings' picks. A Router subclass that overrides the deprecatedasync_get_available_deploymentturns reservations off for its callers. The reservation store holds at most 10,000 keys, and under enough concurrency it drops 30 s reservations before 1 h start recordsDependency graph
/v1/chat/completions/v1/decisions/v1/messages/v1/responses/v1/embeddings/v1/messages/count_tokens/v1/realtimews /v1/responses/vllm/.../v1/moderations/v1/ocrx-litellm-call-idRouterRouter.acompletionRouter/v1/messagesabatch_completion/v1/videos/v1/images/generationsOn the merged tree at 694cb4f, 7,323 unit tests passed across tests/unit/router_strategy, the router, decisions, pass-through, request processing and realtime streaming files (
lpr-unit_merged.log), and the two least-busy integration files passed 4 of 4 (lpr-integ_merged.log). At 5860094,tests/unit/router_strategy/test_least_busy.pypasses 56 of 56, and its realtime test fails without the fixNot verified
Videos were skipped for cost. Image generation, prompt management, the Anthropic mid-stream retry,
abatch_completionwith comma-separated models, the silent experiment, the auto-router encoder, arouting_strategy_argschange at runtime and 10,000-key eviction were not driven, nor were Bedrock and Vertex lazy streams. Realtime sessions open longer than the 30 s reservation were not driven. The dashboard was not opened because the PR touches noui/fileFinal Attestation
Bugbot summary