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.
This commit is contained in:
Deepanshu 2026-08-20 17:18:34 -04:00
parent d8ea14d8ff
commit 96e53c037f
2 changed files with 68 additions and 2 deletions

View file

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

View file

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