-
-
Notifications
You must be signed in to change notification settings - Fork 11.7k
Fix bugs that bypasses per-team member budget limit #26204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1908,34 +1908,102 @@ async def increment_spend_counters( | |
| ) | ||
|
|
||
|
|
||
| async def _reseed_spend_from_db(counter_key: str) -> float: | ||
| """ | ||
| Read the authoritative spend for a missing counter from the DB. The | ||
| counter_key prefix encodes the table to query: | ||
|
|
||
| spend:key:{token} -> LiteLLM_VerificationToken.spend | ||
| spend:team:{team_id} -> LiteLLM_TeamTable.spend | ||
| spend:team_member:{uid}:{tid} -> LiteLLM_TeamMembership.spend | ||
| spend:user:{user_id} -> LiteLLM_UserTable.spend | ||
| spend:org:{org_id} -> LiteLLM_OrganizationTable.spend | ||
|
|
||
| Returns 0.0 if prisma is unavailable, the row is missing, or the | ||
| key format is unrecognized. On failure, logs and returns 0.0 rather | ||
| than raising so the caller can still record the current increment. | ||
| """ | ||
| if prisma_client is None: | ||
| return 0.0 | ||
| # Per-window counters (spend:*:window:{duration}) share prefixes with | ||
| # primary counters but don't correspond to a DB row; their ambiguity | ||
| # would otherwise be silently parsed as a regular counter and miss. | ||
| if ":window:" in counter_key: | ||
| return 0.0 | ||
| try: | ||
| if counter_key.startswith("spend:key:"): | ||
| token = counter_key[len("spend:key:") :] | ||
| row = await prisma_client.db.litellm_verificationtoken.find_unique( | ||
| where={"token": token} | ||
| ) | ||
| elif counter_key.startswith("spend:team_member:"): | ||
| suffix = counter_key[len("spend:team_member:") :] | ||
| if ":" not in suffix: | ||
| return 0.0 | ||
| user_id, team_id = suffix.rsplit(":", 1) | ||
| row = await prisma_client.db.litellm_teammembership.find_unique( | ||
| where={"user_id_team_id": {"user_id": user_id, "team_id": team_id}} | ||
| ) | ||
| elif counter_key.startswith("spend:team:"): | ||
| team_id = counter_key[len("spend:team:") :] | ||
| row = await prisma_client.db.litellm_teamtable.find_unique( | ||
| where={"team_id": team_id} | ||
| ) | ||
| elif counter_key.startswith("spend:user:"): | ||
| user_id = counter_key[len("spend:user:") :] | ||
| row = await prisma_client.db.litellm_usertable.find_unique( | ||
| where={"user_id": user_id} | ||
| ) | ||
| elif counter_key.startswith("spend:org:"): | ||
| org_id = counter_key[len("spend:org:") :] | ||
| row = await prisma_client.db.litellm_organizationtable.find_unique( | ||
| where={"organization_id": org_id} | ||
| ) | ||
| else: | ||
| return 0.0 | ||
|
Michael-RZ-Berri marked this conversation as resolved.
|
||
| except Exception: | ||
| verbose_proxy_logger.exception( | ||
| "Failed to reseed spend counter %s from DB", counter_key | ||
| ) | ||
| return 0.0 | ||
| if row is None: | ||
| return 0.0 | ||
| return float(getattr(row, "spend", 0.0) or 0.0) | ||
|
Comment on lines
+1911
to
+1971
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A follow-up could either extend
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes the window budget can get exceeded once on each expiry (if the primary budget is not exceeded), but this was pre-existing and is currently out of scope / will go in another PR.
Michael-RZ-Berri marked this conversation as resolved.
|
||
|
|
||
|
|
||
| async def _init_and_increment_spend_counter( | ||
| counter_key: str, | ||
| source_cache_key: str, | ||
| increment: float, | ||
| ): | ||
| """ | ||
| Initialize counter from cached object's DB-loaded spend if not yet set, | ||
| then atomically increment in both in-memory and Redis. | ||
| Initialize counter from the authoritative DB spend value if not yet | ||
| set, then atomically increment in both in-memory and Redis. | ||
|
|
||
| On first access per pod: | ||
| 1. Check spend_counter_cache (in-memory -> Redis via DualCache for init check) | ||
| 2. If not found anywhere, read base spend from user_api_key_cache (DB-loaded object) | ||
| 1. Check spend_counter_cache (in-memory -> Redis via DualCache) | ||
| 2. If not found, reseed from the DB (`_reseed_spend_from_db`). Falls | ||
| back to the cached object's `.spend` via user_api_key_cache only | ||
| if prisma is unavailable, since that value can lag the flusher. | ||
| 3. Seed counter via async_increment_cache (not async_set_cache) to avoid a | ||
| check-then-set race: if two pods cold-start simultaneously, both may see | ||
| the counter as absent and seed it. Using increment instead of set means | ||
| the worst case is over-counting (conservative — blocks slightly early) | ||
| rather than under-counting (would allow overspend). | ||
| the counter as absent and seed it. Using increment means the worst case | ||
| is over-counting (conservative, blocks slightly early) rather than | ||
| under-counting (would allow overspend). | ||
| 4. Increment atomically (both in-memory + Redis) | ||
| """ | ||
| current = await spend_counter_cache.async_get_cache(key=counter_key) | ||
| if current is None: | ||
| source = await user_api_key_cache.async_get_cache(key=source_cache_key) | ||
| base_spend = 0.0 | ||
| if source is not None: | ||
| if isinstance(source, dict): | ||
| base_spend = source.get("spend", 0.0) or 0.0 | ||
| else: | ||
| base_spend = getattr(source, "spend", 0.0) or 0.0 | ||
| base_spend = await _reseed_spend_from_db(counter_key) | ||
| if prisma_client is None: | ||
| # Best-effort fallback when prisma is unavailable (tests or | ||
| # early-startup paths). May be stale but avoids resetting to 0. | ||
| source = await user_api_key_cache.async_get_cache(key=source_cache_key) | ||
| if source is not None: | ||
| if isinstance(source, dict): | ||
| base_spend = source.get("spend", 0.0) or 0.0 | ||
| else: | ||
| base_spend = getattr(source, "spend", 0.0) or 0.0 | ||
| if base_spend > 0: | ||
| await spend_counter_cache.async_increment_cache( | ||
| key=counter_key, value=base_spend | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be stale? I see you are caching this, but I do not see any cache invalidation on update of the budget
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but it's at most stale for a minute and there are six other stale caches in this file that are similar