fix(openai-compat): unload the outgoing engine on explicit-ID switches
Review follow-up (Greptile/CodeRabbit on #1614): caching instances without a switch rule would let each distinct explicit engine ID stay resident, accumulating sidecars / multi-GB in-process models. Mirror get_active_tts_backend's MM2-01 switch rule: a different explicit ID (omnivoice included, which resolves to the active engine) unloads the outgoing instance first, best-effort.
This commit is contained in:
@@ -158,6 +158,25 @@ _OPENAI_VOICE_ALIASES = {
|
||||
# ── TTS: POST /v1/audio/speech ──────────────────────────────────────────────
|
||||
|
||||
|
||||
#: Last engine explicitly requested via a `model` ID on this route, and its
|
||||
#: instance — mirrors get_active_tts_backend's switch rule (MM2-01): loading a
|
||||
#: different explicit engine unloads the outgoing one first, so cached explicit
|
||||
#: IDs can't accumulate multi-GB in-process models / sidecars.
|
||||
_explicit_engine: dict = {}
|
||||
|
||||
|
||||
def _unload_explicit_engine() -> None:
|
||||
inst = _explicit_engine.get("instance")
|
||||
if inst is None:
|
||||
return
|
||||
try:
|
||||
inst.unload()
|
||||
except Exception as exc: # noqa: BLE001 — a bad unload must not block a switch
|
||||
logger.warning("explicit engine switch: %s.unload() raised: %s",
|
||||
type(inst).__name__, exc)
|
||||
_explicit_engine.clear()
|
||||
|
||||
|
||||
def _resolve_engine(model_id: str):
|
||||
"""Map an OpenAI model name to a VoiceStudio backend."""
|
||||
from services.tts_backend import (
|
||||
@@ -179,11 +198,20 @@ def _resolve_engine(model_id: str):
|
||||
)
|
||||
from services.tts_backend import OmniVoiceBackend
|
||||
if cls is OmniVoiceBackend:
|
||||
# OmniVoice only ever runs as the shared active engine — the
|
||||
# explicit-omnivoice request is the active-engine request.
|
||||
_unload_explicit_engine()
|
||||
return get_active_tts_backend()
|
||||
# Cached singleton, not a fresh cls(): SubprocessBackend engines would
|
||||
# spawn a sidecar process and reload their model on EVERY request, and
|
||||
# register a new atexit hook each time (get_engine_instance's contract).
|
||||
return get_engine_instance_for(model_id)
|
||||
# And on a switch between explicit IDs, unload the outgoing engine so
|
||||
# the cache can't accumulate residents (same rule as the active path).
|
||||
if _explicit_engine.get("id") != model_id:
|
||||
_unload_explicit_engine()
|
||||
_explicit_engine["id"] = model_id
|
||||
_explicit_engine["instance"] = get_engine_instance_for(model_id)
|
||||
return _explicit_engine["instance"]
|
||||
except ValueError:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
|
||||
@@ -62,3 +62,39 @@ def test_unknown_engine_id_still_400s(oc, monkeypatch):
|
||||
oc._resolve_engine("not-an-engine")
|
||||
assert exc.value.status_code == 400
|
||||
assert "Unknown model" in exc.value.detail
|
||||
|
||||
|
||||
def test_switching_explicit_engine_ids_unloads_the_outgoing_one(oc, monkeypatch):
|
||||
"""Cache must not accumulate residents: a different explicit `model` ID
|
||||
unloads the outgoing engine first (same switch rule as the active-engine
|
||||
path, MM2-01)."""
|
||||
svc = _tts_mod()
|
||||
unloaded = []
|
||||
|
||||
def _make(name):
|
||||
class _B:
|
||||
ident = name
|
||||
|
||||
def __init__(self):
|
||||
pass
|
||||
|
||||
@staticmethod
|
||||
def is_available():
|
||||
return True, "ok"
|
||||
|
||||
def unload(self):
|
||||
unloaded.append(name)
|
||||
|
||||
return _B
|
||||
|
||||
engines = {"a-engine": _make("a"), "b-engine": _make("b")}
|
||||
monkeypatch.setattr(svc, "get_backend_class", lambda i: engines[i])
|
||||
|
||||
first = oc._resolve_engine("a-engine")
|
||||
second = oc._resolve_engine("a-engine")
|
||||
assert first is second
|
||||
assert unloaded == []
|
||||
|
||||
third = oc._resolve_engine("b-engine")
|
||||
assert third is not first
|
||||
assert unloaded == ["a"]
|
||||
|
||||
Reference in New Issue
Block a user