Skip to content

fix(router): await redis flush before reading back budget spend - #32844

Closed
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
litellm_fix_router_budget_sync_race
Closed

devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
litellm_fix_router_budget_sync_race

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Relevant issues

Fixes #32614

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Type

🐛 Bug Fix

Changes

RouterBudgetLimiting._sync_in_memory_spend_with_redis runs two steps in order: (1) push the queued in-memory spend increments to Redis, then (2) read the current provider spend back from Redis to refresh the in-memory cache. The push step in _push_in_memory_increments_to_redis fired the Redis write as a background task and returned immediately

if len(self.redis_increment_operation_queue) > 0:
    asyncio.create_task(
        self.dual_cache.redis_cache.async_increment_pipeline(
            increment_list=self.redis_increment_operation_queue,
        )
    )
self.redis_increment_operation_queue = []

So step 2 could read Redis before that fire-and-forget increment landed, get a stale (pre-increment) value, and then overwrite the fresher in-memory spend with it. Under load the in-memory provider spend regresses, which lets an over-budget provider slip back into routing until the next sync catches up

The fix awaits the pipeline so the write is guaranteed to complete before the read-back. The queue is snapshotted and cleared before the await so increments arriving during the flush aren't dropped

increment_list = self.redis_increment_operation_queue
self.redis_increment_operation_queue = []
if len(increment_list) > 0:
    await self.dual_cache.redis_cache.async_increment_pipeline(
        increment_list=increment_list,
    )

Added a regression test in tests/test_litellm/router_strategy/test_budget_limiter_hotpath.py that drives a full sync with a fake Redis recording event order; it asserts the increment pipeline completes (pipeline_end) before the read-back (batch_get) and that the read-back sees the just-written spend. It fails on the old fire-and-forget code and passes with the fix

Screenshots / Proof of Fix

Live proxy against a real Redis, hitting the real xAI API (real $), captured at commit 0bdeea1dec. Config wires router_settings.provider_budget_config.xai with a Redis-backed dual cache. Three real grok-4.5 calls, then the synced provider spend key in Redis matches the summed request cost with the queue fully drained (no stale overwrite)

$ redis-cli get 'provider_spend:xai:1d'   # before any request
(empty)

$ for i in 1 2 3; do curl -s http://localhost:4000/v1/chat/completions \
    -H "Authorization: Bearer sk-1234" -H "Content-Type: application/json" \
    -d '{"model":"grok-demo","messages":[{"role":"user","content":"Reply with exactly one word: pong"}],"max_tokens":16}' ; done
# request 1 usage cost ~ 0.000452
# request 2 usage cost ~ 0.000440
# request 3 usage cost ~ 0.000440

$ redis-cli get 'provider_spend:xai:1d'   # after 3 requests, post-sync
0.001332

Proxy debug log confirms each increment is pushed and the queue drains to empty on subsequent ticks

budget_limiter.py:502 - Incremented spend for provider_spend:xai:1d by 0.00043999999999999996
budget_limiter.py:534 - Pushing Redis Increment Pipeline for queue: [{'key': 'provider_spend:xai:1d', 'increment_value': 0.00043999999999999996, 'ttl': 86385}]
budget_limiter.py:534 - Pushing Redis Increment Pipeline for queue: []

Deterministic before/after on the specific ordering (the race is timing-dependent, so this is the reliable signal). Same working tree, only litellm/router_strategy/budget_limiter.py differs

Before (fire-and-forget, commit 0bdeea1dec with the fix reverted):

>       assert redis.events == ["pipeline_start", "pipeline_end", "batch_get"]
E       AssertionError: assert ['batch_get'] == ['pipeline_start', 'pipeline_end', 'batch_get']
1 failed

After (commit 0bdeea1dec):

$ LITELLM_LOCAL_MODEL_COST_MAP=True uv run --no-sync pytest \
    tests/test_litellm/router_strategy/test_budget_limiter_hotpath.py -q
10 passed

Link to Devin session: https://app.devin.ai/sessions/98a4449e087e43b1972219953850fef3

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a race condition in RouterBudgetLimiting._push_in_memory_increments_to_redis where a fire-and-forget asyncio.create_task allowed the subsequent Redis read-back to race ahead of the write, causing stale spend values to overwrite the fresher in-memory state and potentially letting over-budget providers slip back into routing.

  • The fix replaces asyncio.create_task(...) with a direct await, and correctly snapshots the queue into increment_list before clearing it so any new increments queued during the flush are not lost.
  • A regression test using a fake Redis that records event ordering (pipeline_start → pipeline_end → batch_get) verifies the guarantee deterministically without real network calls.

Confidence Score: 5/5

Safe to merge — the change is minimal, targeted, and directly addresses the documented race condition without altering any public interface or data format.

The diff is a two-line behavioural change (fire-and-forget → await) plus a queue snapshot that prevents lost increments during the flush. The logic is straightforward, the ordering guarantee is proven by the new regression test, and the change does not touch any auth, schema, or critical-path code outside the Redis sync helper.

No files require special attention.

Important Files Changed

Filename Overview
litellm/router_strategy/budget_limiter.py Converts fire-and-forget asyncio.create_task to an awaited call for the Redis increment pipeline; snapshots and clears the queue before the await to prevent dropped increments during the flush
tests/test_litellm/router_strategy/test_budget_limiter_hotpath.py Adds regression test with a fake Redis that records event order (pipeline_start → pipeline_end → batch_get), verifying the flush completes before the read-back and the read-back sees the incremented value

Reviews (1): Last reviewed commit: "fix(router): await redis flush before re..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing litellm_fix_router_budget_sync_race (0bdeea1) with litellm_internal_staging (eb7e4a5)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (4e3c437) during the generation of this report, so eb7e4a5 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@yuneng-berri
yuneng-berri deleted the branch main September 13, 2026 04:30
@yuneng-berri yuneng-berri reopened this Sep 13, 2026
@devin-ai-integration
devin-ai-integration Bot changed the base branch from litellm_internal_staging to main September 23, 2026 15:45
@devin-ai-integration
devin-ai-integration Bot requested a review from a team September 23, 2026 15:45
@github-actions

Copy link
Copy Markdown
Contributor

#32614 was fixed by #43125 on main, so this pull request is closed. Reopen it if something was missed.

@github-actions github-actions Bot closed this Sep 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Router budget sync can overwrite memory cache with stale Redis spend

2 participants