Repository navigation
perf(proxy): request-scoped RedisBatch plan for pre-call identity, spend, rate limit and routing reads - #43403
Conversation
…end, rate limit and routing reads 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)".
|
|
|
|
| error = next((item for item in items if isinstance(item, Exception)), None) | ||
| if error is not None: | ||
| future.set_exception(error) | ||
| continue | ||
| try: | ||
| future.set_result(command.decode(items)) | ||
| except Exception as e: | ||
| future.set_exception(e) | ||
| _exit_circuit_breaker( | ||
| redis_cache._circuit_breaker, | ||
| admission, # pyright: ignore[reportPrivateUsage] # same breaker async_batch_get_cache consults | ||
| ) |
There was a problem hiding this comment.
Failed commands count as success
The pipeline returns individual command errors as result slots. This code fails the affected future but then records a circuit-breaker success and a successful batch event. Repeated command failures therefore do not count toward the breaker's failure threshold, and monitoring reports those batches as successful.
There was a problem hiding this comment.
Fixed in 7ec1f03: a per-command error now logs a Redis failure service event. Breaker stays connection-level by design, item errors are not connectivity failures
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ng auth, drop unused spend MGETs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Closing in favour of #43407, the P2 PR going forward for LIT-8882 (Yassin's call) |
TLDR
Problem this solves:
How it solves it:
RedisBatch: declare mget/get/evalsha/incrbyfloat/set/delete, onepipeline(transaction=False)RedisRequestPlan(ContextVar, per request): subsystems declare, first await runs the roundRoot cause in one sentence: each pre-call owner (auth prefetch, spend counter batch, rate limiter, routing read) issues its own Redis call even though every key is derivable before any of them awaits, so independent reads serialize into separate round trips
User Flow
Before: a developer's chat request is admitted, but the proxy spends eight serial Redis hops before it even picks a deployment
"model": "gpt-group"and"user": "perf-enduser"After: the same request is admitted with the same decisions, and the independent reads leave in one hop
Linear ticket
Resolves LIT-8882
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 more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Stacked on #43369 (base
fe94831). Same fixture bytes for both arms, real Redis at 127.0.0.1:6379, real Postgres, mock deployments so the only network in the request path is Redis. Both proxies ran from their own checkout withPYTHONPATH=<checkout>:<checkout>/enterpriseplus asitecustomize.pythat wrapsredis.asyncio.client.Redis.execute_commandandPipeline.executeand appends one line per round trip (a pipeline or a Lua call counts once). Timer jobs (_sync_in_memory_spend_with_redis) are filtered out. Config:usage-based-routing-v2withrouter_settings.redis_host,litellm_settings.cacheredis,enable_redis_auth_cache: true, a team, user, end user and key each with budgets and tpm/rpm limits{"model":"gpt-group","user":"perf-enduser","messages":[{"role":"user","content":"Reply with the single word: pong B"}],"max_tokens":5}Before (fe94831)
python litellm/proxy/proxy_cli.py --config ~/perf_rt/config_full.yaml --port 4010, one warm-up request, then:HTTP 200,"mock reply from a". Redis round trips in request order (pre-call above the line, 8 sequential hops):Total 14 round trips, 8 of them sequential pre-call. A colder run (L1 expired, auth objects re-read from Redis) measured 30
After (7ec1f03)
HTTP 200,"mock reply from b"(usage-based routing alternates deployments; same decision path). Redis round trips in request order:Pre-call: 4 sequential rounds (round 1 is two pipelines on two Redis clients, issued concurrently) against 6 on the base for the same keys (the
team_membershipGET+SET pair on the base leg is L1 state, present or absent on both arms depending on the previous request). On the earlier cold comparison the fixed build measured 18 total round trips against 30Same request three times on each proxy: every leg HTTP 200, admitted, same team/user/end-user spend keys read, routing alternated between both deployments on both arms
Admin UI on the fixed proxy (http://127.0.0.1:4011/ui, logged in with the master key): Test Key page sends
Reply with the single word: pong Btogpt-groupand getsmock reply from b; Logs page shows the fixture request as Success with teamperf-team, modelgpt-group, end userperf-enduserRegime
Warm L1 (key row cached), Redis standalone (not Cluster), two mock deployments, budgets and limits set but far from exhausted. Rejection paths (over budget, over rpm, cooled down deployment) are covered by the unit tests below, not by this live run
Type
🚄 Infrastructure
Caveats (if any)
Medium
general_settings.redis_request_pipelineand thespeculativemode from the ticket are not in this PRtwo_phaseshape only: reads in round 1, rate limit Lua after, reservation writes unchangedteam_membershipGET+SET each request is pre-existing: the spend writer deletes that key post-call, so it is cold every timeLow
RedisCacheinstance; router and cache Redis are separate clients in this config, so two concurrent pipelinesTaxonomy audit
Greptile round 1 (on 0cf3bc6, 2/5): all four findings fixed in 7ec1f03: later descriptors incremented before an earlier reject was known (serial order restored), deferred auth write-back could restore an invalidated grant (resolved during auth), per-command pipeline errors reported as success (failure event logged), unused spend MGET flushed at request exit (
RedisBatch.discardon close)Audited the full diff against blocks A-BB before pushing. Fixed: B4 (a cancelled round left declared futures pending;
executenow fails every future onBaseExceptionand re-raises non-Exception), H4 (four lines over 120 chars), deadbefore_executeAPI removed. Judged not applicable: A1/A2 (adapters only read; write-back SETs use the same codec and TTL path), B1 (non-transactional pipeline carries independent commands, no read-modify-write), C series (async_batch_get_cache_sharedsignature and behavior kept, all public APIs additive), D/Z (prefetch is best-effort warm-up, enforcement getters still fail closed on their own reads), E series (refund semantics preserved and extended to later applied groups via the Lua status slot), M1/M4 (nothing credential-bearing is logged), T8 (new tests land in covered shards). H lint: noOptional[X], no baredict/Any, no docs in source, no em dashes,grep litellm.proxy litellm/caching/redis_batch.py litellm/caching/redis_request_plan.pyempty (no cyclic import into the proxy hub)Final Attestation
Link to Devin session: https://app.devin.ai/sessions/7d8fee3bf51542e38aaad9e757595014
Open in Devin Desktop: https://app.devin.ai/desktop/session/7d8fee3bf51542e38aaad9e757595014?variant=devin
Requested by: @yassin-berriai