scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-10-09 03:18:44 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 109

fix(rate-limiting): fold scope_by_key_hash into the bucket-key fingerprint

Browse source 
_policy_fingerprint hashed limit/period_seconds/enabled_for/disabled_for/
apply_to_key_alias but not scope_by_key_hash, even though _DedupSignature
already treats it as a distinct policy. _hash_tag's key_hash-derived suffix
is empty whenever key_hash resolves to None, so a key-hash-scoped entry and
an otherwise-identical unscoped entry collided onto the same counter for any
call with no resolvable virtual key.
This commit is contained in:
Deepanshu 2026-08-25 11:37:06 -04:00
parent 7633035cf1
commit 993adb0c9c
2 changed files with 38 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
litellm/proxy/hooks/model_based_tag_rate_limits_hook.py
View file

@ -700,18 +700,24 @@ def _fixed_length_identity(tag_value: str) -> str:
def _policy_fingerprint(entry: TagRateLimitEntry) -> str:
"""
Two entries can share a `name` and `tag_id` while genuinely disagreeing
on `limit`, `period_seconds`, or any of the scoping fields --
`_DedupSignature`/`resolve_any` already treat that as two distinct
policies (see `distinct_signature_count_by_name` in `_build_group_limits`),
so the Redis/in-memory bucket key must too, or two differently-configured
entries that happen to share a name check and charge the identical
counter. Hashed to a fixed-length digest for the same reason
on `limit`, `period_seconds`, `scope_by_key_hash`, or any of the scoping
fields -- `_DedupSignature`/`resolve_any` already treat that as two
distinct policies (see `distinct_signature_count_by_name` in
`_build_group_limits`), so the Redis/in-memory bucket key must too, or two
differently-configured entries that happen to share a name check and
charge the identical counter. `scope_by_key_hash` specifically needs its
own slot here rather than relying on `_hash_tag`'s `key_hash`-derived
suffix to carry it: that suffix is empty whenever `key_hash` resolves to
`None` (no virtual key on the call), which would otherwise collide an
unscoped entry with a key-hash-scoped one that agrees on every other
field. Hashed to a fixed-length digest for the same reason
`_fixed_length_identity` hashes `tag_value`: an operator's own
`enabled_for`/`disabled_for`/`apply_to_key_alias` list has no length bound.
"""
fingerprint_source: Final = (
entry.limit,
entry.period_seconds,
entry.scope_by_key_hash,
_scope_signature(entry.enabled_for),
_scope_signature(entry.disabled_for),
entry.apply_to_key_alias,

28
tests/test_litellm/proxy/hooks/test_model_based_tag_rate_limits_hook.py
View file

@ -106,6 +106,7 @@ def _expected_bucket_key(
limit: float = 1,
enabled_for: dict | None = None,
disabled_for: dict | None = None,
scope_by_key_hash: bool = False,
) -> str:
"""
Builds the exact key the real code would compute (via _hash_tag's
@ -130,6 +131,7 @@ def _expected_bucket_key(
period_seconds=period_seconds,
enabled_for=enabled_for,
disabled_for=disabled_for,
scope_by_key_hash=scope_by_key_hash,
),
deployment_scope=deployment_scope,
team_scope=team_scope,
@ -599,6 +601,28 @@ def test_bucket_key_differs_for_same_named_entries_with_different_scoping_only()
assert excluding_u1 != excluding_u2
def test_bucket_key_differs_for_same_named_entries_diverging_only_on_scope_by_key_hash():
"""
_DedupSignature already folds scope_by_key_hash into dedup (two
deployments declaring the same name/tag_id but different
scope_by_key_hash become two distinct _ConfiguredLimit entries, not one
merged one), but _policy_fingerprint didn't fold it into the bucket-key
hash. When a request's key_hash resolves to None -- e.g. no virtual key
on the call -- both entries' key_hash-derived suffix is empty too, so an
unscoped entry and a key-hash-scoped entry that otherwise share every
other field collided onto the identical counter, letting one entry's
admission or accounting silently corrupt the other's.
"""
now = 0.0
unscoped = _expected_bucket_key(
"grp", "requests", "daily", "end_user_id", "u1", 86400, now, limit=100, scope_by_key_hash=False
)
key_hash_scoped_but_no_key_present = _expected_bucket_key(
"grp", "requests", "daily", "end_user_id", "u1", 86400, now, limit=100, scope_by_key_hash=True, key_hash=None
)
assert unscoped != key_hash_scoped_but_no_key_present
# ---------------------------------------------------------------------------
# _build_group_limits -- scoping fields fold into the dedup signature
# ---------------------------------------------------------------------------
@ -1730,10 +1754,10 @@ async def test_log_success_event_accounts_against_the_key_hash_admission_checked
now = time_controller.now().timestamp()
keyed_bucket = _expected_bucket_key(
"grp", "tokens", "daily", "end_user_id", "u1", 86400, now, key_hash="keyA", limit=500000
"grp", "tokens", "daily", "end_user_id", "u1", 86400, now, key_hash="keyA", limit=500000, scope_by_key_hash=True
)
unkeyed_bucket = _expected_bucket_key(
"grp", "tokens", "daily", "end_user_id", "u1", 86400, now, key_hash=None, limit=500000
"grp", "tokens", "daily", "end_user_id", "u1", 86400, now, key_hash=None, limit=500000, scope_by_key_hash=True
)
assert (
float(await limiter.internal_usage_cache.async_get_cache(key=keyed_bucket, litellm_parent_otel_span=None))