Repository navigation
chore(release): backport #40639 to stable/1.101.x - #42635
Conversation
The cascade zeroed end-user spend with a single update_many whose where clause enumerated every dependent user id. Prisma compiles that IN-list into one prepared statement carrying one bind variable per customer, and PostgreSQL caps a statement at 32,767 of them. Once a shared budget had more dependents than that the statement could not be parsed at all, so the atomic cascade rolled back, budget_reset_at never advanced, and the tier stayed due on every later tick forever. Customers sitting at their cap were blocked indefinitely with only a recurring log line to show for it. End users now match on budget_id like every other gated table, plus a NULL-budget_id branch for the implicitly created rows that carry no link and ride the default tier. The statement's bind count now tracks the number of expiring tiers rather than the customer population, so a reset costs the same whether a budget has ten dependents or a million. Fixes #40564 Claude-Session: https://claude.ai/code/session_01Hn5E8Jz1LjGLFyiYxBRcBW (cherry picked from commit 760043b)
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| assert [_bind_count(write["where"]) for write in writes] == [2], ( | ||
| f"the cascade must not enumerate {population} user ids: past " | ||
| f"{_POSTGRES_MAX_BIND_VARIABLES} binds PostgreSQL refuses the statement, got {writes[:1]}" | ||
| ) | ||
| assert _batch_writes(mock_prisma_client, "budget")[0]["data"]["budget_reset_at"] is not None |
There was a problem hiding this comment.
This only inspects recorded predicates, violating the requirement to test behavior. Add execution-based coverage before merging
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| _POSTGRES_MAX_BIND_VARIABLES: Final = 32767 | ||
|
|
||
|
|
||
| def _bind_count(where: Dict[str, Any]) -> int: |
There was a problem hiding this comment.
The new parameter uses Dict[str, Any], violating the requirement for specific, fully typed parameters. This must be corrected before merging
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| """End users reset on the budget link like every other gated table, plus a | ||
| NULL-budget_id branch: rows created implicitly persist no link and ride the | ||
| default tier (litellm.max_end_user_budget_id). | ||
|
|
||
| Matching on the link rather than enumerating user ids keeps a statement's | ||
| bind count proportional to the expiring tiers instead of the customer | ||
| population, which past ~32,700 dependents exceeds PostgreSQL's per-statement | ||
| bind ceiling and wedges the cascade permanently (#40564). | ||
| """ |
There was a problem hiding this comment.
This adds lengthy incident history, violating the requirement that necessary complex-logic comments remain concise. Shorten it before merging
| """End users reset on the budget link like every other gated table, plus a | |
| NULL-budget_id branch: rows created implicitly persist no link and ride the | |
| default tier (litellm.max_end_user_budget_id). | |
| Matching on the link rather than enumerating user ids keeps a statement's | |
| bind count proportional to the expiring tiers instead of the customer | |
| population, which past ~32,700 dependents exceeds PostgreSQL's per-statement | |
| bind ceiling and wedges the cascade permanently (#40564). | |
| """ |
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
yuneng-berri
left a comment
yuneng-berri
left a comment
There was a problem hiding this comment.
Verified: patch-id matches upstream 760043b, CI green. Greptile P2s target upstream code, kept verbatim for the backport
2 participants
TLDR
Problem this solves:
How it solves it:
User Flow
Before: an operator with a shared budget tier linked to more than 32,767 customers sees the tier's reset never land
{"budget_id": "shared-tier", "max_budget": 10, "budget_duration": "1d"}and get 200"budget_id": "shared-tier", each returning 200Budget has been exceededon POST https://litellm-domain/v1/chat/completionsspendis still at the cap; the same 429 keeps coming back on every later day, with only a recurring reset error in the proxy logAfter: the same tier resets on schedule no matter how many customers share it
{"budget_id": "shared-tier", "max_budget": 10, "budget_duration": "1d"}and get 200"budget_id": "shared-tier", each returning 200Budget has been exceededon POST https://litellm-domain/v1/chat/completionsspend: 0and POST https://litellm-domain/v1/chat/completions returns 200 againRelevant issues
Backport of #40639 (fixes #40564) onto stable/1.101.x. One pick, cherry-picked with
-xfrom 760043b, the PR's single commit under merge 8a4fae0 on main. Patch-id check against the source is VERBATIM, no adaptation. Every referenced symbol (_SPENT_ROWS_WHERE, theextraparameter on_queue_budget_linked_resets) already exists on the lineWhat is included:
Not included on purpose: #41488 (paged end-user cache invalidation after a reset) is a later perf follow-up on the same surface. It is not needed for this fix to work and was left out to keep the backport to the one requested change. The existing 1.101.x backport commits on this branch's base are untouched
No version bump. 1.101.1 has not shipped: no Docker Hub or GHCR image, no PyPI release, no tag, no release branch, so this pick rides the pending 1.101.1
Known noise on this line:
ruff checkon the touched test file reports one new UP006 (Dictvsdict) from the picked test helper, the same pattern as 52 pre-existing hits in that file and identical to what main carries. The line'sruff format --checkis clean on the touched moduleAffected release
Linear ticket
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 proxy on localhost:4000 against a local PostgreSQL and real OpenAI calls (gpt-4.1-nano), same script run twice: Before at the merge base 4b432a9 (origin/stable/1.101.x), After at this PR's tip 173f71d. Config:
master_key: sk-1234,proxy_budget_rescheduler_min_time: 10,proxy_budget_rescheduler_max_time: 15. The tier has 40,000 customers; one is created through the API, the other 39,999 are seeded with a single SQL insert because that manyPOST /customer/newcalls is the only part a reviewer would not want to curlBefore (merge base), 60 seconds later, after the 30s window expired and the reset job ran three times:
After (this PR's tip), same 60 seconds later:
Targeted test delta on the line: baseline at 4b432a9 147 passed across
tests/test_litellm/proxy/common_utils/test_reset_budget_job.pyandtests/litellm_utils_tests/test_proxy_budget_reset.py; post-pick 150 passed, 0 new failures. The three new tests are the pick's own regression tests for #40564, including the 40,000 customer case. The fulltests/test_litellmsuite is left to CI: the local venv is synced to main and 11 unrelated modules fail collection on this line either wayGauntlet-mini over the final tree (five lenses: correctness, dependents, blind black-box, contract and backward compatibility, conventions and tests): SURVIVED, no confirmed findings. The black-box lens is the live replay above. Strongest rejected dissent: the new writes filter on
spend > 0where the old enumeration did not, so linked rows at zero spend are no longer touched; rejected becausespendisFloat @default(0.0)and non-null inschema.prisma, so zeroing a zero is a no-op and the observable state is identical. The NULL-budget_id branch is gated on the samemax_end_user_budget_id in budget_idscondition as the existing NULL-row read in_collect_endusers_to_reset, so the two stay symmetricType
🐛 Bug Fix
Caveats (if any)
Low
Final Attestation
Link to Devin session: https://app.devin.ai/sessions/f446cd66c6ef417192eb2922ac02b43a
Open in Devin Desktop: https://app.devin.ai/desktop/session/f446cd66c6ef417192eb2922ac02b43a?variant=devin