Skip to content

fix(proxy): sort spend updates to prevent DB deadlocks - #27221

Closed
pnookala-godaddy wants to merge 1 commit into
BerriAI:litellm_internal_stagingfrom
pnookala-godaddy:worktree-fix-db-spend-sorted-only
Closed

fix(proxy): sort spend updates to prevent DB deadlocks#27221
pnookala-godaddy wants to merge 1 commit into
BerriAI:litellm_internal_stagingfrom
pnookala-godaddy:worktree-fix-db-spend-sorted-only

Conversation

@pnookala-godaddy

Copy link
Copy Markdown
Contributor

Pre-Submission checklist

  • Tests added in tests/test_litellm/ (parametrized across all 7 spend buckets)
  • PR scope isolated to a single fix

Type

🐛 Bug Fix

Changes

Problem

When multiple proxy pods flush batched spend updates concurrently, each pod wraps per-entity updates in a single Prisma transaction. Without consistent iteration order, two pods can lock overlapping rows in different orders and trigger PostgreSQL deadlocks.

Fix

Iterate entity IDs in sorted order inside each transaction so all pods acquire row locks in the same order. Applied to user, key, team, team_member, org, end_user, and tag spend paths. Prisma's batch_() issues statements sequentially within the transaction, so iteration order equals lock acquisition order.

Files changed

  • litellm/proxy/db/db_spend_update_writer.py
  • litellm/proxy/utils.py
  • tests/test_litellm/proxy/db/test_db_spend_update_writer.py

@greptile-apps

greptile-apps Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes potential PostgreSQL deadlocks in multi-pod LiteLLM deployments by wrapping each spend-bucket iteration with sorted(), ensuring all pods acquire row-level locks in the same order within each Prisma batch_() transaction. The change is minimal and the approach is sound.

  • All 7 spend buckets (user, end_user, key, team, team_member, org, tag) now iterate in sorted key order, preventing the lock-order inversion that causes deadlocks when concurrent pods process overlapping entity sets.
  • Tests are parametrized across all 7 buckets using only mocked Prisma clients, with no real network calls — consistent with the repo's test policy.

Confidence Score: 5/5

Safe to merge — the change adds only sorted() around existing .items() iterations with no logic alterations.

The fix is a small, targeted addition that does not touch any control flow, data transformation, or schema. Each modified path now sorts the same keys the same way on every pod, which is the correct mechanism to prevent row-lock ordering inversions. Tests exercise all 7 spend buckets with proper mocks and verify the iteration order directly.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/db/db_spend_update_writer.py Adds sorted() to 5 per-table transaction loops and the shared _update_entity_spend_in_db helper; change is minimal, correctly applied, and does not alter any other logic.
litellm/proxy/utils.py Adds sorted() to the end_user spend loop in ProxyUpdateSpend.update_end_user_spend; one-line change, no side-effects.
tests/test_litellm/proxy/db/test_db_spend_update_writer.py Adds a parametrized async test covering all 7 spend buckets using only mocks; verifies sorted iteration order end-to-end for each path.

Reviews (1): Last reviewed commit: "fix(proxy): sort spend updates to preven..." | Re-trigger Greptile

@VANDRANKI VANDRANKI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The analysis is correct — without consistent iteration order, two pods can acquire the same row locks in opposite sequence and deadlock. sorted() on the dict items is the canonical Python fix; it adds negligible overhead on typical batch sizes.

Prisma batch_() issuing statements sequentially within the transaction is the key invariant that makes iteration order == lock acquisition order — worth keeping that comment in the code.

Test coverage is excellent: all 7 spend buckets are parametrized and the test directly asserts that where args arrive at the batcher in sorted order. Clean PR.

Iterate user/key/team/team_member/org/end_user/tag spend dicts in sorted
order inside each Prisma transaction so concurrent pods acquire row locks
in the same order, avoiding PostgreSQL deadlocks under load.
@pnookala-godaddy
pnookala-godaddy force-pushed the worktree-fix-db-spend-sorted-only branch from 54d342d to 209bd0b Compare May 5, 2026 20:50
@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

oss-pr-review-agent-shin Bot added a commit that referenced this pull request May 6, 2026
@oss-pr-review-agent-shin

Copy link
Copy Markdown
Contributor

🤖 litellm-agent: Merged into staging branch litellm_agent_oss_staging_05_06_2026. Staging PR: #27250

@krrish-berri-2

Copy link
Copy Markdown
Contributor

PR Review: BerriAI/litellm#27221 — Fix Postgres Deadlocks via Sorted Transaction Key Ordering


🟢 Merge Confidence: 5/5 — READY TO SHIP


What This PR Does

This PR fixes a Postgres deadlock bug in the LiteLLM proxy's spend tracking system. Under high concurrency (multiple proxy pods writing spend updates simultaneously), different pods could acquire row locks in different orders across the six transaction dictionaries (user, key, team, team-member, org, end-user), creating a circular wait → deadlock condition.

The fix is elegantly simple: sort the keys before iterating each transaction dictionary before batching DB writes. This enforces a consistent lock-acquisition order across all pods, eliminating the circular wait.


Key Changes

File Change
db_spend_update_writer.py Sorted iteration applied to all 6 spend-update transaction buckets before DB write batching
proxy/utils.py Same sorted iteration applied to ProxyUpdateSpend.update_end_user_spend

Strengths ✅

  • Correct root cause fix — Sorted lock ordering is the canonical solution to this class of deadlock (Dining Philosophers / lock ordering theorem). The approach is textbook correct.
  • Minimal blast radius — Only iteration order changes; no schema changes, no new data structures, no behavioral changes to spend logic itself.
  • Comprehensive tests — 143 lines of parametrized tests covering sorted iteration order for every single transaction bucket. Good regression coverage.
  • Low coupling risk — The change doesn't touch public APIs, proxy config, or provider integrations.

Risk Signals ⚠️ (Minor)

  • Hot path touchedcommit_spend_updates_to_db and ProxyUpdateSpend.update_end_user_spend are called frequently in production. That said, sorted() on small dicts is negligibly cheap.
  • Shared utils modifiedproxy/utils.py is broadly depended upon, but the change scope is narrow.
  • No CI runs visible — Typical for this OSS repo, but means no automated gate beyond manual review and Greptile.

Verdict

This is a well-scoped, low-risk, high-correctness fix for a real production pain point. The solution follows the standard deadlock-prevention pattern (consistent global ordering of lock acquisition), is easy to reason about, and is well-tested. No concerns. Approve and merge.

oss-pr-review-agent-shin Bot added a commit that referenced this pull request May 6, 2026
fzowl pushed a commit to fzowl/litellm that referenced this pull request Jun 24, 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.

3 participants