From 96e53c037fd0233fe8d6630830b0a0333a224d4b Mon Sep 17 00:00:00 2001 From: Deepanshu Date: Thu, 20 Aug 2026 17:18:34 -0400 Subject: [PATCH] fix(rate-limiting): dedup admission against the full routing group, not just healthy members Admission derived resolve_any's candidate set from healthy_deployments (Router's cooldown-filtered list for this hop), while success accounting reconstructs the full, static group membership. A member merely cooled down at admission time would shrink admission's candidate set without changing success's, so the two could dedup to different resolved_group values and land on different buckets whenever a member was temporarily unhealthy. Admission now derives its candidate set from the same full group membership success does. --- litellm/proxy/hooks/tag_rate_limiter.py | 18 ++++++- .../proxy/hooks/test_tag_rate_limiter.py | 52 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/hooks/tag_rate_limiter.py b/litellm/proxy/hooks/tag_rate_limiter.py index 6395db08e24..7ee33d455eb 100644 --- a/litellm/proxy/hooks/tag_rate_limiter.py +++ b/litellm/proxy/hooks/tag_rate_limiter.py @@ -1009,8 +1009,22 @@ class _PROXY_TagRateLimiter( # pyright: ignore[reportUnusedClass] # only refer await self._release_stale_hop_reservations(resolved_request_kwargs) metadata_variable_name: Final = get_metadata_variable_name_from_kwargs(resolved_request_kwargs) team_id: Final = _extract_team_id(resolved_request_kwargs, metadata_variable_name) - candidate_model_names: Final = tuple( - name for d in healthy_deployments if isinstance(name := d.get("model_name"), str) + # Built from the full routing-group membership, not `healthy_deployments` + # (Router's own cooldown-filtered list for this hop): a member that's + # merely cooled down right now is still a real member of the group for + # the purpose of deciding resolved_group, and success accounting has no + # way to know which members were healthy at admission time -- it can + # only reconstruct the full, static membership (see its own comment + # below). Deriving both sides from the same full-membership source is + # the only way they're guaranteed to dedup to the identical bucket + # regardless of cooldown state at either point in time. + routing_group_deployments: Final = self.llm_router._get_routing_group_deployments( # pyright: ignore[reportPrivateUsage] # reused across module boundaries, matching resolve_any's own reliance on this method + model=model, team_id=team_id + ) + candidate_model_names: Final = ( + tuple(dep["model_name"] for dep in routing_group_deployments) + if routing_group_deployments is not None + else tuple(name for d in healthy_deployments if isinstance(name := d.get("model_name"), str)) ) configured: Final = self._index.get(self.llm_router).resolve_any(model, team_id, candidate_model_names) if not configured: 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 49ca7fb934b..8e4a52fa263 100644 --- a/tests/test_litellm/proxy/hooks/test_tag_rate_limiter.py +++ b/tests/test_litellm/proxy/hooks/test_tag_rate_limiter.py @@ -986,6 +986,58 @@ async def test_log_success_event_accounts_against_the_same_bucket_admission_chec ) +@pytest.mark.asyncio +async def test_admission_dedups_against_the_full_group_not_just_currently_healthy_members(time_controller): + """ + `healthy_deployments` is Router's cooldown-filtered list for this one + hop -- a member merely cooled down right now is excluded from it, but + it's still a real member of the routing group. Deriving resolve_any's + candidate set from `healthy_deployments` instead of the full group would + make admission's resolved_group choice depend on which members happen to + be healthy at that exact moment, while success accounting (which has no + way to know what was healthy at admission time) always reconstructs the + full, static membership -- landing the two sides on different buckets + whenever a member is cooled down. Admission must dedup against the same + full membership success does, regardless of which members are currently + healthy. + """ + token_limits = { + "token_limits": {"limits": [{"name": "daily", "tag_id": "end_user_id", "limit": 10, "period_seconds": 86400}]} + } + router = litellm.Router( + model_list=[ + _deployment("backend-a", "dep-a", token_limits), + _deployment("backend-b", "dep-b", token_limits), + ], + routing_groups=[ + RoutingGroup(group_name="my-group", models=["backend-a", "backend-b"], routing_strategy="simple-shuffle") + ], + ) + limiter = _make_limiter(time_controller) + limiter.update_variables(llm_router=router) + + # The shared entry always dedups to "backend-a" (alphabetically first). + # Pre-load *that* bucket over the limit; the "backend-b" bucket (what a + # healthy_deployments-derived candidate set would wrongly resolve to, + # since backend-a is the only one excluded below) stays empty. + now = time_controller.now().timestamp() + over_limit_key = _expected_bucket_key( + "my-group", "tokens", "daily", "end_user_id", "u1", 86400, now, resolved_group="backend-a" + ) + await limiter.internal_usage_cache.async_set_cache(key=over_limit_key, value=20.0, litellm_parent_otel_span=None) + + # Simulate backend-a being cooled down: Router would exclude it from the + # healthy_deployments list passed to this hop's admission. + healthy_excluding_backend_a = [d for d in router.model_list if d["model_name"] == "backend-b"] + with pytest.raises(ProxyRateLimitError): + await limiter.async_filter_deployments( + model="my-group", + healthy_deployments=healthy_excluding_backend_a, + messages=None, + request_kwargs={"metadata": {"tags": ["end_user_id:u1"]}}, + ) + + @pytest.mark.asyncio async def test_log_success_event_accounts_against_the_key_hash_admission_checked(time_controller): """