[Fix] Refresh Redis TTL on counter writes, skip stale in-memory in Redis - #26829
Conversation
|
Michael Riad Zaky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
@greptile-ai |
8682528 to
b47d1e1
Compare
|
@greptile-ai |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes multi-pod budget bypass by (1) skipping stale per-pod in-memory on a clean Redis miss and reseeding from DB instead, (2) adding The DB-write-first ordering is now consistently applied across keys, users, teams, team members, and linked-budget keys. Two minor P2 findings: the Confidence Score: 5/5Safe to merge — no security bypasses or data-loss paths introduced; both findings are P2. All P1 concerns from prior review rounds (DB-before-Redis ordering, explicit TTL on reset writes) have been addressed. The two remaining findings are P2: a silent exception swallow that causes over-enforcement (not bypass) in a transient DB failure, and a sequential loop that could be batched. No P0 or P1 issues are present. litellm/proxy/common_utils/reset_budget_job.py — the
|
| Filename | Overview |
|---|---|
| litellm/caching/redis_cache.py | Adds refresh_ttl flag to async_increment; when True, calls expire() unconditionally on every write instead of only on initial creation. Logic is correct and backward-compatible. |
| litellm/caching/dual_cache.py | Threads refresh_ttl kwarg through async_increment_cache to the underlying Redis layer. Minimal, correct change. |
| litellm/proxy/proxy_server.py | Introduces redis_clean_miss flag in get_current_spend to skip stale per-pod in-memory on a clean Redis None; passes refresh_ttl=True on all spend counter increments. Correctly handles single-pod (no Redis) and multi-pod cases. |
| litellm/proxy/db/spend_counter_reseed.py | Mirrors the redis_clean_miss pattern in coalesced and uses refresh_ttl=True when warming the cache from DB. Correctly skips stale in-memory on a clean Redis miss. |
| litellm/proxy/common_utils/reset_budget_job.py | Introduces _invalidate_spend_counter and correctly orders DB-write-then-Redis-invalidate for keys, users, teams, linked-budget keys, and team members. The find_many exception swallow in reset_budget_for_litellm_team_members can leave stale Redis counters after a successful DB reset; sequential invalidation loops could be batched. |
| tests/test_litellm/caching/test_redis_cache.py | Adds two clean unit tests for refresh_ttl behaviour — one verifying expire() is called unconditionally, one verifying it is skipped on the default path. All mock-only, no real network calls. |
| tests/test_litellm/proxy/common_utils/test_reset_budget_job.py | Adds five counter-invalidation tests covering team-member, key, user, team, and linked-budget paths. All use mocks, no real network calls. |
| tests/test_litellm/proxy/test_proxy_server.py | Adds two tests verifying the redis_clean_miss behaviour in get_current_spend: clean Redis None triggers DB reseed, Redis error falls back to in-memory. Both are well-structured and mock-only. |
Reviews (2): Last reviewed commit: "Reorder counter invalidation to run afte..." | Re-trigger Greptile
b47d1e1 to
55d6dc8
Compare
|
@greptile-ai |
0a5c094 to
e8f88cd
Compare
e8f88cd to
4e26835
Compare
…ntMultiPod [Fix] Refresh Redis TTL on counter writes, skip stale in-memory in Redis
Relevant issues
Budgets can still be bypassed on multi-pod / multi-worker setups because of how caching is organized, and pods not communicating spending to eachother. This PR does a few things to enforce budgets more correctly on multi-pod:
get_current_spend and SpendCounterReseed.coalesced skip stale per-pod in-memory on a clean Redis miss; reseed from DB instead.
RedisCache.async_increment gains refresh_ttl opt-in. Spend counters now use it so the one minute TTL extends / resets when the counter is being hit, other keys keep the previous set-once behavior. This is for performance reasons (keys accessed often avoid pinging the DB too much, especially in multi-pod).
ResetBudgetJob invalidates the Redis counter alongside every DB row reset (keys, users, teams, team members, budgets-linked keys), so refreshed counters still get reset.
Linear ticket
Resolves LIT-2466.
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Before:

After:

Type
🐛 Bug Fix
🚄 Infrastructure
✅ Test
Changes
dual_cache, redis_cache, reset_budget_job, spend_counter_reseed, proxy_server