mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(router): de-register strategy routers when a deployment is removed
upsert_deployment's pop-and-re-add and delete_deployment cleaned model_list but never the auto/complexity/adaptive/quality registries. The re-add then died on the init functions' 'already exists' check (swallowed under ignore_invalid_deployments), so any DB-stored auto-router deployment that was updated or reloaded silently vanished from /v1/models, /model/info and /model_group/info while the stale registry object kept serving requests with its old config; deletes left it routing as a ghost. Fixes #33168
This commit is contained in:
parent
229159c790
commit
4218ce0e19
2 changed files with 115 additions and 0 deletions
|
|
@ -7711,6 +7711,29 @@ class Router:
|
|||
len(config.available_models),
|
||||
)
|
||||
|
||||
def _remove_deployment_from_strategy_registries(self, model_name: str) -> None:
|
||||
"""
|
||||
Drop a model_name's strategy-router registration (auto / complexity /
|
||||
adaptive / quality) when its deployment is removed from the router.
|
||||
|
||||
Every `init_*_router_deployment` raises "already exists" when the
|
||||
model_name is still registered, so a removal path that cleans
|
||||
`model_list` but not these registries makes the deployment
|
||||
un-re-addable: the re-add dies before `_add_model_to_list_and_index_map`
|
||||
(swallowed under `ignore_invalid_deployments`), the row disappears from
|
||||
`/v1/models` & `/model/info`, and the stale registry object keeps
|
||||
serving requests with its old config. See issue #33168.
|
||||
|
||||
Safe for regular deployments: they are never in these registries, so
|
||||
the pops are no-ops. Strategy deployments are unique per model_name
|
||||
(the init functions enforce this), so popping by name cannot orphan a
|
||||
sibling deployment.
|
||||
"""
|
||||
self.auto_routers.pop(model_name, None)
|
||||
self.complexity_routers.pop(model_name, None)
|
||||
self.adaptive_routers.pop(model_name, None)
|
||||
self.quality_routers.pop(model_name, None)
|
||||
|
||||
def _is_quality_router_deployment(self, litellm_params: LiteLLM_Params) -> bool:
|
||||
"""
|
||||
Check if the deployment is a quality-router deployment.
|
||||
|
|
@ -8243,6 +8266,13 @@ class Router:
|
|||
self._invalidate_model_group_info_cache()
|
||||
self._invalidate_access_groups_cache()
|
||||
self._update_deployment_indices_after_removal(model_id=deployment_id, removal_idx=removal_idx)
|
||||
# de-register any strategy router (auto/complexity/
|
||||
# adaptive/quality) tied to the removed deployment, so
|
||||
# the add_deployment below can re-register it instead
|
||||
# of dying on the init functions' "already exists"
|
||||
# check (which silently delists the model under
|
||||
# ignore_invalid_deployments). See issue #33168.
|
||||
self._remove_deployment_from_strategy_registries(model_name=_deployment_on_router.model_name)
|
||||
|
||||
# if the model_id is not in router
|
||||
self.add_deployment(deployment=deployment)
|
||||
|
|
@ -8276,6 +8306,14 @@ class Router:
|
|||
self._invalidate_model_group_info_cache()
|
||||
self._invalidate_access_groups_cache()
|
||||
self._update_deployment_indices_after_removal(model_id=id, removal_idx=deployment_idx)
|
||||
# de-register any strategy router tied to this deployment —
|
||||
# otherwise it keeps serving the deleted model as a ghost and
|
||||
# blocks any future re-add under the same name (issue #33168)
|
||||
_removed_model_name = (
|
||||
item.get("model_name") if isinstance(item, dict) else getattr(item, "model_name", None)
|
||||
)
|
||||
if _removed_model_name:
|
||||
self._remove_deployment_from_strategy_registries(model_name=_removed_model_name)
|
||||
_budget_limiter = self._get_router_deployment_budget_limiter()
|
||||
if _budget_limiter is not None:
|
||||
_budget_limiter.unregister_deployment_budget(model_id=id)
|
||||
|
|
|
|||
|
|
@ -5304,3 +5304,80 @@ class TestRouterRequestTimeoutPropagation:
|
|||
)
|
||||
== 60
|
||||
)
|
||||
|
||||
|
||||
class TestStrategyRegistryCleanupOnRemoval:
|
||||
"""upsert_deployment / delete_deployment must de-register strategy routers
|
||||
(auto/complexity/adaptive/quality) for the removed deployment — otherwise
|
||||
the re-add dies on the init functions' "already exists" check and the
|
||||
model silently disappears from every listing while the stale registry
|
||||
object keeps routing. Issue #33168."""
|
||||
|
||||
def _make_router(self):
|
||||
return litellm.Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "gpt-4o-mini",
|
||||
"litellm_params": {"model": "gpt-4o-mini", "api_key": "fake-key"},
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
def _complexity_deployment(self, simple_tier: str = "gpt-4o-mini"):
|
||||
from litellm.types.router import Deployment, LiteLLM_Params
|
||||
|
||||
return Deployment(
|
||||
model_name="test-auto-latest",
|
||||
litellm_params=LiteLLM_Params(
|
||||
model="auto_router/complexity_router",
|
||||
complexity_router_config={
|
||||
"tiers": {
|
||||
"SIMPLE": simple_tier,
|
||||
"MEDIUM": simple_tier,
|
||||
"COMPLEX": simple_tier,
|
||||
"REASONING": simple_tier,
|
||||
}
|
||||
},
|
||||
complexity_router_default_model=simple_tier,
|
||||
),
|
||||
model_info={"id": "complexity-registry-test-id"},
|
||||
)
|
||||
|
||||
def test_upsert_updated_complexity_router_stays_listed(self):
|
||||
router = self._make_router()
|
||||
router.add_deployment(self._complexity_deployment())
|
||||
assert "test-auto-latest" in router.get_model_names()
|
||||
assert "test-auto-latest" in router.complexity_routers
|
||||
|
||||
# change the config -> upsert removes + re-adds the deployment
|
||||
router.upsert_deployment(self._complexity_deployment(simple_tier="gpt-4o"))
|
||||
|
||||
assert "test-auto-latest" in router.get_model_names()
|
||||
assert (
|
||||
router.complexity_routers["test-auto-latest"].config.tiers["SIMPLE"]
|
||||
== "gpt-4o"
|
||||
)
|
||||
|
||||
def test_delete_complexity_router_clears_registry_and_allows_re_add(self):
|
||||
router = self._make_router()
|
||||
deployment = self._complexity_deployment()
|
||||
router.add_deployment(deployment)
|
||||
|
||||
router.delete_deployment(id="complexity-registry-test-id")
|
||||
|
||||
# ghost registration must be gone once the deployment is deleted
|
||||
assert "test-auto-latest" not in router.complexity_routers
|
||||
|
||||
# and the same name must be re-addable + listed again
|
||||
router.add_deployment(deployment)
|
||||
assert "test-auto-latest" in router.get_model_names()
|
||||
assert "test-auto-latest" in router.complexity_routers
|
||||
|
||||
def test_delete_regular_deployment_untouched_by_registry_cleanup(self):
|
||||
router = self._make_router()
|
||||
model_id = router.get_model_ids()[0]
|
||||
|
||||
deleted = router.delete_deployment(id=model_id)
|
||||
|
||||
assert deleted is not None
|
||||
assert "gpt-4o-mini" not in router.get_model_names()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue