Repository navigation
fix(proxy): give spend_counter_cache its own 10000-entry in-memory cache - #42414
devin-ai-integration[bot] wants to merge 4 commits into
Conversation
The spend counter DualCache used the generic InMemoryCache default of 200 entries, so deployments with more than 200 active budget scopes evicted still-valid counters and reseeded them from lagging DB spend on the next request. A key the proxy had just rejected for being over budget could be admitted again until the batch writer flushed. Fixes #40221 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
|
There was a problem hiding this comment.
🔍 Devin Review: 2 flags
Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
|
…path and on any e2e stack Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Review for PR #42414: Give Cache Isolation & Memory Profiling
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…lass Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ation Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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 ec97ed6. Configure here.
TLDR
Problem this solves:
How it solves it:
spend_counter_cachegets its ownInMemoryCachesized to 10000 entriesSPEND_COUNTER_CACHE_MAX_SIZE; no other cache changes sizeUser Flow
Before: a key that is over its budget gets admitted again once more than 200 other keys, teams, or end users have made requests
{"max_budget": 0.000001, "models": ["claude-haiku-4-5"]}and gets back a keysk-...budget_exceeded: "Budget has been exceeded! Key=key (sk-...) Current cost: 1.3e-05, Max budget: 1e-06""user"values) send POST http://localhost:4000/v1/chat/completions through a different key and each get a 200"spend": 0.0because the batch writer has not flushed yetAfter: the over-budget key stays blocked while hundreds of other scopes take traffic
{"max_budget": 0.000001, "models": ["claude-haiku-4-5"]}and gets back a keysk-...budget_exceeded: "Budget has been exceeded! Key=key (sk-...) Current cost: 1.3e-05, Max budget: 1e-06""user"values) send POST http://localhost:4000/v1/chat/completions through a different key and each get a 200"spend": 0.0because the batch writer has not flushed yetbudget_exceededRelevant issues
Fixes #40221
Affected release
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<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 more@greptileaito re-request a review after pushing changes)Delays 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
Setup shared by both runs: one proxy worker on port 20221, Postgres 16, no Redis, real Anthropic
claude-haiku-4-5calls (max_tokens: 1),proxy_batch_write_at: 60so the DB row lags the in-memory counter long enough to see the reseedBefore (dfd8ffc)
After (1de5a82)
budget_exceededas the second oneRegression tests:
tests/test_litellm/proxy/test_proxy_server.py::test_over_budget_key_spend_survives_hundreds_of_other_scopes_incrementingdrivesincrement_spend_countersfor one key and 300 end users, then reads the key back throughget_current_spend. It fails on the merge base (key spend read back as 0.0 after 300 other scopes were charged) and passes on the tip.tests/integration/spend/test_spend_counter_scope_churn.py::test_over_budget_key_stays_blocked_while_hundreds_of_end_users_take_traffic(groupaccounting,uv run --no-sync python tests/integration/run.py accounting) starts an owned local proxy without Redis and withproxy_batch_write_at: 600against the scripted upstream, registers a model at 0.001 and 0.002 per token, generates a probe key withmax_budget: 0.06and a churn key, makes one 40-token call on the probe key (200), confirms the second call is a 422budget_exceeded, sends 250 chat completions on the churn key with 250 distinctuservalues, then asserts the probe key still gets 422budget_exceededand that the upstream saw no request for it. Withlitellm/constants.pyandlitellm/proxy/proxy_server.pyreverted to the fix commit's parent it fails at that assertion (over-budget key admitted after 250 other scopes took traffic: 200 ...) and the group passes 13 of 13 on the tip. No provider credentials or third party service are involvedAdmin UI Playground, before and after
Same rig (one worker, Postgres, no Redis, real
claude-haiku-4-5,proxy_batch_write_at: 60). Before run captured at merge base55e95c0279, after run at1de5a82e04(the laterb0a174a7and675de2f1commits only touch tests). Reproduce it with:claude-haiku-4-5, open Optional Settings, set Max Budget to0.000001, click Create Key and copy the virtual keyclaude-haiku-4-5, open Model Settings, enable Use Advanced Parameters and set Max Tokens to1hi. The first message gets a completion; sendhiagain and the Playground shows422 Budget has been exceeded"user"on each (theseq 1 250 | xargsline above); all return 200 and GET /key/info on the probe key still shows"spend": 0.0hia third time in the PlaygroundBefore (
55e95c0279): blocked after the second message, then admitted again after the churnAfter (
1de5a82e04): blocked after the second message and still blocked after the churn, DB spend still 0.04-worker base vs tip, all three provider surfaces
Same Postgres and Anthropic, both trees started with
--num_workers 4andproxy_batch_write_at: 600so the DB row stays at 0.0 for the whole churn. For each of/v1/chat/completions,/v1/messagesand/v1/responses: new key withmax_budget: 0.000001, one call (200), 12 concurrent warm calls, then 8 concurrent probes that must all be 422 before the churn counts. Churn is 1200 chat completions on another key with 1200 distinctuservalues (48 in flight, 13s). Then 12 concurrent probesMerge base
55e95c0279: probes after the churn were200 422 422 422 200 422 200 422 422 422 422 200(chat),200 200 200 422 422 422 422 422 200 422 422 422(messages),200 422 422 200 422 422 422 422 422 422 422 422(responses), DB spend 0.0 before and after. One admission per worker whose counter was evicted. A settled over-budget key was also admitted 2 of 12 times right after Postgres was paused and unpaused under a 30-request mixed burstTip
675de2f1: 12 of 12 probes 422 on all three surfaces with the same pre-check, churn and DB spend 0.0, and 8 of 8 probes 422 while Postgres was paused plus 12 of 12 after unpause. A fresh key got a 200 right after unpause, so the block came from the counter and not from the outageAlso run on both trees at 4 workers: happy paths through raw curl, the OpenAI SDK (sync, and async streaming) and the Anthropic SDK, budget enforcement for key, team, end user, per-model key budget and
/key/updateraising a budget, malformed input, unauthenticated and unknown-key requests,/key/updateduring in-flight traffic, a probe after the counter TTL and batch flush, and one worker killed under load. No base vs tip difference outside the fixed behaviour. Two defects showed identically on both trees and are left for follow-up issues:messages: 1on/v1/messagesreturns 500 instead of 400, and/key/generatewith ateam_idthat does not exist returns 200Related: #40233 proposes the same idea against the wrong base with a conflict. This PR was written independently from the issue and does not carry code from it
Type
🐛 Bug Fix
Caveats (if any)
Medium
Low
QA runbook
accountingintegration group on a local proxy, Postgres and the scripted upstream, with Redis removed from the owned proxy so the in-memory counter is the only thing keeping the key blocked while the DB row lags{"max_budget": 0.06, "models": [<model>]}(probe key) and again with{"models": [<model>]}(churn key)"type": "budget_exceeded""user"value; expect more than 200 of them to return 200budget_exceededand no upstream request (before this PR: 200)Final Attestation
ran /live-pr-risk and found no regressions/backward incompatible risks
Link to Devin session: https://app.devin.ai/sessions/ae976f398c084972ab30c6bcc600c415
Open in Devin Desktop: https://app.devin.ai/desktop/session/ae976f398c084972ab30c6bcc600c415?variant=devin
Note
Medium Risk
Touches proxy budget enforcement and in-memory spend counters; wrong sizing or eviction could still admit over-budget traffic, though scope is limited to cache configuration plus tests.
Overview
Fixes budget enforcement bypass when many distinct spend scopes (keys, teams, end users) are active:
spend_counter_cacheno longer uses the default 200-entry in-memory layer, which could evict a just-blocked key’s counter and reseed it from lagging DB spend.The proxy now wires
spend_counter_cacheto a dedicatedInMemoryCachecapped atSPEND_COUNTER_CACHE_MAX_SIZE(10,000) inconstants.py/proxy_server.py; other caches are unchanged.Regression coverage adds a unit test that floods
increment_spend_counterswith hundreds of end-user scopes then asserts key spend is still readable, plus an integration test (andcontracts.jsonmapping) that an over-budget key stays 422budget_exceededafter ~250 other end users take traffic with Redis disabled and delayed batch writes.Reviewed by Cursor Bugbot for commit ec97ed6. Bugbot is set up for automated code reviews on this repo. Configure here.