mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(guardrails): stop re-initializing DB guardrails on every poll (#30542)
* fix(guardrails): stop re-initializing DB guardrails on every poll
InMemoryGuardrailHandler._has_guardrail_params_changed compared the
in-memory LitellmParams against the raw dict loaded from the DB. The
in-memory side carries every field default and coerces enums via
model_dump(), while the DB side only holds the keys originally stored,
so the two shapes never compared equal and the guardrail was rebuilt on
every poll cycle.
Each rebuild created a fresh instance, but delete_in_memory_guardrail
only removed the old callback from litellm.callbacks. Request handling
promotes guardrail callbacks into the success/failure/async lists, so
the previous instance stayed referenced there and instances accumulated.
Normalize both sides through LitellmParams(...).model_dump() before
diffing, and purge the callback from every callback list on delete.
* refactor(guardrails): narrow params-normalization fallback to ValidationError
The comparison normalizer caught a bare Exception and silently fell back
to the raw dict, which hid the cause and quietly degraded the affected
guardrail back to re-initializing on every poll. Catch only the
ValidationError that LitellmParams construction can raise, log a warning
so the offending row is diagnosable, and let any other error surface
instead of being swallowed.
* refactor(callbacks): add remove_callback_from_all_lists helper to manager
Move the knowledge of which callback lists a callback can be promoted
into out of the guardrail registry and into LoggingCallbackManager, where
the rest of the callback-list bookkeeping already lives. delete_in_memory_guardrail
now delegates to the new helper instead of iterating the lists itself.
(cherry picked from commit 9fa74ad8b4)
This commit is contained in:
parent
05efbc6454
commit
a68db8b5bb
4 changed files with 253 additions and 19 deletions
|
|
@ -394,6 +394,22 @@ class LoggingCallbackManager:
|
|||
+ litellm._async_failure_callback
|
||||
)
|
||||
|
||||
def remove_callback_from_all_lists(self, obj, require_self=False) -> None:
|
||||
"""
|
||||
Remove a callback object from every callback list it may have been
|
||||
promoted into, so a re-initialized callback leaves no stale instance behind.
|
||||
"""
|
||||
for callback_list in (
|
||||
litellm.callbacks,
|
||||
litellm.success_callback,
|
||||
litellm.failure_callback,
|
||||
litellm._async_success_callback,
|
||||
litellm._async_failure_callback,
|
||||
):
|
||||
self.remove_callback_from_list_by_object(
|
||||
callback_list, obj, require_self=require_self
|
||||
)
|
||||
|
||||
def get_active_additional_logging_utils_from_custom_logger(
|
||||
self,
|
||||
) -> Set[AdditionalLoggingUtils]:
|
||||
|
|
|
|||
|
|
@ -5,6 +5,8 @@ import os
|
|||
from datetime import datetime, timezone
|
||||
from typing import Any, Dict, List, Literal, Optional, Set, Type, cast
|
||||
|
||||
from pydantic import ValidationError
|
||||
|
||||
import litellm
|
||||
from litellm import Router
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
|
@ -598,21 +600,25 @@ class InMemoryGuardrailHandler:
|
|||
def delete_in_memory_guardrail(self, guardrail_id: str) -> None:
|
||||
"""
|
||||
Delete a guardrail in memory and remove from litellm callbacks.
|
||||
|
||||
The callback is purged from every callback list, not just
|
||||
litellm.callbacks: request handling promotes guardrail callbacks into the
|
||||
success/failure/async lists, so removing it from only litellm.callbacks
|
||||
leaves the old instance stranded in those lists on every re-initialization.
|
||||
"""
|
||||
# Remove from in-memory storage
|
||||
self.IN_MEMORY_GUARDRAILS.pop(guardrail_id, None)
|
||||
self._sources.pop(guardrail_id, None)
|
||||
|
||||
# Remove the callback from litellm.callbacks
|
||||
custom_guardrail_callback = self.guardrail_id_to_custom_guardrail.pop(
|
||||
guardrail_id, None
|
||||
)
|
||||
if custom_guardrail_callback:
|
||||
litellm.logging_callback_manager.remove_callback_from_list_by_object(
|
||||
callback_list=litellm.callbacks,
|
||||
obj=custom_guardrail_callback,
|
||||
require_self=False,
|
||||
)
|
||||
if custom_guardrail_callback is None:
|
||||
return
|
||||
|
||||
litellm.logging_callback_manager.remove_callback_from_all_lists(
|
||||
custom_guardrail_callback
|
||||
)
|
||||
|
||||
def list_in_memory_guardrails(self) -> List[Guardrail]:
|
||||
"""
|
||||
|
|
@ -654,6 +660,34 @@ class InMemoryGuardrailHandler:
|
|||
self.delete_in_memory_guardrail(guardrail_id)
|
||||
return stale_ids
|
||||
|
||||
@staticmethod
|
||||
def _normalize_litellm_params_for_comparison(
|
||||
params: Optional[Any],
|
||||
) -> Optional[Dict[str, Any]]:
|
||||
"""
|
||||
Render litellm_params to a canonical dict so an in-memory LitellmParams and
|
||||
the raw dict loaded from the DB compare equal when they describe the same
|
||||
config. The in-memory side is a LitellmParams whose model_dump() carries
|
||||
every field default and coerces enums, while the DB side is the raw stored
|
||||
dict holding only the keys originally provided. Comparing those two shapes
|
||||
directly never matches, so each DB poll would re-initialize the guardrail
|
||||
forever; normalizing both through LitellmParams keeps the diff meaningful.
|
||||
"""
|
||||
if params is None:
|
||||
return None
|
||||
if isinstance(params, LitellmParams):
|
||||
return params.model_dump()
|
||||
if isinstance(params, dict):
|
||||
try:
|
||||
return LitellmParams(**params).model_dump()
|
||||
except ValidationError as e:
|
||||
verbose_proxy_logger.warning(
|
||||
f"Could not normalize guardrail litellm_params for comparison; "
|
||||
f"treating the guardrail as changed. Error: {e}"
|
||||
)
|
||||
return params
|
||||
return params
|
||||
|
||||
def _has_guardrail_params_changed(
|
||||
self, guardrail_id: str, new_guardrail: Guardrail
|
||||
) -> bool:
|
||||
|
|
@ -670,19 +704,11 @@ class InMemoryGuardrailHandler:
|
|||
return True
|
||||
|
||||
# Compare litellm_params
|
||||
existing_params = existing.get("litellm_params")
|
||||
new_params = new_guardrail.get("litellm_params")
|
||||
|
||||
# Convert to dicts for comparison
|
||||
existing_dict = (
|
||||
existing_params.model_dump()
|
||||
if isinstance(existing_params, LitellmParams)
|
||||
else existing_params
|
||||
existing_dict = self._normalize_litellm_params_for_comparison(
|
||||
existing.get("litellm_params")
|
||||
)
|
||||
new_dict = (
|
||||
new_params.model_dump()
|
||||
if isinstance(new_params, LitellmParams)
|
||||
else new_params
|
||||
new_dict = self._normalize_litellm_params_for_comparison(
|
||||
new_guardrail.get("litellm_params")
|
||||
)
|
||||
|
||||
# Compare and identify specific differences
|
||||
|
|
|
|||
|
|
@ -192,6 +192,29 @@ def test_remove_callback_from_list_by_object():
|
|||
assert len(litellm._async_failure_callback) == 0
|
||||
|
||||
|
||||
def test_remove_callback_from_all_lists():
|
||||
manager = LoggingCallbackManager()
|
||||
manager._reset_all_callbacks()
|
||||
|
||||
class TestLogger(CustomLogger):
|
||||
pass
|
||||
|
||||
obj = TestLogger()
|
||||
manager.add_litellm_callback(obj)
|
||||
manager.add_litellm_success_callback(obj)
|
||||
manager.add_litellm_failure_callback(obj)
|
||||
manager.add_litellm_async_success_callback(obj)
|
||||
manager.add_litellm_async_failure_callback(obj)
|
||||
|
||||
manager.remove_callback_from_all_lists(obj)
|
||||
|
||||
assert obj not in litellm.callbacks
|
||||
assert obj not in litellm.success_callback
|
||||
assert obj not in litellm.failure_callback
|
||||
assert obj not in litellm._async_success_callback
|
||||
assert obj not in litellm._async_failure_callback
|
||||
|
||||
|
||||
def test_reset_callbacks(callback_manager):
|
||||
# Add various callbacks
|
||||
callback_manager.add_litellm_callback("test")
|
||||
|
|
|
|||
|
|
@ -180,3 +180,172 @@ def test_sync_guardrail_from_db_marks_source_db_when_unchanged():
|
|||
handler.sync_guardrail_from_db(g)
|
||||
|
||||
assert handler.get_source("collide") == "db"
|
||||
|
||||
|
||||
def _db_litellm_params() -> dict:
|
||||
"""
|
||||
Shape produced by GuardrailRegistry.get_all_guardrails_from_db: litellm_params
|
||||
is a raw dict (not a LitellmParams), holding only the keys originally stored,
|
||||
a non-schema extra key, and plain-string enum values.
|
||||
"""
|
||||
return {
|
||||
"guardrail": "litellm_content_filter",
|
||||
"mode": "pre_call",
|
||||
"default_on": True,
|
||||
"version": 2,
|
||||
"blocked_words": [{"keyword": "secret", "action": "BLOCK"}],
|
||||
}
|
||||
|
||||
|
||||
def test_unchanged_db_params_do_not_register_as_changed():
|
||||
"""
|
||||
A DB poll returns litellm_params as a raw dict while the in-memory copy is a
|
||||
LitellmParams whose model_dump() fills every field default and coerces enums.
|
||||
The two shapes must compare equal when the config is identical; otherwise
|
||||
every poll cycle re-initializes the guardrail indefinitely.
|
||||
"""
|
||||
handler = InMemoryGuardrailHandler()
|
||||
raw = _db_litellm_params()
|
||||
gid = "11111111-1111-1111-1111-111111111111"
|
||||
handler.IN_MEMORY_GUARDRAILS[gid] = Guardrail(
|
||||
guardrail_id=gid,
|
||||
guardrail_name="cf",
|
||||
litellm_params=LitellmParams(**raw),
|
||||
)
|
||||
|
||||
new = Guardrail(guardrail_id=gid, guardrail_name="cf", litellm_params=dict(raw))
|
||||
assert handler._has_guardrail_params_changed(gid, new) is False
|
||||
|
||||
|
||||
def test_changed_db_params_register_as_changed():
|
||||
"""Normalizing both sides must still surface a genuine config change."""
|
||||
handler = InMemoryGuardrailHandler()
|
||||
raw = _db_litellm_params()
|
||||
gid = "22222222-2222-2222-2222-222222222222"
|
||||
handler.IN_MEMORY_GUARDRAILS[gid] = Guardrail(
|
||||
guardrail_id=gid,
|
||||
guardrail_name="cf",
|
||||
litellm_params=LitellmParams(**raw),
|
||||
)
|
||||
|
||||
changed = {**raw, "blocked_words": [{"keyword": "different", "action": "BLOCK"}]}
|
||||
new = Guardrail(guardrail_id=gid, guardrail_name="cf", litellm_params=changed)
|
||||
assert handler._has_guardrail_params_changed(gid, new) is True
|
||||
|
||||
|
||||
def test_unnormalizable_db_params_register_as_changed_without_raising():
|
||||
"""
|
||||
A DB row whose litellm_params fail LitellmParams validation must not crash the
|
||||
poll loop. The comparison falls back to treating the guardrail as changed so it
|
||||
re-initializes (and surfaces the bad row in logs) rather than propagating the
|
||||
validation error up through the polling cycle.
|
||||
"""
|
||||
handler = InMemoryGuardrailHandler()
|
||||
raw = _db_litellm_params()
|
||||
gid = "55555555-5555-5555-5555-555555555555"
|
||||
handler.IN_MEMORY_GUARDRAILS[gid] = Guardrail(
|
||||
guardrail_id=gid,
|
||||
guardrail_name="cf",
|
||||
litellm_params=LitellmParams(**raw),
|
||||
)
|
||||
|
||||
malformed = {**raw, "default_on": "not-a-bool-xyz"}
|
||||
new = Guardrail(guardrail_id=gid, guardrail_name="cf", litellm_params=malformed)
|
||||
assert handler._has_guardrail_params_changed(gid, new) is True
|
||||
|
||||
|
||||
def _all_callback_lists():
|
||||
import litellm
|
||||
|
||||
return [
|
||||
litellm.callbacks,
|
||||
litellm.success_callback,
|
||||
litellm.failure_callback,
|
||||
litellm._async_success_callback,
|
||||
litellm._async_failure_callback,
|
||||
]
|
||||
|
||||
|
||||
def test_delete_in_memory_guardrail_removes_callback_from_all_lists():
|
||||
"""
|
||||
Request handling promotes guardrail callbacks from litellm.callbacks into the
|
||||
success/failure/async lists. delete_in_memory_guardrail must purge the callback
|
||||
from every list, otherwise a re-initialized guardrail leaves its old instance
|
||||
stranded in those lists and instances accumulate.
|
||||
"""
|
||||
handler = InMemoryGuardrailHandler()
|
||||
callback = CustomGuardrail(
|
||||
guardrail_name="cf-delete",
|
||||
default_on=True,
|
||||
event_hook=GuardrailEventHooks.pre_call,
|
||||
)
|
||||
gid = "33333333-3333-3333-3333-333333333333"
|
||||
handler.IN_MEMORY_GUARDRAILS[gid] = _make_guardrail(gid, "cf-delete")
|
||||
handler._sources[gid] = "db"
|
||||
handler.guardrail_id_to_custom_guardrail[gid] = callback
|
||||
|
||||
lists = _all_callback_lists()
|
||||
snapshots = [list(cb_list) for cb_list in lists]
|
||||
try:
|
||||
for cb_list in lists:
|
||||
cb_list.append(callback)
|
||||
|
||||
handler.delete_in_memory_guardrail(gid)
|
||||
|
||||
for cb_list in lists:
|
||||
assert callback not in cb_list
|
||||
finally:
|
||||
for cb_list, snapshot in zip(lists, snapshots):
|
||||
cb_list[:] = snapshot
|
||||
|
||||
|
||||
def test_repeated_db_sync_does_not_accumulate_runner_instances():
|
||||
"""
|
||||
End-to-end regression for the OOM: across repeated DB polls (with the config
|
||||
genuinely changing each cycle to force re-initialization), exactly one live
|
||||
guardrail instance must exist across all callback lists. On the unfixed code
|
||||
the stale instance lingers in the success/failure lists and the distinct count
|
||||
climbs above one.
|
||||
"""
|
||||
import litellm
|
||||
|
||||
handler = InMemoryGuardrailHandler()
|
||||
gid = "44444444-4444-4444-4444-444444444444"
|
||||
name = "cf-accum"
|
||||
|
||||
def db_guardrail(word: str) -> Guardrail:
|
||||
params = {
|
||||
**_db_litellm_params(),
|
||||
"blocked_words": [{"keyword": word, "action": "BLOCK"}],
|
||||
}
|
||||
return Guardrail(guardrail_id=gid, guardrail_name=name, litellm_params=params)
|
||||
|
||||
def promote_into_request_lists() -> None:
|
||||
manager = litellm.logging_callback_manager
|
||||
for callback in list(litellm.callbacks):
|
||||
manager.add_litellm_success_callback(callback)
|
||||
manager.add_litellm_failure_callback(callback)
|
||||
manager.add_litellm_async_success_callback(callback)
|
||||
manager.add_litellm_async_failure_callback(callback)
|
||||
|
||||
def distinct_runner_instances() -> int:
|
||||
seen = set()
|
||||
for callback in litellm.logging_callback_manager._get_all_callbacks():
|
||||
if (
|
||||
isinstance(callback, CustomGuardrail)
|
||||
and getattr(callback, "guardrail_name", None) == name
|
||||
):
|
||||
seen.add(id(callback))
|
||||
return len(seen)
|
||||
|
||||
lists = _all_callback_lists()
|
||||
snapshots = [list(cb_list) for cb_list in lists]
|
||||
try:
|
||||
for cycle in range(5):
|
||||
handler.sync_guardrail_from_db(db_guardrail(f"word-{cycle}"))
|
||||
promote_into_request_lists()
|
||||
|
||||
assert distinct_runner_instances() == 1
|
||||
finally:
|
||||
for cb_list, snapshot in zip(lists, snapshots):
|
||||
cb_list[:] = snapshot
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue