mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-13 08:52:02 +00:00
fix(router): clean per-strategy router registries on upsert/delete
This commit is contained in:
parent
b200d664ee
commit
3d7d9e7aa1
2 changed files with 115 additions and 0 deletions
|
|
@ -8228,6 +8228,11 @@ class Router:
|
|||
self._invalidate_access_groups_cache()
|
||||
self._update_deployment_indices_after_removal(model_id=deployment_id, removal_idx=removal_idx)
|
||||
|
||||
self._remove_strategy_router_registrations(
|
||||
model_name=_deployment_on_router.model_name,
|
||||
litellm_params=_deployment_on_router.litellm_params,
|
||||
)
|
||||
|
||||
# if the model_id is not in router
|
||||
self.add_deployment(deployment=deployment)
|
||||
return deployment
|
||||
|
|
@ -8253,6 +8258,8 @@ class Router:
|
|||
if id in self.model_id_to_deployment_index_map:
|
||||
deployment_idx = self.model_id_to_deployment_index_map[id]
|
||||
|
||||
deployment_on_router = self.get_deployment(model_id=id)
|
||||
|
||||
try:
|
||||
if deployment_idx is not None:
|
||||
# Pop the item from the list first
|
||||
|
|
@ -8263,12 +8270,60 @@ class Router:
|
|||
_budget_limiter = self._get_router_deployment_budget_limiter()
|
||||
if _budget_limiter is not None:
|
||||
_budget_limiter.unregister_deployment_budget(model_id=id)
|
||||
if deployment_on_router is not None:
|
||||
self._remove_strategy_router_registrations(
|
||||
model_name=deployment_on_router.model_name,
|
||||
litellm_params=deployment_on_router.litellm_params,
|
||||
)
|
||||
return item
|
||||
else:
|
||||
return None
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
def _remove_strategy_router_registrations(self, model_name: str, litellm_params: LiteLLM_Params) -> None:
|
||||
"""
|
||||
Remove the per-strategy router registrations for a deployment being
|
||||
removed or replaced.
|
||||
|
||||
`upsert_deployment` / `delete_deployment` pop the deployment from
|
||||
`model_list` but the strategy registries (`auto_routers`,
|
||||
`complexity_routers`, `quality_routers`, `adaptive_routers`) are keyed by
|
||||
`model_name` and were only ever reset wholesale in `set_model_list`.
|
||||
Without this cleanup, re-adding an updated deployment raises
|
||||
"...already exists" (silently dropping it from `model_list` under
|
||||
`ignore_invalid_deployments`), and deletes leave a ghost router that keeps
|
||||
serving traffic until a full restart.
|
||||
|
||||
Adaptive registration is cleaned unconditionally because a
|
||||
complexity-router deployment with `adaptive: True` also registers an
|
||||
adaptive router under the same `model_name`, so it cannot be gated on the
|
||||
adaptive-router prefix alone.
|
||||
"""
|
||||
if self._is_auto_router_deployment(litellm_params=litellm_params):
|
||||
self.auto_routers.pop(model_name, None)
|
||||
if self._is_complexity_router_deployment(litellm_params=litellm_params):
|
||||
self.complexity_routers.pop(model_name, None)
|
||||
if self._is_quality_router_deployment(litellm_params=litellm_params):
|
||||
self.quality_routers.pop(model_name, None)
|
||||
self._remove_adaptive_router_registration(model_name=model_name)
|
||||
|
||||
def _remove_adaptive_router_registration(self, model_name: str) -> None:
|
||||
"""
|
||||
Drop the adaptive router registered under `model_name` and unregister its
|
||||
post-call hook so it stops recording signals after the deployment is gone.
|
||||
"""
|
||||
adaptive_router = self.adaptive_routers.pop(model_name, None)
|
||||
if adaptive_router is None:
|
||||
return
|
||||
from litellm.router_strategy.adaptive_router.hooks import (
|
||||
AdaptiveRouterPostCallHook,
|
||||
)
|
||||
|
||||
for callback in litellm.logging_callback_manager.get_custom_loggers_for_type(AdaptiveRouterPostCallHook):
|
||||
if isinstance(callback, AdaptiveRouterPostCallHook) and callback.adaptive_router is adaptive_router:
|
||||
litellm.logging_callback_manager.remove_callback_from_all_lists(callback)
|
||||
|
||||
def _get_router_deployment_budget_limiter(
|
||||
self,
|
||||
) -> Optional[RouterBudgetLimiting]:
|
||||
|
|
|
|||
|
|
@ -962,6 +962,66 @@ class TestRouterComplexityDeploymentMethods:
|
|||
assert adaptive.model_to_prefs["premium"].quality_tier == 3
|
||||
|
||||
|
||||
class TestComplexityRouterRegistryLifecycle:
|
||||
"""Regression tests for issue #33168.
|
||||
|
||||
DB-stored complexity-router models silently disappear from `/v1/models` on
|
||||
`PATCH /model/{id}/update` and keep routing as a ghost on `/model/delete`,
|
||||
because `upsert_deployment` / `delete_deployment` clean `model_list` but never
|
||||
the `complexity_routers` registry. These exercise the router-level lifecycle
|
||||
the proxy DB-sync uses.
|
||||
"""
|
||||
|
||||
def _make_deployment(self, model_id: str, simple_model: str):
|
||||
from litellm.types.router import Deployment, LiteLLM_Params
|
||||
|
||||
return Deployment(
|
||||
model_name="auto_router/complexity_router/my-router",
|
||||
litellm_params=LiteLLM_Params(
|
||||
model="auto_router/complexity_router/my-router",
|
||||
complexity_router_default_model=simple_model,
|
||||
complexity_router_config={"tiers": {"SIMPLE": simple_model, "MEDIUM": "gpt-4o"}},
|
||||
),
|
||||
model_info={"id": model_id},
|
||||
)
|
||||
|
||||
def _router(self, simple_model: str = "gpt-4o-mini"):
|
||||
# ignore_invalid_deployments mirrors the proxy default, under which the
|
||||
# pre-fix "...already exists" error was swallowed and the deployment
|
||||
# silently dropped from model_list.
|
||||
return Router(
|
||||
model_list=[self._make_deployment("cr-1", simple_model).to_json(exclude_none=True)],
|
||||
ignore_invalid_deployments=True,
|
||||
)
|
||||
|
||||
def test_upsert_reregisters_and_keeps_model_listed(self):
|
||||
router = self._router(simple_model="gpt-4o-mini")
|
||||
name = "auto_router/complexity_router/my-router"
|
||||
assert name in router.complexity_routers
|
||||
assert router.get_deployment(model_id="cr-1") is not None
|
||||
|
||||
updated = self._make_deployment("cr-1", simple_model="gpt-4o")
|
||||
router.upsert_deployment(deployment=updated)
|
||||
|
||||
# Deployment must remain visible in model_list (feeds /v1/models & /model/info).
|
||||
assert router.get_deployment(model_id="cr-1") is not None
|
||||
assert name in router.model_name_to_deployment_indices
|
||||
# Registry must be rebuilt with the patched config, not left stale.
|
||||
assert name in router.complexity_routers
|
||||
assert router.complexity_routers[name].config.default_model == "gpt-4o"
|
||||
|
||||
def test_delete_removes_registry_no_ghost(self):
|
||||
router = self._router()
|
||||
name = "auto_router/complexity_router/my-router"
|
||||
assert name in router.complexity_routers
|
||||
|
||||
router.delete_deployment(id="cr-1")
|
||||
|
||||
assert router.get_deployment(model_id="cr-1") is None
|
||||
assert name not in router.complexity_routers
|
||||
assert name not in router.model_name_to_deployment_indices
|
||||
|
||||
|
||||
class TestAsyncPreRoutingHookMultiFormat:
|
||||
"""Test async_pre_routing_hook with multiple input formats."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue