fix(proxy): retry rate-limit fallbacks from a pristine request snapshot - #40596
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes rate-limit fallback retries by preserving a pristine request snapshot before pre-call processing and rebuilding each fallback attempt from that snapshot. It retains alias-resolved fallback lookup, honors key-level fallback disabling, and restores the primary attempt’s state when retries fail or are exhausted. Regression tests cover OpenTelemetry metadata, successful and exhausted fallbacks, model aliases, and key metadata behavior Confidence Score: 5/5The PR appears safe to merge, with no outstanding correctness, security, or repository-rule violations The alias fallback issue is fixed by resolving fallback models after the first pre-call pass, and the previously disputed test-mocking finding was withdrawn after confirming the production path remains intact. The test-helper typing thread was manually resolved without explanation. No changes were made after the previous review, and no new issues were identified Important Files Changed
Reviews (5): Last reviewed commit: "fix(proxy): honor key-level disable_fall..." | Re-trigger Greptile |
…n fallback resolution Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…_limit_fallback_pristine_data
…d retry from a client-request snapshot The fallback retry in _pre_call_with_fallbacks re-entered common_processing_pre_call_logic with data already enriched by the first pass, so add_litellm_data_to_request deep-copied a metadata dict holding the live OTel span and the request failed with a 500 (cannot pickle '_thread.RLock') instead of the intended 429 or fallback. Capture the configured fallbacks and a snapshot of the client request before the first pass, look up the fallback chain by the normalized model group after the limiter raises, and run each fallback attempt on a fresh copy of that snapshot. Replaces the mock-heavy tests with a rig that runs the real v3 limiter and a live OTel span through the proxy_logging_obj seam Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ead of building a dict literal Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
Key metadata disable_fallbacks only lands on data during add_key_level_controls, so the local rate-limit fallback retry now rechecks it post pre-call. Also use a real UserAPIKeyAuth in the skip pre-call test since the path reads router_settings Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
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 9974cf4. Configure here.
|
Live audit on dynamic_rate_limiter_v3: base a8979fe returns the RLock 500, head 9974cf4 serves the fallback. Full report in the session
|
… on rate-limit fallback The local rate-limit fallback path introduced in BerriAI#40596 restores the request from a snapshot taken before the pre-call pass, so the guardrails resolved for the requested model were dropped when another deployment was selected, and the raw request-body disable_fallbacks field gated the fallback before the key-level disable_fallbacks override had been applied Carry the requested model's merged guardrail list onto every fallback pass and read the effective disable_fallbacks value after the pre-call pass Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Backport of BerriAI#40596 to rc/1.102.0. Cherry-picked from merge commit c5325b1 (main), originally by app/devin-ai-integration.
TLDR
Problem this solves:
cannot pickle '_thread.RLock' objectHow it solves it:
disable_fallbacksfrom key metadata is checked after the first pass, since that pass is what writes itUser Flow
Before: a developer whose key is rate limited on a model that has a fallback gets an opaque 500
otelinlitellm_settings.callbacksandfallbacks: [{"gpt-main": ["gpt-fallback"]}]inrouter_settingsmodel_rpm_limit: {"gpt-main": 1}"model": "gpt-main"and gets 200 from gpt-main"cannot pickle '_thread.RLock' object", no retry-after header, no fallbackAfter: the same request lands on the fallback, or returns a real 429 when the fallback is limited too
otelinlitellm_settings.callbacksandfallbacks: [{"gpt-main": ["gpt-fallback"]}]inrouter_settingsmodel_rpm_limit: {"gpt-main": 1}"model": "gpt-main"and gets 200 from gpt-mainx-litellm-model-group: gpt-fallback. If the fallback is limited too, the response is HTTP 429 withretry-after: 60and a message naming the gpt-main limitRelevant issues
Reported by a customer (Pylon #8439)
Affected release
regression in v1.93.0.dev1 (last working v1.92.2, still present through v1.101.0 and the v1.102.0-rc.1 line)
Linear ticket
Resolves LIT-7470
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
Live audit of the
dynamic_rate_limiter_v3path, one leg per side, each from its own worktree withpython -m litellm.proxy.proxy_cli --config config.yaml --port <port> --num_workers 2 --use_v2_migration_resolver, its own Postgres cluster and Redis container, real OpenAI and Anthropic calls, no mocks. Before ran the merge base a8979fe, After ran the PR tip 9974cf4, 127 graded cells in total across the three endpoints, streaming and non streaming, curl plus the OpenAI and Anthropic Python SDKs. Every 200 was matched to exactly oneLiteLLM_SpendLogsrow bylitellm_call_id. Dashboard screenshots and the annotated recording of both legs are in this comment; the full matrix with every command, response id and spend row is in the audit report attached to the Devin sessionThe limiter counts one request per pre-call, so with
rpm: 1the first request in the minute is served by the primary and every later one in that window is rejected locally. Each case below starts with that warm request. Helpers ($Uis the leg URL,$JisContent-Type: application/json,showprints status,x-litellm-model-group,retry-after, id and error message)Before (a8979fe)
/v1/chat/completions, gpt-main saturated, fallback configured
K=$(key), thenchat $KreturnsHTTP 200 group=gpt-main id=chatcmpl-EOpUdRQAaDrMSaDjEGM2pLxZHNXWgchat $KreturnsHTTP 500 err=cannot pickle '_thread.RLock' object(call id 29dbc126-cb51-4d4f-bd1e-6de803acc0d7)chat $K '"stream":true,'returnsHTTP 500 err=cannot pickle '_thread.RLock' object(call id 5234dedf-2215-46b9-96ce-d71b6cd2646f)InternalServerError: 500 cannot pickle '_thread.RLock' object/v1/messages, same key
msgs $KreturnsHTTP 200 group=gpt-fallback(call id b57bf95e-f2ef-45ff-a8b2-428d0c7f9b8a; this route did not crash on base)msgs $K '"stream":true,'returnsHTTP 200 group=gpt-fallback id=msg_d2db43f8…, stream consumed tomessage_stopHTTP 200 group=gpt-fallbackresponses/v1/responses, same key
resp $KreturnsHTTP 200 group=gpt-fallback(call id 760359f1-8232-4423-bee8-f60ea8d04104; this route did not crash on base)resp $K '"stream":true,'returnsHTTP 200 group=gpt-fallback id=resp_LhMgYxLa…, stream consumed toresponse.completedHTTP 200 group=gpt-fallbackresponses, streams end withresponse.completed/v1/chat/completions, aliased model (key
model_group_aliasalias-main -> gpt-main)K3=$(key '{"router_settings":{"model_group_alias":{"alias-main":"gpt-main"}}}'), thenchat $K3 '' gpt-mainreturnsHTTP 200 group=gpt-mainchat $K3 '' alias-mainreturnsHTTP 500 err=cannot pickle '_thread.RLock' object(call id 3e436131-e7ff-4eec-af2c-32d42de10ee7), streaming variant the same/v1/chat/completions, primary and only fallback both saturated (gpt-ex -> gpt-ex-fb)
chat $K '' gpt-exreturnsHTTP 200 group=gpt-ex,chat $K '' gpt-ex-fbreturnsHTTP 200 group=gpt-ex-fbchat $K '' gpt-exreturnsHTTP 500 err=cannot pickle '_thread.RLock' object(call id 592c8796-78f3-4c8c-af81-8f4df5ec1bde)/v1/chat/completions, key metadata
disable_fallbacks: trueKD=$(key '{"metadata":{"disable_fallbacks":true}}'), gpt-main already saturatedchat $KDreturnsHTTP 429 retry-after=60 err=Model capacity reached for gpt-main ... Model RPM: 1, Remaining: 0(no fallback attempted, same as After)36 request mixed burst (12 per endpoint, half streaming), gpt-main saturated
python burst.py base c0_controlreturns 12 xHTTP 500 cannot pickle '_thread.RLock' object(every chat completions request) and 24 xHTTP 200 group=gpt-fallback(messages and responses)verify_burst.sh base c0_controlreportsok_ids=24 missing=0 duplicated=0inLiteLLM_SpendLogsAfter (9974cf4)
/v1/chat/completions, gpt-main saturated, fallback configured
K=$(key), thenchat $KreturnsHTTP 200 group=gpt-main id=chatcmpl-EOpT0U8myTI5YIwtr2kZ1UfdA9XNzchat $KreturnsHTTP 200 group=gpt-fallback id=chatcmpl-EOpT1f0Rll6MhBq725vsPzQcN2BmL, one spend row for call id 7098c00b-62ea-4fa9-9a05-3c003e74bff8 withmodel_group=gpt-fallbackchat $K '"stream":true,'returnsHTTP 200 group=gpt-fallback id=chatcmpl-EOpT1a33wcbSeE7r1nKPFGl3cVIQw, stream ends withdata: [DONE], one spend rowHTTP 200 group=gpt-fallbackresponses (chatcmpl-EOpT8ADm…, chatcmpl-EOpT9AxS…, chatcmpl-EOpTAeCq…, chatcmpl-EOpTAfdu…), each with one spend row/v1/messages, same key
msgs $KreturnsHTTP 200 group=gpt-fallback(call id 122a85cf-c41a-4244-85f3-7d2502155ddf), one spend rowmsgs $K '"stream":true,'returnsHTTP 200 group=gpt-fallback id=msg_292b0698…, stream consumed tomessage_stop, one spend rowHTTP 200 group=gpt-fallbackresponses, each with one spend row/v1/responses, same key
resp $KreturnsHTTP 200 group=gpt-fallback(call id f34fbf66-09e6-4783-ae8a-e180e024ec04), one spend rowresp $K '"stream":true,'returnsHTTP 200 group=gpt-fallback id=resp_6K8-T-tn…, stream consumed toresponse.completed, one spend rowHTTP 200 group=gpt-fallbackresponses, streams end withresponse.completed, each with one spend row/v1/chat/completions, aliased model (key
model_group_aliasalias-main -> gpt-main)K3=$(key '{"router_settings":{"model_group_alias":{"alias-main":"gpt-main"}}}'), thenchat $K3 '' gpt-mainreturnsHTTP 200 group=gpt-mainchat $K3 '' alias-mainreturnsHTTP 200 group=gpt-fallback id=chatcmpl-EOpXEdOY7xZJ95GQuzPJDCkj0GCgB, spend row shows the request served by gpt-fallback/v1/chat/completions, primary and only fallback both saturated (gpt-ex -> gpt-ex-fb)
chat $K '' gpt-exreturnsHTTP 200 group=gpt-ex,chat $K '' gpt-ex-fbreturnsHTTP 200 group=gpt-ex-fbchat $K '' gpt-exreturnsHTTP 429 retry-after=60 err=Model capacity reached for gpt-ex ... Model RPM: 1, Remaining: 0, and the streaming variant returns the same 429/v1/chat/completions, key metadata
disable_fallbacks: trueKD=$(key '{"metadata":{"disable_fallbacks":true}}'), gpt-main already saturatedchat $KDreturnsHTTP 429 retry-after=60 err=Model capacity reached for gpt-main ... Model RPM: 1, Remaining: 0, no fallback attempted36 request mixed burst (12 per endpoint, half streaming), gpt-main saturated
python burst.py head c1_rediswithdocker stopof the leg's Redis 1.5 s in anddocker startat 12 s: 36 xHTTP 200 group=gpt-fallback,verify_burst.shreportsok_ids=36 missing=0 duplicated=0python burst.py head c2_pgpausewithkill -STOPon the leg's Postgres cluster 1.5 s in andkill -CONTat 15 s: 36 xHTTP 200 group=gpt-fallback,/health/readinessshowed"db":"disconnected"during the pause, after resumeok_ids=36 missing=0 duplicated=0python burst.py head c4_restartwithkill -TERMof the proxy parent 1.5 s in and a relaunch from the same tree: 36 xHTTP 200 group=gpt-fallback,ok_ids=36 missing=0 duplicated=0python burst.py head c3_workerkillwithkill -9of one uvicorn worker 1.5 s in: the sibling worker kept serving and uvicorn respawned the killed one, 20 xHTTP 200 group=gpt-fallback, 16 transport disconnects for requests in flight on the killed worker, and 5 of the 20 successful ids never got a spend row because the killed worker's in-memory spend queue died with it. This is what SIGKILL does to a uvicorn worker on both sides and is unrelated to the fix; recorded as a limitation, not a passType
🐛 Bug Fix
Caveats (if any)
Low
disable_fallbacks: truein the body or in key metadata, and a key-level fallback pointing at a missing model group (400 on both)router_settings.fallbacks: []means no fallbacks (429) on both base and head; onlynullor an absent key falls through to the global listmodelsallowlist unlessenforce_fallback_model_accessis set, on both base and head; predates this PR, follow-up ticket materialLEGACY_MULTI_INSTANCE_RATE_LIMITING=true) was not driven live. It raises the same error through the same seam and the retry code does not inspect the limiter, so the behavior should match, but this is inferred, not observedFinal Attestation
ran /live-pr-risk and found no regressions/backward incompatible risks
Link to Devin session: https://app.devin.ai/sessions/cdbe8de849c546a8a5c1c0f9e635ced3
Open in Devin Desktop: https://app.devin.ai/desktop/session/cdbe8de849c546a8a5c1c0f9e635ced3?variant=devin
Requested by: @yucheng-berri