diff --git a/litellm/proxy/management_endpoints/model_management_endpoints.py b/litellm/proxy/management_endpoints/model_management_endpoints.py index 2d8f4f67b29..7d44a502ac1 100644 --- a/litellm/proxy/management_endpoints/model_management_endpoints.py +++ b/litellm/proxy/management_endpoints/model_management_endpoints.py @@ -2256,36 +2256,50 @@ async def clear_cache() -> ReconcileOutcome: # This is a config model, preserved by the reconcile below config_models.append(model) - # NOTE: deployments are deliberately NOT wiped here. This used to + # ORDINARY db deployments are deliberately NOT wiped. This used to # delete_deployment() every db model before the reload put them back, which # left the router serving ZERO db models for the whole width of the reload # -- a real data-plane hole that every inference request landing in it fell - # into. It was also redundant: the reload's _delete_deployment evicts exactly - # the ids the db no longer lists, and upsert_deployment pops-and-re-adds a - # deployment whose params changed and no-ops one that did not (router.py - # upsert_deployment), so the reconcile converges to the same state on its - # own. Every mutation is visible to that comparison -- `blocked` and (for - # premium) `updated_at` are written into model_info. + # into. It was also redundant for them: the reload's _delete_deployment + # evicts exactly the ids the db no longer lists, and upsert_deployment + # pops-and-re-adds a deployment whose params changed while no-opping one + # that did not, so the reconcile converges on its own. Every mutation is + # visible to that comparison -- `blocked` and (for premium) `updated_at` + # are written into model_info. # - # The auto-router pops below are NOT redundant and stay: they are keyed by - # model_name, which no deployment-id reconcile touches. - - # Clear only DB-backed auto-router-family entries, keyed by model_name, so the - # reload below rebuilds them fresh. A blanket .clear() would also drop config-defined - # routers, which are never re-added below (add_deployment only reloads DB models), - # leaving them permanently unroutable until a full proxy restart for every tenant. - # Restrict to deployments whose model is actually an auto_router/* so a config - # router that merely shares a model_name with a regular DB model isn't evicted. The - # auto_router/ prefix also covers quality_router/ and adaptive_router/, so pop the - # name from every router registry (no-op where absent); missing quality/adaptive - # entries would otherwise make init raise "already exists" on reload and abort it. - db_router_names: Final = { - model.get("model_name") + # AUTO-ROUTER db deployments are the exception and ARE wiped, below, + # together with their strategy entries. Their strategy registries are keyed + # by model_name, which no deployment-id reconcile touches, so they have to + # be popped and rebuilt here. But the rebuild only happens on the ADD path: + # Router.upsert_deployment returns early when a deployment is unchanged and + # never reaches add_deployment -> _add_deployment -> + # init_auto_router_deployment, which is what repopulates the registries. + # Popping without deleting would therefore strip every db-backed auto, + # complexity, adaptive and quality router on this pod and never put it back, + # so ANY unrelated model write would leave them unroutable until a restart. + # Deleting the deployment forces upsert down the add path, which rebuilds + # both the deployment and its strategy entry. + # + # Restrict to deployments whose model is actually an auto_router/* so a + # config router that merely shares a model_name with a regular db model + # isn't evicted -- config routers are never re-added by the reload (it only + # reloads db models) and would be permanently unroutable for every tenant. + # The auto_router/ prefix also covers quality_router/ and adaptive_router/, + # so pop the name from every registry (no-op where absent); a missing + # quality/adaptive entry would otherwise make init raise "already exists" + # on reload and abort it. + db_router_deployments: Final = [ + model for model in current_models if model.get("model_name") is not None and model.get("model_info", {}).get("db_model", False) and str(model.get("litellm_params", {}).get("model", "")).startswith("auto_router/") - } + ] + db_router_names: Final = {model["model_name"] for model in db_router_deployments} + for model in db_router_deployments: + router_model_id = model.get("model_info", {}).get("id") + if router_model_id is not None: + llm_router.delete_deployment(id=router_model_id) for model_name in db_router_names: llm_router.auto_routers.pop(model_name, None) llm_router.complexity_routers.pop(model_name, None) diff --git a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py index 36784498964..aafd84e4c83 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py @@ -485,12 +485,13 @@ class TestClearCache: ): await clear_cache() - # clear_cache must NOT wipe deployments. It used to delete_deployment() - # every db model before the reload restored them, which left the router - # serving zero db models for the width of the reload. The reload's own - # _delete_deployment/upsert_deployment pair converges to the same state - # without that hole, so the wipe was a pure data-plane outage. - mock_router.delete_deployment.assert_not_called() + # clear_cache must wipe ONLY the db auto-router deployments -- the ones whose + # strategy entries are popped below and can only be rebuilt via the add path. + # Ordinary db models are left alone: wiping them un-served every db model for + # the width of the reload, and the reconcile converges without it. + assert mock_router.delete_deployment.call_count == 2 + mock_router.delete_deployment.assert_any_call(id="db-model-1") + mock_router.delete_deployment.assert_any_call(id="db-model-2") # DB-backed router entries are cleared so they can be re-populated by the # reload below; the config-backed router must survive, since add_deployment() @@ -506,6 +507,65 @@ class TestClearCache: prisma_client=mock_prisma, proxy_logging_obj=mock_logging ) + @pytest.mark.asyncio + async def test_clear_cache_wipes_auto_routers_but_leaves_ordinary_db_models(self): + """An ordinary db model must survive clear_cache; a db auto-router must not. + + Two separate hazards meet here, and fixing one naively breaks the other: + + - Wiping ordinary db models un-serves EVERY db model for the width of the + reload. The reconcile converges without that, so the wipe is a pure + data-plane hole. + - NOT wiping a db auto-router strands it. Its strategy registries are keyed by + model_name and are popped here, but Router.upsert_deployment returns early + for an unchanged deployment and never reaches the add path that rebuilds + them. Any unrelated model write would then leave every db-backed auto, + complexity, adaptive and quality router unroutable until a restart. + + So the wipe is scoped to exactly the auto-router deployments. + """ + from litellm.proxy.management_endpoints.model_management_endpoints import ( + clear_cache, + ) + + mock_router = MagicMock() + mock_router.model_list = [ + { + "model_name": "ordinary-db-model", + "model_info": {"id": "db-ordinary-1", "db_model": True}, + "litellm_params": {"model": "openai/gpt-4o"}, + }, + { + "model_name": "db-auto-router", + "model_info": {"id": "db-auto-1", "db_model": True}, + "litellm_params": {"model": "auto_router/db-auto-router"}, + }, + ] + mock_router.delete_deployment = MagicMock(return_value=True) + mock_router.auto_routers = {"db-auto-router": MagicMock()} + mock_router.complexity_routers = {} + mock_router.adaptive_routers = {} + mock_router.quality_routers = {} + + mock_config = MagicMock() + mock_config._add_deployment_locked = AsyncMock( + return_value=ReconcileOutcome(still_desired=frozenset(), live_after=frozenset()) + ) + + with ( + patch("litellm.proxy.proxy_server.llm_router", mock_router), + patch("litellm.proxy.proxy_server.proxy_config", mock_config), + patch("litellm.proxy.proxy_server.prisma_client", MagicMock()), + patch("litellm.proxy.proxy_server.proxy_logging_obj", MagicMock()), + patch("litellm.proxy.proxy_server.verbose_proxy_logger"), + ): + await clear_cache() + + # The auto-router deployment is wiped so the reload takes the add path and + # rebuilds its strategy entry; the ordinary db model is never touched. + mock_router.delete_deployment.assert_called_once_with(id="db-auto-1") + assert "db-auto-router" not in mock_router.auto_routers + class TestClearCachePreservesConfigRouters: """