fix(rate-limiting): reject invalid in-memory cache sizes, add a per-tag Redis TTL override

An unresolved os.environ/ substitution or a config typo could set
tag_rate_limiter_max_in_memory_cache_size to a negative number or a
string; InMemoryCache raises comparing its size against that value, and
DualCache.async_set_cache swallows the exception, silently disabling
every counter write for this hook without Redis. Only positive integers
are now accepted; anything else falls back to the safe default with a
warning.

Also adds TagRateLimitEntry.key_ttl_seconds so a high-cardinality tag_id
can shed its Redis (or in-memory fallback) keys sooner without
shortening period_seconds itself. Concurrency's safety-floor TTL is
still never lowered by this override.
This commit is contained in:
Deepanshu 2026-08-17 10:24:38 -04:00
parent fe9e36a0bc
commit a2a19cbdd9
4 changed files with 132 additions and 19 deletions

View file

@ -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,

View file

@ -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, ...] = ()

View file

@ -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)

View file

@ -35112,6 +35112,8 @@ export interface components {
};
/** TagRateLimitEntry */
TagRateLimitEntry: {
/** Key Ttl Seconds */
key_ttl_seconds?: number | null;
/** Limit */
limit: number;
/** Name */