Repository navigation
fix(router): await budget redis pipeline before sync reads (internal copy of #32618) - #43125
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
| flush_task: Final = asyncio.create_task(self._flush_queued_increments_with_lock(redis_cache)) | ||
| return await self._await_flush_task(flush_task) |
There was a problem hiding this comment.
Redis delays completed requests When Redis is slow, the awaited success callback waits for this write and any earlier flushes. Otherwise completed requests are delayed.
Knowledge Base Used: Observability and governance
There was a problem hiding this comment.
This callback runs on the background logging worker or a stream-end task after the response is sent, so awaiting delays spend recording, not the client
There was a problem hiding this comment.
You're right — in these invocation paths async_log_success_event runs after the response has been delivered (or in the background logging worker), so awaiting the Redis flush does not add latency to the client request. It can delay spend recording while Redis is slow, but that is the intended trade-off here to ensure the subsequent sync cannot overwrite memory with stale Redis data. I’m withdrawing the “delays completed requests” concern.
| try: | ||
| await redis_cache.async_increment_pipeline(increment_list=increment_list) | ||
| except Exception as error: | ||
| log_redis_failure(verbose_router_logger, logging.ERROR, "Error syncing in-memory cache with Redis", error) | ||
| await self._requeue_detached_increment_operations() | ||
| return False |
There was a problem hiding this comment.
Failed pipelines can double spend If Redis applies some increments before a connection failure, this non-transactional pipeline retries them all. Shared spend rises twice, potentially rejecting valid requests.
Knowledge Base Used: Routing and resilience
There was a problem hiding this comment.
A drop between send and reply replays at most the failed batch, an overcount the window reset clears; main dropped that spend, undercounting
There was a problem hiding this comment.
That’s fair. A Redis connection drop makes execution ambiguous, so re-queuing the detached batch deliberately favors conservative accounting: it can overcount a batch that Redis partially applied, but avoids the undercount/data loss in the previous fire-and-forget path, and the window reset bounds the effect. I was treating any possible duplicate as an unconditional correctness bug; given this budget-limiter trade-off, I’m withdrawing the finding.
| self.redis_increment_operation_queue = [ | ||
| RedisPipelineIncrementOperation( | ||
| key=key, | ||
| increment_value=sum(operation["increment_value"] for operation in group), | ||
| ttl=group[-1]["ttl"], | ||
| ) | ||
| for key, group in grouped_operations | ||
| ] |
There was a problem hiding this comment.
Retries mix budget windows If Redis remains unavailable across a window reset, grouping by key gives old and new spend one expiry. Expired spend can falsely exhaust the new budget.
Knowledge Base Used: Routing and resilience
There was a problem hiding this comment.
Only a Redis outage spanning a window reset replays old-window spend, bounded by what queued during the outage; main lost it entirely, undercounting
There was a problem hiding this comment.
You're right — this is limited to a Redis outage (or stalled flush) that crosses the budget-window boundary. The retained queue is bounded to increments accumulated during that outage; without this requeue, main drops those increments and undercounts spend. Since the window reset writes the new spend under the same Redis key, replaying the retained batch can temporarily mix old-window spend into the new window, but that is an outage-only edge case rather than the general behavior I implied. I’m withdrawing the broader finding.
|
bugbot run |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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 40e42ff. Configure here.
Internal copy of #32618 by @emerzon at its head d3b1ddc, pushed to an origin
litellm_branch with the commits and authors intact because CircleCI never builds a fork head. The fix is the original author's workTLDR
Problem this solves:
How it solves it:
User Flow
Before: a team running two proxy replicas behind one Redis with a provider budget gets a third completion through after the second one used the budget up
provider_budget_configwith an openaibudget_limitof 0.00017 for1dand boots two proxy instances sharing one Redis0.0003372 >= 0.00017After: the same burst is refused from the first request over the budget, on both instances
provider_budget_configand boots the same two instances429 Exceeded budget for provider openai: 0.0002248 >= 0.00017, and so does every later oneRelevant issues
Fixes #32614
Refs #20886
Backports #20888 onto
mainSupersedes #32618, whose commits this branch carries unchanged
Linear ticket
Resolves LIT-8390
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/router_strategy/test_budget_limiter_hotpath.pyandtests/unit/proxy/hooks/test_unit_test_max_model_budget_limiter.py, 90 passed at 40e42ffChanges
A flush task owns the detached batch through the Redis write and its cleanup. Cancellation waits for that task to finish, including while the flush waits for its lock and after repeated cancellation, so an applied batch is not replayed. Failed writes coalesce retained increments by spend key, keep the latest TTL, and skip the Redis read. Synchronization merges spend queued during Redis I/O under the queue lock, and the flush lock serializes overlapping syncs. The virtual-key model budget limiter initializes the inherited queue state
The merge commit d3b1ddc brings the branch up to main and removes stale LIT013 lint markers the merge made unnecessary; it changes no behavior
The commit 40e42ff sums the pending increments by key once per sync and adds that total to the Redis values, instead of scanning the whole queue once per Redis key, so the queue lock is held for a single pass; the merged spend is the same
Screenshots / Proof of Fix
Every case ran the same topology on both sides: two proxy instances (
--num_workers 2each) on two random ports, sharing one Redis 7 on 127.0.0.1:21668 that was flushed before each leg, with real OpenAI, Gemini, and Anthropic calls. Instance A got a serial burst of 17 requests at 5 per second over one keep-alive connection, then fresh connections went B, A, B, A. Each request carries a 260-item filler list so it costs a measurable amount, and asks for the single word pong withmax_tokens5. The virtual key case adds a Postgres database so/key/generateworksProvider budget config, used by the three endpoint cases:
Deployment, tag, and provider reader config:
Virtual key config (
coordination_redisis what gives the proxy's key spend counters a Redis;router_settings.redis_hostalone leaves them in memory):Request bodies (
$FILLERisitem0 item1 ... item259); the deployment, tag, and virtual key cases use the first body withmodelset toopenai-deploy,openai-tag, andopenai-chat:{"model":"openai-chat","messages":[{"role":"user","content":"Ignore this list: $FILLER. Reply with the single word pong."}],"max_tokens":5,"reasoning_effort":"none"} {"model":"gemini-responses","input":"Ignore this list: $FILLER. Reply with the single word pong.","max_output_tokens":5,"reasoning":{"effort":"minimal"}} {"model":"anthropic-messages","max_tokens":5,"messages":[{"role":"user","content":"Ignore this list: $FILLER. Reply with the single word pong."}]}The burst and the fresh connections, run once per case with
$Aand$Bset to the two instance ports,$TOKENto the master key (or the generated virtual key), and$HDRempty except-H 'x-litellm-tags: lit8390tag'on the tag case:The virtual key case first mints the key on A and then runs the burst above with it:
The provider reader case sends two gemini requests to A and then reads both instances:
Before (cbba146)
Provider budget on /v1/chat/completions
ROUTE=chat/completionsA req01 HTTP 200,A req02 HTTP 200,A req03 HTTP 200, thenHTTP 429for req04 through req17B fresh1 HTTP 429,A fresh2 HTTP 429,B fresh3 HTTP 429,A fresh4 HTTP 429Exceeded budget for provider openai: 0.0003372 >= 0.00017(18 of them): three requests got through where the budget covered twoProvider budget on /v1/responses
ROUTE=responsesand the gemini bodyA req01 HTTP 200,A req02 HTTP 200,A req03 HTTP 200, thenHTTP 429for req04 through req17HTTP 429Exceeded budget for provider gemini: 0.002133 >= 0.00107(18 of them): again one request more than the budget coveredProvider budget on /v1/messages
ROUTE=messagesand the anthropic bodyHTTP 200, no budget error at allredis-cli -p 21668 GET provider_spend:anthropic:1dreads0.0: this endpoint's spend never reaches provider budgets (see Caveats)Deployment budget on /v1/chat/completions
openai-deploybodyA req01 HTTP 200,A req02 HTTP 200,A req03 HTTP 200, thenHTTP 429for req04 through req17 and all four fresh connectionsExceeded budget for deployment model_name: openai-deploy, litellm_params.model: openai/gpt-5.6-luna, model_id: openai-deploy-id: 0.0003372 >= 1d(18 of them): one request more than the budget coveredTag budget on /v1/chat/completions
openai-tagbody and-H 'x-litellm-tags: lit8390tag'A req01 HTTP 200,A req02 HTTP 200, thenHTTP 429for req03 through req17 and all four fresh connectionsExceeded budget for tag='lit8390tag', tag_spend=0.0002248, tag_budget_limit=0.00017(19 of them)GET /provider/budgets on both instances
/v1/responsesrequests to A (A gemini1 HTTP 200,A gemini2 HTTP 200), wait 3 seconds, read both instances{"providers":{"gemini":{"budget_limit":0.00107,"time_period":"1d","spend":0.001422,"budget_reset_at":"2026-09-26T04:13:39.843304+00:00"}}}spendof0.001422with its ownbudget_reset_atVirtual key model budget on /v1/chat/completions
A req01 HTTP 200,A req02 HTTP 200,A req03 HTTP 200, thenHTTP 422for req04 through req17B fresh1 HTTP 422,A fresh2 HTTP 422,B fresh3 HTTP 422,A fresh4 HTTP 422LiteLLM Virtual Key: <hash>, key_alias: lit8390-base2-1790311139, exceeded budget for model=openai-chat(18 of them), andredis-cli -p 21668 GET virtual_key_spend:<hash>:openai-chat:1dreads0.0003372after the legAfter (40e42ff)
Provider budget on /v1/chat/completions
ROUTE=chat/completionsA req01 HTTP 200,A req02 HTTP 200, thenHTTP 429for req03 through req17B fresh1 HTTP 429,A fresh2 HTTP 429,B fresh3 HTTP 429,A fresh4 HTTP 429Exceeded budget for provider openai: 0.0002248 >= 0.00017(19 of them): the third request is refused, exactly the first one over the budgetProvider budget on /v1/responses
ROUTE=responsesand the gemini bodyA req01 HTTP 200,A req02 HTTP 200, thenHTTP 429for req03 through req17HTTP 429Exceeded budget for provider gemini: 0.00142575 >= 0.00107(19 of them)Provider budget on /v1/messages
ROUTE=messagesand the anthropic bodyHTTP 200, identical to Beforeredis-cli -p 21668 GET provider_spend:anthropic:1dreads0.0again, so this endpoint is unchanged by the PRDeployment budget on /v1/chat/completions
openai-deploybodyA req01 HTTP 200,A req02 HTTP 200, thenHTTP 429for req03 through req17 and all four fresh connectionsExceeded budget for deployment model_name: openai-deploy, litellm_params.model: openai/gpt-5.6-luna, model_id: openai-deploy-id: 0.0002248 >= 1d(19 of them): the third request is refusedTag budget on /v1/chat/completions
openai-tagbody and-H 'x-litellm-tags: lit8390tag'A req01 HTTP 200,A req02 HTTP 200, thenHTTP 429for req03 through req17 and all four fresh connections, identical to BeforeExceeded budget for tag='lit8390tag', tag_spend=0.0002248, tag_budget_limit=0.00017(19 of them)GET /provider/budgets on both instances
/v1/responsesrequests to A (A gemini1 HTTP 200,A gemini2 HTTP 200), wait 3 seconds, read both instances{"providers":{"gemini":{"budget_limit":0.00107,"time_period":"1d","spend":0.00142575,"budget_reset_at":"2026-09-26T04:17:51.854185+00:00"}}}spendof0.00142575with its ownbudget_reset_atVirtual key model budget on /v1/chat/completions
A req01 HTTP 200,A req02 HTTP 200,A req03 HTTP 200, thenHTTP 422for req04 through req17, identical to BeforeB fresh1 HTTP 422,A fresh2 HTTP 422,B fresh3 HTTP 422,A fresh4 HTTP 422LiteLLM Virtual Key: <hash>, key_alias: lit8390-tip2-1790311243, exceeded budget for model=openai-chat(18 of them), andredis-cli -p 21668 GET virtual_key_spend:<hash>:openai-chat:1dreads0.0003372after the legThe three provider budget cases also ran on this branch merged into main at 0d47347 (merged tree 24c7a0abf4) with the same result as After: chat and responses refuse from the third request (19 x
openai: 0.0002248 >= 0.00017, 19 xgemini: 0.0014295 >= 0.00107), messages all 200. Main has moved by three commits since, none touching the router, the budget limiter, or the caches. The legacy CircleCI suitetests/local_testing/test_router_budget_limiter.pyreads 12 passed, 1 failed at both cbba146 and 40e42ff, the one failure being the Azure case in CaveatsType
Bug Fix
Test
Caveats (if any)
Low
0.0002248 >= 1d), unchanged main code, tracked in LIT-8634BaseRoutingStrategy, which still clears the queue when a Redis write fails; that path serves rpm and tpm counters, not budgets, and is out of scope heretests/local_testing/test_router_budget_limiter.py::test_provider_budgets_e2e_testfails identically at cbba146 and 40e42ff with an AzureNotFoundErrorfor a deployment the test account no longer has, before any budget code runsrun-cilabel, which is why this copy existsunit: twotest_auth_checks::test_get_end_user_objectcases, tentest_jwt_key_mappingtests, and eleven load-sensitive tests outside the budget code (fiveoff_the_event_looptiming checks, the threetest_jev_logging_failure_preserves_verdictcases, twotest_call_mcp_toolcases, andtest_path_source_peak_stays_below_list_pipeline), all red on main pipelines 90363 and 90381 (unit jobs 2214837 and 2215836) and all passing in isolation at cbba146 and 40e42fflocal_testing_part1:test_get_model_info_bedrock_cross_region_capability_parity, red on main 90363 and 90381;test_get_model_info_bedrock_regioncleared on the rerunlocal_testing_part2: green on the rerun, sotest_openai_stream_options_call_text_completionwas a flakebuild_and_test: twotest_keys::test_key_model_listcases, red on main 90363 and fixed there by 1d51a8d (fix(proxy): authorize key model aliases the same way as team aliases #43049), which is newer than the merge basebuild_and_test: twotest_rust_python_harnessOCR cases, whose modules e2302be (refactor(ocr): remove the Python OCR execution path and require the Rust route #43081) removed on main while the parametrization still lists them, red on main 90381 toousing_litellm_on_windows: the job fails with zero failing tests on both runs, same as main 90363 and 90381Final Attestation
Regression tests exercise the changed behavior and identified edge cases
40e42ff passes /live-pr-risk