diff --git a/litellm/proxy/hooks/tag_rate_limiter.py b/litellm/proxy/hooks/tag_rate_limiter.py index 96645f4b600..15ed2ae4f1e 100644 --- a/litellm/proxy/hooks/tag_rate_limiter.py +++ b/litellm/proxy/hooks/tag_rate_limiter.py @@ -579,6 +579,13 @@ def _classify_check( ) +def _bucket_ttl_seconds(entry: TagRateLimitEntry) -> int: + """Redis (and in-memory fallback) TTL for a non-concurrency bucket key. + `entry.key_ttl_seconds` overrides the default of period_seconds + 3600 + when set -- see TagRateLimitEntry.key_ttl_seconds.""" + return entry.key_ttl_seconds if entry.key_ttl_seconds is not None else entry.period_seconds + 3600 + + def _increment_operation_for_limit( configured_limit: _ConfiguredLimit, model_group: str, @@ -606,10 +613,32 @@ def _increment_operation_for_limit( return RedisPipelineIncrementOperation( key=key, increment_value=increment_value, - ttl=configured_limit.entry.period_seconds + 3600, + ttl=_bucket_ttl_seconds(configured_limit.entry), ) +def _resolve_max_in_memory_cache_size() -> int | None: + """ + `litellm_settings` values reach `litellm.tag_rate_limiter_max_in_memory_cache_size` + via a plain, unvalidated `setattr`, so a config typo (a negative number, or a + string like "500" from an unresolved os.environ/ substitution) can reach here. + InMemoryCache raises when comparing its size against a non-positive-int + max_size_in_memory, and DualCache.async_set_cache swallows that exception, so + an invalid value would otherwise silently disable every counter write for this + hook rather than fail loudly -- rejected here in favor of the safe default instead. + """ + configured: Final = litellm.tag_rate_limiter_max_in_memory_cache_size + if isinstance(configured, int) and not isinstance(configured, bool) and configured > 0: + return configured + if configured is not None: + verbose_proxy_logger.warning( + "tag_rate_limiter: tag_rate_limiter_max_in_memory_cache_size=%r is not a positive integer; " + "falling back to the default in-memory cache size.", + configured, + ) + return None + + class _PROXY_TagRateLimiter( # pyright: ignore[reportUnusedClass] # only referenced via the deferred import in litellm_logging.py's callback resolver; basedpyright doesn't trace that usage CustomLogger ): @@ -631,13 +660,9 @@ class _PROXY_TagRateLimiter( # pyright: ignore[reportUnusedClass] # only refer # distinct tag value this hook sees. Deployments rate-limiting on a # high-cardinality tag_id (e.g. per end user) without Redis can raise # `litellm_settings.tag_rate_limiter_max_in_memory_cache_size` so - # active buckets aren't evicted before their period elapses. 0 would - # disable this hook's in-memory cache outright, so it's rejected here - # in favor of the safe default. - configured_max_cache_size: Final = litellm.tag_rate_limiter_max_in_memory_cache_size - max_cache_size: Final = configured_max_cache_size if configured_max_cache_size else None + # active buckets aren't evicted before their period elapses. isolated_dual_cache: Final = DualCache( - in_memory_cache=InMemoryCache(max_size_in_memory=max_cache_size), + in_memory_cache=InMemoryCache(max_size_in_memory=_resolve_max_in_memory_cache_size()), redis_cache=internal_usage_cache.redis_cache, ) self.internal_usage_cache = InternalUsageCache(dual_cache=isolated_dual_cache) @@ -819,11 +844,14 @@ class _PROXY_TagRateLimiter( # pyright: ignore[reportUnusedClass] # only refer # A reservation's TTL must comfortably outlast any real in-flight # request, or a slow request's reservation self-heals (expires) # while it is still genuinely running, silently admitting extra - # requests past the configured limit. period_seconds is still - # honored if the operator wants an even longer safety margin, but - # never shortens the floor below it. - return max(configured_limit.entry.period_seconds, _CONCURRENCY_MIN_SAFETY_TTL_SECONDS) - return configured_limit.entry.period_seconds + 3600 + # requests past the configured limit. period_seconds (or an + # explicit key_ttl_seconds override) is still honored if the + # operator wants an even longer safety margin, but this floor is + # never lowered below it, even by an explicit override. + entry: Final = configured_limit.entry + requested_ttl: Final = entry.key_ttl_seconds if entry.key_ttl_seconds is not None else entry.period_seconds + return max(requested_ttl, _CONCURRENCY_MIN_SAFETY_TTL_SECONDS) + return _bucket_ttl_seconds(configured_limit.entry) async def _read_only_values( self, diff --git a/litellm/types/router.py b/litellm/types/router.py index 0bdacb0b739..f99f7890f09 100644 --- a/litellm/types/router.py +++ b/litellm/types/router.py @@ -143,6 +143,13 @@ class TagRateLimitEntry(BaseModel): limit: float period_seconds: int scope_by_key_hash: bool = False + # Overrides this entry's bucket/reservation key TTL (Redis, and the + # in-memory fallback when Redis isn't configured). Defaults to + # period_seconds + 3600 when unset -- see _PROXY_TagRateLimiter._ttl_for. + # A high-cardinality tag_id can keep many keys alive at once; lowering + # this lets an operator shed them sooner without shortening + # period_seconds itself. + key_ttl_seconds: int | None = None model_config = ConfigDict(protected_namespaces=()) @@ -152,6 +159,12 @@ class TagRateLimitEntry(BaseModel): raise ValueError("period_seconds must be a positive integer") return self + @model_validator(mode="after") + def _validate_key_ttl_seconds(self) -> "TagRateLimitEntry": + if self.key_ttl_seconds is not None and self.key_ttl_seconds <= 0: + raise ValueError("key_ttl_seconds must be a positive integer when set") + return self + class TagRateLimitGroup(BaseModel): limits: tuple[TagRateLimitEntry, ...] = () diff --git a/tests/test_litellm/proxy/hooks/test_tag_rate_limiter.py b/tests/test_litellm/proxy/hooks/test_tag_rate_limiter.py index 7119a0c154d..bd7cf85e92a 100644 --- a/tests/test_litellm/proxy/hooks/test_tag_rate_limiter.py +++ b/tests/test_litellm/proxy/hooks/test_tag_rate_limiter.py @@ -5,6 +5,7 @@ Unit tests for tag-scoped token/request/dollar rate limiting. import asyncio import uuid from datetime import datetime, timedelta +from typing import Final import pytest @@ -14,6 +15,7 @@ from litellm.proxy.common_utils.proxy_rate_limit_error import ProxyRateLimitErro from litellm.proxy.hooks.tag_rate_limiter import ( _CONCURRENCY_MIN_SAFETY_TTL_SECONDS, _bucket_key, + _bucket_ttl_seconds, _build_group_limits, _build_limits_index, _ConfiguredLimit, @@ -2243,16 +2245,27 @@ async def test_max_in_memory_cache_size_setting_lets_high_cardinality_tags_avoid ) +@pytest.mark.parametrize( + "invalid_configured_size", + [ + 0, # would hit InMemoryCache.set_cache's `max_size_in_memory == 0` short-circuit, disabling the cache + -1, # would loop `heapq.heappop` on an empty heap in InMemoryCache.evict_cache and raise IndexError + "500", # an unresolved os.environ/ substitution or config typo; `len(...) >= "500"` raises TypeError + True, # bool is an int subclass; must not be misread as the positive integer 1 + ], +) @pytest.mark.asyncio -async def test_max_in_memory_cache_size_of_zero_falls_back_to_the_safe_default(time_controller, monkeypatch): +async def test_invalid_max_in_memory_cache_size_falls_back_to_the_safe_default( + time_controller, monkeypatch, invalid_configured_size +): """ - 0 would hit InMemoryCache.set_cache's own `max_size_in_memory == 0` - short-circuit and silently disable this hook's in-memory cache outright, - so it must be rejected in favor of the safe 200-item default rather than - passed straight through: a limit=1 bucket must still reject a second, - immediate request for the same tag. + DualCache.async_set_cache swallows any exception raised while writing, so an + invalid configured size would otherwise silently disable every counter write + for this hook (every read then sees an empty counter and is admitted) instead + of failing loudly. Each of these must be rejected in favor of the safe + default: a limit=1 bucket must still reject a second, immediate request. """ - monkeypatch.setattr(litellm, "tag_rate_limiter_max_in_memory_cache_size", 0) + monkeypatch.setattr(litellm, "tag_rate_limiter_max_in_memory_cache_size", invalid_configured_size) limiter = _PROXY_TagRateLimiter(internal_usage_cache=DualCache(), time_provider=time_controller.now) router = _single_request_per_minute_router() @@ -2273,3 +2286,60 @@ async def test_max_in_memory_cache_size_of_zero_falls_back_to_the_safe_default(t messages=None, request_kwargs={"metadata": {"tags": ["end_user_id:u1"]}}, ) + + +# --------------------------------------------------------------------------- +# per-tag Redis/bucket key TTL override +# --------------------------------------------------------------------------- + + +def _request_limit(period_seconds: int, key_ttl_seconds: int | None = None) -> _ConfiguredLimit: + return _ConfiguredLimit( + unit="requests", + entry=TagRateLimitEntry( + name="per_minute", tag_id="end_user_id", limit=1, period_seconds=period_seconds, key_ttl_seconds=key_ttl_seconds + ), + deployment_scope=None, + ) + + +def _concurrency_limit(period_seconds: int, key_ttl_seconds: int | None = None) -> _ConfiguredLimit: + return _ConfiguredLimit( + unit="concurrency", + entry=TagRateLimitEntry( + name="active", tag_id="end_user_id", limit=1, period_seconds=period_seconds, key_ttl_seconds=key_ttl_seconds + ), + deployment_scope=None, + ) + + +def test_bucket_ttl_seconds_defaults_to_period_plus_one_hour_when_unset(): + assert _bucket_ttl_seconds(_request_limit(period_seconds=60).entry) == 60 + 3600 + + +def test_bucket_ttl_seconds_honors_key_ttl_seconds_override(): + assert _bucket_ttl_seconds(_request_limit(period_seconds=60, key_ttl_seconds=120).entry) == 120 + + +def test_ttl_for_concurrency_honors_key_ttl_seconds_above_the_safety_floor(): + above_floor: Final = _CONCURRENCY_MIN_SAFETY_TTL_SECONDS + 100 + assert _PROXY_TagRateLimiter._ttl_for(_concurrency_limit(period_seconds=60, key_ttl_seconds=above_floor)) == above_floor + + +def test_ttl_for_concurrency_never_drops_below_the_safety_floor_even_with_a_lower_override(): + """ + A reservation's TTL must comfortably outlast any real in-flight request, so + an operator-set override below _CONCURRENCY_MIN_SAFETY_TTL_SECONDS must not + be honored as-is -- a slow request's reservation would otherwise self-heal + (expire) while still genuinely running, silently admitting extra requests. + """ + below_floor: Final = 10 + assert ( + _PROXY_TagRateLimiter._ttl_for(_concurrency_limit(period_seconds=60, key_ttl_seconds=below_floor)) + == _CONCURRENCY_MIN_SAFETY_TTL_SECONDS + ) + + +def test_tag_rate_limit_entry_rejects_non_positive_key_ttl_seconds(): + with pytest.raises(ValueError): + TagRateLimitEntry(name="per_minute", limit=1, period_seconds=60, key_ttl_seconds=0) diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index 2f0e4ad0666..a707f516c4d 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -35112,6 +35112,8 @@ export interface components { }; /** TagRateLimitEntry */ TagRateLimitEntry: { + /** Key Ttl Seconds */ + key_ttl_seconds?: number | null; /** Limit */ limit: number; /** Name */