-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
fix(proxy): prevent no-Redis spend-counter reseed race #35150
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
base: litellm_internal_staging
Are you sure you want to change the base?
Changes from all commits
3f47ebe
22180b7
9f919b0
865d154
c0018b4
04833c3
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 |
|---|---|---|
|
|
@@ -8,12 +8,15 @@ | |
| deployments. This module reseeds from the authoritative DB instead. | ||
|
|
||
| A per-counter singleflight lock collapses concurrent reseeds on the same pod | ||
| to one DB query per cold-cache window. The lock dict is bounded LRU to cap | ||
| memory in long-lived deployments. | ||
| to one DB query per cold-cache window. The lock registry retains active locks | ||
| and bounds idle entries with LRU eviction in long-lived deployments. | ||
| """ | ||
|
|
||
| import asyncio | ||
| from collections import OrderedDict | ||
| from collections.abc import AsyncIterator | ||
| from contextlib import asynccontextmanager | ||
| from dataclasses import dataclass | ||
| from datetime import datetime | ||
| from typing import TYPE_CHECKING, ClassVar, Optional | ||
|
|
||
|
|
@@ -36,6 +39,12 @@ | |
| from litellm.proxy.utils import PrismaClient | ||
|
|
||
|
|
||
| @dataclass | ||
| class _CounterLock: | ||
| lock: asyncio.Lock | ||
| users: int = 0 | ||
|
|
||
|
|
||
| class SpendCounterReseed: | ||
| """ | ||
| Reseeds spend counters from the authoritative DB and warms the cache, | ||
|
|
@@ -53,23 +62,60 @@ class SpendCounterReseed: | |
| and get_tag_objects_batch(); callers pass those values as fallback_spend. | ||
| """ | ||
|
|
||
| _locks: ClassVar["OrderedDict[str, asyncio.Lock]"] = OrderedDict() | ||
| _locks: ClassVar["OrderedDict[str, _CounterLock]"] = OrderedDict() | ||
| _registry_lock: ClassVar[Optional[asyncio.Lock]] = None | ||
|
|
||
| @staticmethod | ||
| async def _get_lock(counter_key: str) -> asyncio.Lock: | ||
| def _get_registry_lock() -> asyncio.Lock: | ||
| if SpendCounterReseed._registry_lock is None: | ||
| SpendCounterReseed._registry_lock = asyncio.Lock() | ||
| async with SpendCounterReseed._registry_lock: | ||
| lock = SpendCounterReseed._locks.get(counter_key) | ||
| if lock is not None: | ||
| SpendCounterReseed._locks.move_to_end(counter_key) | ||
| return lock | ||
| lock = asyncio.Lock() | ||
| SpendCounterReseed._locks[counter_key] = lock | ||
| if len(SpendCounterReseed._locks) > SPEND_COUNTER_RESEED_LOCKS_MAX_SIZE: | ||
| SpendCounterReseed._locks.popitem(last=False) | ||
| return lock | ||
| return SpendCounterReseed._registry_lock | ||
|
|
||
| @staticmethod | ||
| def _get_or_create_lock(counter_key: str) -> _CounterLock: | ||
| counter_lock = SpendCounterReseed._locks.get(counter_key) | ||
| if counter_lock is None: | ||
| counter_lock = _CounterLock(lock=asyncio.Lock()) | ||
| SpendCounterReseed._locks[counter_key] = counter_lock | ||
| SpendCounterReseed._locks.move_to_end(counter_key) | ||
| return counter_lock | ||
|
|
||
| @staticmethod | ||
| def _prune_idle_locks() -> None: | ||
| while len(SpendCounterReseed._locks) > SPEND_COUNTER_RESEED_LOCKS_MAX_SIZE: | ||
| idle_key = next( | ||
| ( | ||
| key | ||
| for key, counter_lock in SpendCounterReseed._locks.items() | ||
| if counter_lock.users == 0 and not counter_lock.lock.locked() | ||
| ), | ||
| None, | ||
| ) | ||
| if idle_key is None: | ||
| return | ||
| SpendCounterReseed._locks.pop(idle_key) | ||
|
|
||
| @staticmethod | ||
| async def _get_lock(counter_key: str) -> asyncio.Lock: | ||
| async with SpendCounterReseed._get_registry_lock(): | ||
| counter_lock = SpendCounterReseed._get_or_create_lock(counter_key) | ||
| SpendCounterReseed._prune_idle_locks() | ||
| return counter_lock.lock | ||
|
|
||
| @staticmethod | ||
| @asynccontextmanager | ||
| async def _counter_lock(counter_key: str) -> AsyncIterator[None]: | ||
| async with SpendCounterReseed._get_registry_lock(): | ||
| counter_lock = SpendCounterReseed._get_or_create_lock(counter_key) | ||
| counter_lock.users += 1 | ||
|
Comment on lines
+109
to
+110
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.
Each lock acquisition now mutates the shared AGENTS.md reference: AGENTS.md:L1-L1 Useful? React with 👍 / 👎. |
||
| try: | ||
| lock = await SpendCounterReseed._get_lock(counter_key) | ||
| async with lock: | ||
| yield | ||
| finally: | ||
| async with SpendCounterReseed._get_registry_lock(): | ||
| counter_lock.users -= 1 | ||
| SpendCounterReseed._prune_idle_locks() | ||
|
|
||
| @staticmethod | ||
| async def from_db(prisma_client: Optional["PrismaClient"], counter_key: str) -> Optional[float]: | ||
|
|
@@ -152,8 +198,7 @@ async def coalesced( | |
| Returns the spend value (including 0.0 from a fresh budget reset) | ||
| when the DB read succeeds, or None when the DB is unavailable. | ||
| """ | ||
| lock = await SpendCounterReseed._get_lock(counter_key) | ||
| async with lock: | ||
| async with SpendCounterReseed._counter_lock(counter_key): | ||
| # Re-check after acquiring the lock. Skip in-memory on a clean | ||
| # Redis miss - in-memory is per-pod-stale. | ||
| redis_clean_miss = False | ||
|
|
@@ -197,7 +242,15 @@ async def coalesced( | |
| value=current_value, | ||
| ) | ||
| else: | ||
| await spend_counter_cache.async_increment_cache(key=counter_key, value=db_spend, refresh_ttl=True) | ||
| cached = spend_counter_cache.in_memory_cache.get_cache(key=counter_key) | ||
|
veria-ai[bot] marked this conversation as resolved.
|
||
| if cached is not None: | ||
| current_value = float(cached) | ||
|
Comment on lines
+245
to
+247
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.
When a no-Redis worker has a cold reseed waiting on the database and processes more than Useful? React with 👍 / 👎. |
||
| else: | ||
| current_value = await spend_counter_cache.async_increment_cache( | ||
| key=counter_key, | ||
| value=db_spend, | ||
| refresh_ttl=True, | ||
| ) | ||
| except Exception: | ||
| verbose_proxy_logger.exception( | ||
| "SpendCounterReseed.coalesced: failed to warm counter %s", | ||
|
|
@@ -262,8 +315,7 @@ async def coalesced_window( | |
| entity_id: str, | ||
| window_start: datetime, | ||
| ) -> Optional[float]: | ||
| lock = await SpendCounterReseed._get_lock(counter_key) | ||
| async with lock: | ||
| async with SpendCounterReseed._counter_lock(counter_key): | ||
| redis_clean_miss = False | ||
| if spend_counter_cache.redis_cache is not None: | ||
| try: | ||
|
|
||
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.
When more distinct counters are active than
SPEND_COUNTER_RESEED_LOCKS_MAX_SIZE(especially when operators lower the configurable cap), every acquisition and release scans the ordered registry while holding the event-loop registry lock; if all entries are active, each scan traverses the entire collection. A burst of slow cold reseeds therefore causes quadratic Python work and blocks unrelated counter operations on the same event loop; track idle entries separately or maintain eviction eligibility without a full scanUseful? React with 👍 / 👎.