Repository navigation
fix(proxy): share model rate-limit buckets between a model_group_alias and its target - #42516
Conversation
…s and its target A request sent under a model_group_alias counted in its own per-key, per-team, per-org, and per-project model bucket, so a key could double a deployment's default_api_key_rpm_limit / tpm_limit by alternating the alias and the model group name, and a metadata model_rpm_limit / model_tpm_limit keyed by the model group never applied to alias requests. The limiter now resolves the requested name to its model group before keying any model bucket, looks the limit up by the requested name first and the model group second, and charges post-call tokens to the same bucket.
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ling reserved tokens
|
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 399c727. Configure here.
`_check_summary_model_rate_limit` called `_add_team_model_rate_limit_descriptor_from_metadata` itself on top of the copy `_create_rate_limit_descriptors` appends, handing the limiter one counter as two descriptors. BerriAI#42516 consolidated the two `model_per_team` assembly sites for the pre-call path, where the repeat was charged and halved a team's configured per-model limit. It left this call site reaching for the helper separately, so the repeat moved here rather than disappearing. The repeat is harmless today only because this caller passes `read_only=True`, so the pair is read twice and nothing is incremented -- a property of the call site, not of the code it calls. Every counter consumer below charges a request once per descriptor it is given. Dropping the call also removes the `getattr` reaching for a private name and the guard that depended on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`_check_summary_model_rate_limit` called `_add_team_model_rate_limit_descriptor_from_metadata` itself on top of the copy `_create_rate_limit_descriptors` appends, handing the limiter one counter as two descriptors. BerriAI#42516 consolidated the two `model_per_team` assembly sites for the pre-call path, where the repeat was charged and halved a team's configured per-model limit. It left this call site reaching for the helper separately, so the repeat moved here rather than disappearing. The repeat is harmless today only because this caller passes `read_only=True`, so the pair is read twice and nothing is incremented -- a property of the call site, not of the code it calls. Every counter consumer below charges a request once per descriptor it is given. Dropping the call also removes the `getattr` reaching for a private name and the guard that depended on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TLDR
Problem this solves:
model_group_aliasrequest counted in its own per-key rate-limit bucketdefault_api_key_rpm_limit/tpm_limitby using the aliasmodel_rpm_limit/model_tpm_limitentries skipped alias requests entirelyHow it solves it:
<scope>:<model group>bucketUser Flow
Before: a developer whose key gets 3 requests a minute on
Kimi-K3-lab-claude-compatiblegets 3 more by sending to its aliasdefault_api_key_rpm_limit: 3on theKimi-K3-lab-claude-compatibledeployment androuter_settings.model_group_alias: {"Kimi-K3-lab-claude-compatible[1m]": "Kimi-K3-lab-claude-compatible"}"model": "Kimi-K3-lab-claude-compatible"three times inside a minute; each returns 200 andx-ratelimit-model_per_key-remaining-requestscounts down 2, 1, 0Rate limit exceeded for model_per_key: <key hash>:Kimi-K3-lab-claude-compatible. Limit type: requests. Current limit: 3, Remaining: 0"model": "Kimi-K3-lab-claude-compatible[1m]"and get 200 withx-ratelimit-model_per_key-remaining-requests: 2, a fresh bucket of 3 on the same deploymentAfter: the alias call refuses too, so the key gets 3 requests a minute on that deployment whichever name it uses
default_api_key_rpm_limit: 3on theKimi-K3-lab-claude-compatibledeployment androuter_settings.model_group_alias: {"Kimi-K3-lab-claude-compatible[1m]": "Kimi-K3-lab-claude-compatible"}"model": "Kimi-K3-lab-claude-compatible"three times inside a minute; each returns 200 andx-ratelimit-model_per_key-remaining-requestscounts down 2, 1, 0Rate limit exceeded for model_per_key: <key hash>:Kimi-K3-lab-claude-compatible. Limit type: requests. Current limit: 3, Remaining: 0"model": "Kimi-K3-lab-claude-compatible[1m]"and get the same 429 naming<key hash>:Kimi-K3-lab-claude-compatibleDesign decisions
model_rpm_limitby the alias name keeps that limit. The alternatives were model group only (silently drops such entries) and applying both (double-counts one request). Whichever entry supplies the limit, the counter is the model group's<scope>:<model group>for the key, team, org, and project model limits, the project itpm/otpm limits, and the post-call token charge, so pre-call and post-call always meet on one keyx-ratelimit-model_per_key-*headers name the model group, not the alias, so a reader can see which bucket refused.x-litellm-model-groupstill reports the requested namereserved_modelis now the resolved pair, not the raw name), and post-call reconciliation charges that group. Re-resolving the alias after the call would move the charge to a new target whenever the alias map is reloaded mid-flight, leaving the reservation stranded on the old one (Greptile's finding on the first push)model_rpm_limitentries for both the alias and the group is capped at the larger one overall, and the smaller entry refuses requests sent under its name once the shared counter passes it. The alternative, one counter per entry, is the bypass this PR removes. Key-over-team precedence is unchanged and now applies whichever spelling each scope usedRelevant issues
Affected release
Linear ticket
Resolves LIT-8242
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
Both legs run the same rig: two proxy processes per leg (instances A and B, each
--num_workers 2) on random ports, sharing one Postgres database and one Redis (general_settings.coordination_redis), booted from the same config, with the caller alternating every request between A and B on one fresh virtual key. Real upstreams (Moonshot Kimi K3 and Anthropic Claude Haiku 4.5), no mocks. Redis is flushed between legsShared config (
qa_config.yaml):Boot, per leg (
<root>is the checkout at the leg's commit):Caller, per case (
qa_leg.sh): one fresh virtual key from instance A, then the first model until it returns 429, alternating A, B, A, then the second model once on the same key. Each call prints the status line,x-litellm-model-group, thex-ratelimit-model_per_key-*headers, and the first 400 characters of the bodyBefore (88a4cbd)
cases
/v1/chat/completions RPM: Kimi K3 target, then its alias
/v1/messages RPM: Haiku alias, then its target
/v1/responses RPM: Haiku target, then its alias
/v1/chat/completions TPM: Haiku target, then its alias
After (399c727)
cases
/v1/chat/completions RPM: Kimi K3 target, then its alias
/v1/messages RPM: Haiku alias, then its target
/v1/responses RPM: Haiku target, then its alias
/v1/chat/completions TPM: Haiku target, then its alias
/v1/chat/completions RPM under the limit: alias, then target, one shared countdown
PASS: on every endpoint the alias and its target now share one per-key RPM/TPM bucket across both proxy instances
Live risk check: base 88a4cbd vs head 399c727, merge-ref b474e4fbbb
Verdict: no unintended regression found. Every base-vs-head difference below is either the shared bucket this PR exists to introduce or a pre-existing limiter behavior now applied to that bucket, and the merge-ref re-run matches the head
Same rig as the proof above on both sides (two proxies per side,
--num_workers 2each, one Postgres and one Redis shared, caller alternating A, B, A on one fresh key,PYTHONPATHpinned to each worktree), with one config addition for leg L4, a hidden dict-form alias underrouter_settings.model_group_alias:Breaking
Dependent paths that answered differently before this PR. Each is the change the ticket asks for (its "Suggested fix" names the shared
{api_key}:{resolved_model_group}bucket for the deployment default, the TPM path, and the keymodel_rpm_limit/model_tpm_limitpath)model_rpm_limitkeyed on the target, requests sent to the alias): base 200, 200, 200 with nomodel_per_teamheader at all; head 200, 200, then 429 naming<key hash>:haiku-lab-claude-compatible, withx-ratelimit-model_per_team-remaining-requestscounting 1, 0 on the alias calls. L4 (hidden dict-form alias after the target used its 3): base 200 withremaining-requests: 2, a fresh bucket; head 429 naming the target. Edge:_rate_limited_modelinside every descriptor builder (key default, key and team metadata, org, project, project itpm/otpm)x-ratelimit-model_per_key-*/model_per_team-*headers on an alias request name the model group (L1 third alias call: base...:haiku-lab-claude-compatible[1m], head...:haiku-lab-claude-compatible). A client that parses the bucket name out of the message now reads the target's name;x-litellm-model-groupstill echoes the requested name. Intended, Design decisions aboveBackward incompatible
model_rpm_limitentries for both spellings (L1b, alias 2 and target 4): base ran two budgets, 6 x 200 and the 7th 429; head runs one counter and applies each name's own limit to it: target 200 (3 left of 4), 200 (2 left), alias 429 at limit 2 (the counter already reads 2), alias 429, then target 429 at limit 4 because the two refused alias calls advanced the shared counter too (see Regression risk, pre-existing). Net: the group's total is capped by its largest configured entry and a smaller entry refuses requests sent under its name once the counter passes it. Follows from the requested-first lookup in Design decisions{"alias": 1}and key{"target": 3}, requests to the alias): base applied the team's alias entry (200, then 429 x3 namingmodel_per_team: <team>:haiku-lab-claude-compatible[1m]); head applies the key's target entry (model_per_key2, 1, 0 then 429 naming the group) and the team's alias entry never fires. The precedence order itself is unchanged (get_key_model_rpm_limit: key metadata, keymodel_max_budget, team metadata, deployment default), it is now evaluated on the resolved grouphaiku-lab-tpmat 60 tpm, one alias stream of 17 tokens then the target): the target'sx-ratelimit-model_per_key-remaining-tokensreads 34 on head and 51 on base. Edge: post-call reconciliation onreconcile_model.grouplitellm_remaining_requests_metricandlitellm_remaining_tokens_metricreadx-ratelimit-model_per_key-remaining-*from the logging payload (litellm/integrations/prometheus.py), so a gauge labeled with an alias model now reports the shared bucket's countdown instead of an alias-only one. Traced from the header values above, not driven against a live Prometheus (Not verified){model_per_key:<hash>:<alias>}to{model_per_key:<hash>:<group>}, so at upgrade an alias's in-flight window is abandoned once and the group's window takes over; during a rolling deploy old pods keep counting alias calls in the old key for at most one window (LITELLM_RATE_LIMIT_WINDOW_SIZE). Low, one-timeRegression risk
model_rpm_limit4, six calls, then the raw counter:{model_per_key:<hash>:haiku-lab-claude-compatible}:requests = 6after 4 x 200 and 2 x 429 on base and on head alike. Pre-existing, so not changed here; on the shared bucket it is what makes L1b's target calls 3 to 5 refuse. A follow-up would change the blocked branch of the Lua window scriptresolve_model_group_aliasis single-hop, so an alias whose target is itself an alias still gets its own bucket (same as base, not driven)LEGACY_MULTI_INSTANCE_RATE_LIMITING=true),dynamic_rate_limiter/dynamic_rate_limiter_v3, andmodel_max_budget_limiterkeep raw-name keying, so the alias bypass persists there. Untouched by this PR, out of scopeDependency graph
_rate_limited_model/RateLimitedModel(new) <- key default descriptor: verified live (L1, L4, L5, proof cases); key metadatamodel_rpm_limit: live (L1, L1b, L2b) plustest_model_group_alias_shares_metadata_model_rpm_bucket_with_its_targetandtest_model_rpm_limit_keyed_by_the_alias_name_still_limits_alias_requests_only; team metadata: live (L2, L2b); tpm: live (L3, proof case 4) plustest_model_group_alias_shares_deployment_default_rpm_bucket_with_its_target; org and project descriptors and project itpm/otpm: unit-tested onlymodel_group_resolver(dependency-injected, default_resolve_model_group_alias_via_proxy_router, lazyllm_routerimport): live on both proxy instances; the resolver keeps the admission-time group across an alias reload,test_success_tpm_accounting_keeps_the_admission_target_after_an_alias_reloadreconcile_model.group<- chat non-streaming (proof case 4, live), chat streaming (L3, live),/v1/messagesand/v1/responses(proof cases, live),test_success_tpm_accounting_charges_the_alias_target_bucket; batch rows, websocket per-frame descriptors, and the compaction summary check reach the same builders: unit-level onlyasync_post_call_failure_hook): live, L5x-ratelimit-model_per_key-*/model_per_team-*<- Prometheus gauges: traced; dashboardui/litellm-dashboard: no consumer of these headers or of the bucket name (grep), unaffectedtests/e2e/quota_management/ratelimit/test_model_group_alias_rate_limit_e2e.py: added, both directions, green one2e-changed-testsNot verified
/v1/batches), websocket per-frame descriptors, and the compaction summary check with an alias model, driven end to endmodel_rpm_limit/model_tpm_limitand project itpm/otpm with an alias, driven end to endMerge-ref re-run
mainmoved from 4bcdaf3 to 1373322 while this PR was open; 399c727 merges cleanly onto it (b474e4fbbb) and the proof's cases 1 and 4 re-run there match the head exactlymerge-ref b474e4fbbb: case 1 (Kimi K3 RPM, group x4 then the alias) and case 4 (haiku TPM, group x5 then the alias)
Leg output, base then head
Same seven legs on both sides (
legs.sh), status line,x-litellm-model-group, thex-ratelimit-model_per_(key|team)-*headers, and the start of any 429 detail. 200 bodies elidedbase 88a4cbd (instances :55237/58609)
head 399c727 (instances :23219/42085)
Type
🐛 Bug Fix
Caveats (if any)
proxy-infraunit shard is red at this merge sha with fournative_compactiontests ('object' object has no attribute 'db', aprisma_clientsentinel leaked by an earlier test in the shard order) and, on the second attempt only,test_component_allowlists.py::test_gateway_plus_backend_covers_full_app. All five pass in isolation at base and head, the compaction four fail identically on main's own run 35775627379, and this PR changes nothing that shard coversLEGACY_MULTI_INSTANCE_RATE_LIMITING=true,parallel_request_limiter.py) still keys its per-key model window on the raw requested name ({api_key}::{model}::{minute}), so an alias keeps its own bucket there. It is untouched here because that limiter is a deprecated opt-in the reported deployment does not run, and reworking it would widen a scoped security fix into a second limiter with its own QA rig and review roundsQA runbook
e2e-alias-rl-target(anthropic/claude-haiku-4-5,default_api_key_rpm_limit: 3) androuter_settings.model_group_alias: {"e2e-alias-rl-alias": "e2e-alias-rl-target"}(needs ANTHROPIC_API_KEY)"model": "e2e-alias-rl-target"on that key inside one minute and expect 200 on each"model": "e2e-alias-rl-alias"on the same key inside that minute and expect 429 namingmodel_per_key: <key hash>:e2e-alias-rl-target"model": "e2e-alias-rl-alias"inside one minute and expect 200 on each"model": "e2e-alias-rl-target"on the same key inside that minute and expect 429 namingmodel_per_key: <key hash>:e2e-alias-rl-targetNuance: all four calls of one test must land inside one 60 second window (
LITELLM_RATE_LIMIT_WINDOW_SIZE); the test asserts that before the fourth callFinal Attestation
The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
399c727 passes /live-pr-risk