Merge remote-tracking branch 'origin/main' into fix/1406-corrupt-weights
# Conflicts: # CHANGELOG.md
This commit is contained in:
@@ -42,6 +42,10 @@ The bundled TTS model package (`pyproject.toml`) is versioned independently.
|
||||
|
||||
- A model file damaged by an interrupted download now repairs itself instead of failing every generation with "Error while deserializing header: header too large". Only *missing* files were repaired before; one that arrived corrupt dead-ended as a raw 500. — thanks @overrunau! (#1406)
|
||||
- A slow machine is no longer told its IndexTTS-2 install isn't there. The check that confirms an engine's virtualenv gave up after 10 seconds and counted that as a broken install, so a cold first run 500'd; it now waits longer and treats slow as unproven, not broken. — thanks @OracleNightmare! (#1414)
|
||||
- A generation abandoned while stuck on an internal lock now says so, instead of blaming your hardware and suggesting shorter text. Nothing had been computed, so none of that advice applied. (#1416, #1419)
|
||||
- A slow machine is no longer told its IndexTTS-2 install isn't there. The check that confirms an engine's virtualenv gave up after 10 seconds and counted that as a broken install, so a cold first run 500'd; it now waits longer and treats slow as unproven, not broken. (#1414) — thanks @OracleNightmare!
|
||||
- A broken Python environment now says so, instead of blaming the app's own install. A missing or mismatched torch/transformers surfaced as "omnivoice not importable" and sent people reinstalling the wrong thing. (#1415)
|
||||
- A model that fails to load at startup no longer leaves the app looking healthy while producing nothing — the failure and its remedy now show up in the model status. (#1415)
|
||||
- Generating with the default engine works again on everything built from `main` since the rename — source checkouts, preview builds and Docker `:latest` all run the same backend, whose model import had been rewritten to a class name the library doesn't export, failing every generation with "cannot import name 'VoiceStudio'". The class keeps its library name, and a guard test now pins it. (#1420)
|
||||
- Running from source no longer dies at startup when a database migration is pending. Alembic resolved the migrations folder relative to wherever the app was launched from — fine from the repo root, fatal from the desktop shell (`tauri dev`), which reported "Path doesn't exist: backend/migrations" and stopped. The path is now anchored to the repo, wherever you start it. (#1420)
|
||||
- The first generation after startup no longer stalls or 500s while the model is still loading. A cold load reached from a worker thread waited on a lock owned by a different event loop, which either errored outright or deadlocked until the job was abandoned. (#1417)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
import time
|
||||
import asyncio
|
||||
@@ -25,6 +26,36 @@ def _lazy_torch():
|
||||
return _torch
|
||||
|
||||
|
||||
def _missing_module_is_omnivoice(exc: ModuleNotFoundError) -> bool:
|
||||
"""True when *exc* says the ``omnivoice`` package itself is not importable.
|
||||
|
||||
``ModuleNotFoundError`` is raised for two very different situations along
|
||||
this import, and only one of them is fixable by putting the source tree on
|
||||
``sys.path`` (#1415):
|
||||
|
||||
* ``omnivoice`` (or a submodule of it) is genuinely absent — a missing or
|
||||
broken editable install, which the #564 fallback repairs; ``exc.name``
|
||||
names the omnivoice package.
|
||||
* something ``omnivoice`` imports is absent or broken — a torch /
|
||||
torchaudio / torchvision mismatch, or transformers' lazy module refusing
|
||||
an attribute whose backing import failed
|
||||
("Could not import module 'AutoFeatureExtractor'", which carries no
|
||||
``name`` at all). Nothing about ``sys.path`` is wrong here.
|
||||
|
||||
Treating the second as the first re-imported from the same broken
|
||||
environment, failed identically, and logged that the editable install was
|
||||
missing — a confident diagnosis of the wrong component.
|
||||
|
||||
``exc.name`` is the authority, and its absence is decisive rather than
|
||||
unknown: the stdlib always sets it, so a ModuleNotFoundError without one
|
||||
was raised by hand — which is exactly what transformers' lazy module does.
|
||||
"""
|
||||
name = getattr(exc, "name", None)
|
||||
if not name:
|
||||
return False
|
||||
return name == "omnivoice" or name.startswith("omnivoice.")
|
||||
|
||||
|
||||
def _lazy_omnivoice():
|
||||
global _OmniVoice
|
||||
if _OmniVoice is None:
|
||||
@@ -33,7 +64,21 @@ def _lazy_omnivoice():
|
||||
# branding. The VoiceStudio rename must not touch it (checkpoint
|
||||
# configs reference the class name via transformers architectures).
|
||||
from omnivoice.models.omnivoice import OmniVoice as _OV
|
||||
except ModuleNotFoundError:
|
||||
except ModuleNotFoundError as exc:
|
||||
if not _missing_module_is_omnivoice(exc):
|
||||
# Something in omnivoice's OWN import chain is missing — not
|
||||
# omnivoice itself (#1415). transformers' lazy module raises
|
||||
# ModuleNotFoundError for any attribute whose backing import
|
||||
# failed ("Could not import module 'AutoFeatureExtractor'"),
|
||||
# and a missing torchaudio/torchvision raises it by name. The
|
||||
# source-tree fallback below cannot fix any of those: it
|
||||
# re-imports from the same broken environment and fails
|
||||
# identically, having logged that the *editable install* is
|
||||
# broken — which sent the reporter, and us, after the wrong
|
||||
# thing. Let it through with its own cause intact; classify()
|
||||
# already names it TRANSFORMERS_IMPORT and hints at the real
|
||||
# remedy.
|
||||
raise
|
||||
# The venv's editable install is missing/broken (#564). main.py wires
|
||||
# the source fallback at startup, but resolve it here too so the
|
||||
# model-load path self-heals and logs the paths it searched.
|
||||
@@ -719,7 +764,7 @@ async def run_on_gpu_pool_guarded(fn, *, what: str = "GPU job",
|
||||
# Capture the stacks BEFORE reset(): reset() replaces the executor, and
|
||||
# once the wedged thread is no longer a pool worker we can no longer
|
||||
# tell it apart from any other thread in the process.
|
||||
log_gpu_pool_worker_stacks(what, timeout, executor=ex)
|
||||
stacks = log_gpu_pool_worker_stacks(what, timeout, executor=ex)
|
||||
_reset = getattr(ex, "reset", None)
|
||||
if callable(_reset):
|
||||
try:
|
||||
@@ -734,7 +779,9 @@ async def run_on_gpu_pool_guarded(fn, *, what: str = "GPU job",
|
||||
logger.exception("GPU pool reset after %s timeout failed",
|
||||
_log_safe(what))
|
||||
raise GpuJobTimeoutError(
|
||||
_timeout_guidance(what, timeout, min_vram_gb)
|
||||
_timeout_guidance(
|
||||
what, timeout, min_vram_gb, wedged=_stack_shows_a_wedge(stacks),
|
||||
)
|
||||
) from timeout_exc
|
||||
|
||||
|
||||
@@ -847,7 +894,73 @@ def log_gpu_pool_worker_stacks(what: str, timeout: float, executor=None) -> str:
|
||||
return ""
|
||||
|
||||
|
||||
def _timeout_guidance(what: str, timeout: float, min_vram_gb: float = 0.0) -> str:
|
||||
#: Standard-library modules whose blocking primitives a wedged worker parks in.
|
||||
#: Matched on the *file* of the deepest frame, so a user function that happens
|
||||
#: to be named ``wait`` or ``result`` cannot be mistaken for one of these.
|
||||
_WEDGE_STDLIB_FILES = (
|
||||
"/threading.py", "\\threading.py",
|
||||
"/asyncio/locks.py", "\\asyncio\\locks.py",
|
||||
"/concurrent/futures/_base.py", "\\concurrent\\futures\\_base.py",
|
||||
"/queue.py", "\\queue.py",
|
||||
)
|
||||
|
||||
#: Blocking entry points within those modules. A thread sitting in one of these
|
||||
#: is waiting on another thread, by definition — there is no slow-but-working
|
||||
#: interpretation of it.
|
||||
_WEDGE_FUNCTIONS = frozenset({
|
||||
"acquire", "wait", "result", "get", "join", "_wait_for_tstate_lock",
|
||||
})
|
||||
|
||||
_FRAME_HEAD = re.compile(r'^\s*File "(?P<file>.+)", line \d+, in (?P<func>\S+)\s*$')
|
||||
|
||||
|
||||
def _stack_shows_a_wedge(stacks: "str | None") -> bool:
|
||||
"""True when the abandoned worker's DEEPEST frame is a blocking wait.
|
||||
|
||||
The message this feeds is the one users actually read, and for years it
|
||||
said the same thing whatever happened: "too heavy for the available
|
||||
compute". That is a specific, testable claim, and when the worker is
|
||||
parked on a lock it is simply false — nothing was computed, so nothing was
|
||||
too heavy. #1416 and #1419 both arrived as "my machine is too slow"
|
||||
reports from people whose jobs never ran at all (a cold load waiting on a
|
||||
lock owned by another event loop, #1417), and #1329 is the same wedge seen
|
||||
from the dub loop. Every one of them was sent to look at their hardware.
|
||||
|
||||
Only the last frame counts, and it must be a blocking primitive in a
|
||||
standard-library module. Both halves matter (CodeRabbit): a compute job's
|
||||
*callers* routinely include a lock it has already left, so scanning the
|
||||
whole stack would flag nearly everything; and an application function
|
||||
named ``wait`` or ``result`` is not evidence of anything, so the function
|
||||
name alone is not enough either.
|
||||
|
||||
Reads the text :func:`log_gpu_pool_worker_stacks` already captured — no
|
||||
second stack walk, and no cost at all on the healthy path.
|
||||
|
||||
Conservative: unknown or unparseable stacks return False and keep the old
|
||||
wording. Claiming a hang we cannot see would be the same mistake pointing
|
||||
the other way.
|
||||
"""
|
||||
if not stacks:
|
||||
return False
|
||||
deepest = None
|
||||
for line in str(stacks).splitlines():
|
||||
m = _FRAME_HEAD.match(line)
|
||||
if m:
|
||||
deepest = m
|
||||
if deepest is None:
|
||||
return False
|
||||
func = deepest.group("func")
|
||||
if func not in _WEDGE_FUNCTIONS:
|
||||
return False
|
||||
path = deepest.group("file").replace("\\", "/")
|
||||
return any(
|
||||
path.endswith(tail.replace("\\", "/")) for tail in _WEDGE_STDLIB_FILES
|
||||
)
|
||||
|
||||
|
||||
def _timeout_guidance(
|
||||
what: str, timeout: float, min_vram_gb: float = 0.0, *, wedged: bool = False,
|
||||
) -> str:
|
||||
"""Device-aware timeout message (#896): a CPU-only host must never be told
|
||||
to "set the engine to CPU" or blamed on VRAM — on CPU the job is simply
|
||||
compute-bound. GPU hosts keep the VRAM-contention guidance.
|
||||
@@ -869,6 +982,20 @@ def _timeout_guidance(what: str, timeout: float, min_vram_gb: float = 0.0) -> st
|
||||
device_name, vram_gb = _caps.device_name, _caps.vram_gb
|
||||
except Exception: # noqa: BLE001 — guidance must never mask the timeout
|
||||
pass
|
||||
if wedged:
|
||||
# The worker spent the whole budget parked on a lock. None of the
|
||||
# hardware advice below applies — shorter text and a lighter engine
|
||||
# cannot speed up a job that never started (#1416/#1419/#1329).
|
||||
return (
|
||||
f"{what} was abandoned after {timeout:.0f}s without doing any "
|
||||
"work — it spent the whole time waiting on an internal lock, not "
|
||||
"computing. This is a bug in VoiceStudio, not a limit of your "
|
||||
"machine, so shorter text or a lighter engine won't help. "
|
||||
"Restart the backend to clear it (Settings → Logs → Backend has "
|
||||
"the stack trace that was captured), and please report it with "
|
||||
"that log at https://github.com/debpalash/VoiceStudio/issues — "
|
||||
"the trace names exactly where it stopped."
|
||||
)
|
||||
common = (
|
||||
f"{what} ran for more than {timeout:.0f}s of actual compute time and "
|
||||
"was abandoned — the backend is running, but this job was too heavy "
|
||||
@@ -2335,6 +2462,39 @@ async def preload_model():
|
||||
# distinguishes a real dependency problem from a shutdown-interrupted
|
||||
# import.
|
||||
logger.warning("Model preload failed (non-fatal): %s", e, exc_info=e)
|
||||
# Non-fatal must not mean invisible (#1415). A broken dependency in the
|
||||
# model's import chain fails here and nowhere else until the user tries
|
||||
# to generate — so the app starts clean, reports itself healthy, and
|
||||
# simply produces nothing, which is how the reporter's environment
|
||||
# looked. Record it on the status the UI already reads, with the
|
||||
# classified remedy attached; the next successful load clears it.
|
||||
try:
|
||||
from core.failure import build_failure
|
||||
|
||||
from core.failure import describe_exception
|
||||
|
||||
# The whole chain, not just the surface: transformers reports a
|
||||
# broken dependency as a lazy-attribute error and keeps the real
|
||||
# cause in __cause__, so classifying the outermost message alone
|
||||
# loses the only part that names a remedy.
|
||||
reason = " | ".join(
|
||||
describe_exception(exc) for exc in _exception_chain(e)
|
||||
) or describe_exception(e)
|
||||
failure = build_failure(
|
||||
reason, stage="model-preload", include_diagnostic=False,
|
||||
)
|
||||
detail = failure.get("hint") or failure.get("reason") or str(e)
|
||||
except Exception: # noqa: BLE001 — never lose the warning to this
|
||||
# NOT str(e): the whole point of build_failure is that it sanitizes,
|
||||
# and an exception message routinely carries absolute paths — i.e.
|
||||
# the user's account name — which this string is about to publish
|
||||
# through /model/status (CWE-532; CodeRabbit). A fixed message that
|
||||
# points at the log beats leaking one into the API.
|
||||
detail = (
|
||||
"The TTS model could not be loaded. Settings → Logs → Backend "
|
||||
"has the full error."
|
||||
)
|
||||
_set_loading("failed", detail, error=detail)
|
||||
|
||||
def get_model_status():
|
||||
is_loaded = model is not None
|
||||
|
||||
@@ -0,0 +1,239 @@
|
||||
"""A broken dependency is not a missing install, and must not be silent (#1415).
|
||||
|
||||
The reporter's backend logged this at startup::
|
||||
|
||||
ModuleNotFoundError: Could not import module 'AutoFeatureExtractor'.
|
||||
Are this object's requirements defined correctly?
|
||||
… in _lazy_omnivoice
|
||||
from omnivoice.models.omnivoice import OmniVoice as _OV # line 40
|
||||
|
||||
Line 40 is the *second* import — the one after the #564 source-tree fallback —
|
||||
which proves the fallback fired. It should never have: nothing about
|
||||
``sys.path`` was wrong. transformers' lazy module raises
|
||||
``ModuleNotFoundError`` for any attribute whose backing import failed, so a
|
||||
broken torch/torchaudio/transformers environment arrives wearing the exact
|
||||
exception type that means "the editable install of omnivoice is missing". The
|
||||
handler re-imported from the same broken environment, failed identically, and
|
||||
logged that the *editable install* was broken — sending the reporter, and us,
|
||||
after the wrong component.
|
||||
|
||||
The second half is that the failure was invisible. Preload is deliberately
|
||||
non-fatal, and nothing else touches the model until the user generates — so
|
||||
the app starts clean, reports itself healthy, and simply produces nothing.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mm():
|
||||
"""Resolved at run time, not import time: a module-level import of an app
|
||||
module keeps mutable state in sys.modules across test boundaries."""
|
||||
import services.model_manager as _mm
|
||||
|
||||
return _mm
|
||||
|
||||
|
||||
# ── which failures are actually "omnivoice is missing"? ───────────────────
|
||||
|
||||
def _mnfe(msg: str, name: str | None = None) -> ModuleNotFoundError:
|
||||
exc = ModuleNotFoundError(msg)
|
||||
if name is not None:
|
||||
exc.name = name
|
||||
return exc
|
||||
|
||||
|
||||
def test_a_missing_omnivoice_is_recognised(mm):
|
||||
assert mm._missing_module_is_omnivoice(_mnfe("No module named 'omnivoice'", "omnivoice"))
|
||||
|
||||
|
||||
def test_a_missing_omnivoice_submodule_is_recognised(mm):
|
||||
assert mm._missing_module_is_omnivoice(
|
||||
_mnfe("No module named 'omnivoice.models'", "omnivoice.models")
|
||||
)
|
||||
|
||||
|
||||
def test_the_transformers_lazy_attribute_error_is_not(mm):
|
||||
"""The reported failure. It carries no `name` at all, because transformers
|
||||
raises it by hand rather than through the import machinery — which is
|
||||
precisely what distinguishes it from a real missing module."""
|
||||
exc = _mnfe(
|
||||
"Could not import module 'AutoFeatureExtractor'. "
|
||||
"Are this object's requirements defined correctly?"
|
||||
)
|
||||
assert exc.name is None
|
||||
assert not mm._missing_module_is_omnivoice(exc)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", ["torchaudio", "torchvision", "transformers", "torch"])
|
||||
def test_a_missing_dependency_of_omnivoice_is_not(mm, name):
|
||||
"""A torch/torchvision mismatch fails by name — a real module, just not
|
||||
ours. Putting the omnivoice source on sys.path cannot help."""
|
||||
assert not mm._missing_module_is_omnivoice(
|
||||
_mnfe(f"No module named '{name}'", name)
|
||||
)
|
||||
|
||||
|
||||
def test_a_lookalike_package_name_is_not_ours(mm):
|
||||
assert not mm._missing_module_is_omnivoice(
|
||||
_mnfe("No module named 'omnivoiceX'", "omnivoiceX")
|
||||
)
|
||||
|
||||
|
||||
# ── the fallback fires only when it can help ──────────────────────────────
|
||||
|
||||
def test_a_broken_dependency_does_not_trigger_the_source_fallback(mm, monkeypatch):
|
||||
"""Fail-before: this called ensure_omnivoice_importable, re-imported from
|
||||
the same broken environment, and re-raised having logged the wrong cause."""
|
||||
monkeypatch.setattr(mm, "_OmniVoice", None, raising=False)
|
||||
called = []
|
||||
|
||||
import core.omnivoice_path as omnivoice_path
|
||||
|
||||
monkeypatch.setattr(
|
||||
omnivoice_path, "ensure_omnivoice_importable",
|
||||
lambda *a, **kw: called.append(a),
|
||||
)
|
||||
|
||||
boom = _mnfe("Could not import module 'AutoFeatureExtractor'.")
|
||||
import builtins
|
||||
|
||||
real_import = builtins.__import__
|
||||
|
||||
def _fake_import(name, *a, **kw):
|
||||
if name == "omnivoice.models.omnivoice":
|
||||
raise boom
|
||||
return real_import(name, *a, **kw)
|
||||
|
||||
monkeypatch.setattr(builtins, "__import__", _fake_import)
|
||||
|
||||
with pytest.raises(ModuleNotFoundError) as caught:
|
||||
mm._lazy_omnivoice()
|
||||
assert caught.value is boom, "the original cause must survive untouched"
|
||||
assert called == [], (
|
||||
"the source-tree fallback ran for a failure it cannot fix — that is "
|
||||
"what blamed the editable install for a broken transformers (#1415)"
|
||||
)
|
||||
|
||||
|
||||
def test_a_genuinely_missing_omnivoice_still_triggers_the_fallback(mm, monkeypatch):
|
||||
"""No weakening of the #564 repair this guard sits in front of."""
|
||||
monkeypatch.setattr(mm, "_OmniVoice", None, raising=False)
|
||||
called = []
|
||||
|
||||
import core.omnivoice_path as omnivoice_path
|
||||
|
||||
monkeypatch.setattr(
|
||||
omnivoice_path, "ensure_omnivoice_importable",
|
||||
lambda *a, **kw: called.append(a),
|
||||
)
|
||||
|
||||
import builtins
|
||||
|
||||
real_import = builtins.__import__
|
||||
attempts = {"n": 0}
|
||||
|
||||
def _fake_import(name, *a, **kw):
|
||||
if name == "omnivoice.models.omnivoice":
|
||||
attempts["n"] += 1
|
||||
raise _mnfe("No module named 'omnivoice'", "omnivoice")
|
||||
return real_import(name, *a, **kw)
|
||||
|
||||
monkeypatch.setattr(builtins, "__import__", _fake_import)
|
||||
|
||||
with pytest.raises(ModuleNotFoundError):
|
||||
mm._lazy_omnivoice()
|
||||
assert called, "the #564 source-tree fallback must still run for its own case"
|
||||
assert attempts["n"] == 2, "the import must be retried after the fallback"
|
||||
|
||||
|
||||
# ── a non-fatal preload failure is still visible ──────────────────────────
|
||||
|
||||
def test_a_failed_preload_shows_up_in_the_status(mm, monkeypatch):
|
||||
"""Fail-before: the status stayed "idle" with no error, so the app looked
|
||||
healthy and produced nothing until a generation failed much later."""
|
||||
import asyncio
|
||||
|
||||
monkeypatch.setattr(mm, "model", None, raising=False)
|
||||
monkeypatch.setattr(mm, "resolve_omnivoice_checkpoint", lambda: "org/model")
|
||||
monkeypatch.setattr(mm, "_checkpoint_in_local_cache", lambda *a, **kw: True)
|
||||
|
||||
async def _boom():
|
||||
raise _mnfe(
|
||||
"Could not import module 'AutoFeatureExtractor'. "
|
||||
"Are this object's requirements defined correctly?"
|
||||
)
|
||||
|
||||
monkeypatch.setattr(mm, "_load_model_with_timeout", _boom)
|
||||
|
||||
asyncio.run(mm.preload_model())
|
||||
|
||||
status = mm.get_model_status()
|
||||
assert status["status"] != "ready"
|
||||
assert status.get("error"), "a failed preload left no trace on the status"
|
||||
# The classified remedy, not the raw lazy-attribute wording: this class is
|
||||
# TRANSFORMERS_IMPORT, whose hint names the reinstall that actually fixes it.
|
||||
assert "transformers" in status["error"].lower()
|
||||
|
||||
|
||||
def test_a_successful_preload_clears_a_previous_failure(mm, monkeypatch):
|
||||
"""Driven through `preload_model()` rather than `_set_loading`, so it can
|
||||
only pass if the real success path actually clears the error a previous
|
||||
failure left behind (CodeRabbit)."""
|
||||
import asyncio
|
||||
|
||||
mm._set_loading("failed", "something broke", error="something broke")
|
||||
assert mm.get_model_status().get("error")
|
||||
|
||||
monkeypatch.setattr(mm, "model", None, raising=False)
|
||||
monkeypatch.setattr(mm, "resolve_omnivoice_checkpoint", lambda: "org/model")
|
||||
monkeypatch.setattr(mm, "_checkpoint_in_local_cache", lambda *a, **kw: True)
|
||||
|
||||
loaded = object()
|
||||
|
||||
async def _ok():
|
||||
mm._set_loading("ready", "Model ready", progress=100)
|
||||
return loaded
|
||||
|
||||
monkeypatch.setattr(mm, "_load_model_with_timeout", _ok)
|
||||
try:
|
||||
asyncio.run(mm.preload_model())
|
||||
assert mm.get_model_status()["status"] == "ready"
|
||||
assert not mm.get_model_status().get("error")
|
||||
finally:
|
||||
monkeypatch.setattr(mm, "model", None, raising=False)
|
||||
mm._set_loading("", "")
|
||||
|
||||
|
||||
def test_the_fallback_detail_does_not_leak_a_path(mm, monkeypatch):
|
||||
"""If building the classified failure itself fails, what lands on the
|
||||
status must not be the raw exception — those carry absolute paths, i.e.
|
||||
the user's account name, and this string is published through
|
||||
/model/status (CodeRabbit)."""
|
||||
import asyncio
|
||||
|
||||
monkeypatch.setattr(mm, "model", None, raising=False)
|
||||
monkeypatch.setattr(mm, "resolve_omnivoice_checkpoint", lambda: "org/model")
|
||||
monkeypatch.setattr(mm, "_checkpoint_in_local_cache", lambda *a, **kw: True)
|
||||
|
||||
secret = "/Users/somebody/models/OmniVoice/.venv/lib/x.py"
|
||||
|
||||
async def _boom():
|
||||
raise RuntimeError(f"failed at {secret}")
|
||||
|
||||
monkeypatch.setattr(mm, "_load_model_with_timeout", _boom)
|
||||
|
||||
import core.failure as cf
|
||||
|
||||
monkeypatch.setattr(
|
||||
cf, "build_failure",
|
||||
lambda *a, **kw: (_ for _ in ()).throw(ValueError("classifier broke")),
|
||||
)
|
||||
|
||||
asyncio.run(mm.preload_model())
|
||||
error = mm.get_model_status().get("error", "")
|
||||
assert error, "the failure still has to be visible"
|
||||
assert secret not in error
|
||||
assert "somebody" not in error
|
||||
assert "Logs" in error
|
||||
@@ -0,0 +1,159 @@
|
||||
"""A job that never ran was not "too heavy for the available compute" (#1416).
|
||||
|
||||
When a GPU-pool job overruns its budget the user gets one message, and until
|
||||
now it made the same claim whatever had happened: the job "ran for more than
|
||||
300s of actual compute time" and "was too heavy for the available compute",
|
||||
followed by advice about shorter text, lighter engines and VRAM.
|
||||
|
||||
That is a specific, testable claim, and it is false whenever the worker spent
|
||||
its budget parked on a lock. #1416 (MPS, 16 GB) and #1419 (CPU, Windows) both
|
||||
arrived as "my machine is too slow" reports from people whose jobs never
|
||||
started — a cold model load waiting on a lock owned by a different event loop
|
||||
(#1417) — and #1329's "advances one sentence then stops with no error" is the
|
||||
same wedge seen from the dub loop. All three were sent to look at hardware.
|
||||
|
||||
The stack of every pool worker is already captured at the moment of the
|
||||
timeout (#1338). These pin that it is now *read* as well as logged, and that
|
||||
the reading is conservative in the direction that matters.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mm():
|
||||
"""Resolved at run time, not import time: a module-level import of an app
|
||||
module keeps mutable state in sys.modules across test boundaries."""
|
||||
import services.model_manager as _mm
|
||||
|
||||
return _mm
|
||||
|
||||
|
||||
# Shapes taken from real captures: a thread blocked in a lock/event/future.
|
||||
WEDGED_STACKS = [
|
||||
# asyncio.Lock.acquire on a foreign loop — the #1417 deadlock.
|
||||
' File "/usr/lib/python3.11/asyncio/locks.py", line 114, in acquire\n'
|
||||
" await fut\n",
|
||||
# threading.Event.wait / Condition.wait.
|
||||
' File "/usr/lib/python3.11/threading.py", line 327, in wait\n'
|
||||
" waiter.acquire()\n",
|
||||
# concurrent.futures — a pool job waiting on the pool it occupies.
|
||||
' File "/usr/lib/python3.11/concurrent/futures/_base.py", line 451, in result\n'
|
||||
" self._condition.wait(timeout)\n",
|
||||
# A plain threading.Lock.
|
||||
' File "/usr/lib/python3.11/threading.py", line 604, in acquire\n'
|
||||
" self._block.acquire()\n",
|
||||
]
|
||||
|
||||
# A worker that really is grinding: deepest frame inside the model.
|
||||
COMPUTING_STACK = (
|
||||
' File "/app/backend/services/model_manager.py", line 1801, in _load\n'
|
||||
" return VoiceStudio.from_pretrained(checkpoint)\n"
|
||||
' File "/app/omnivoice/models/omnivoice.py", line 812, in generate\n'
|
||||
" audio = self.llm.forward(tokens)\n"
|
||||
' File "/app/.venv/lib/python3.11/site-packages/torch/nn/modules/module.py", '
|
||||
'line 1553, in _call_impl\n'
|
||||
" return forward_call(*args, **kwargs)\n"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("stack", WEDGED_STACKS)
|
||||
def test_a_parked_worker_is_recognised(mm, stack):
|
||||
assert mm._stack_shows_a_wedge(stack)
|
||||
|
||||
|
||||
def test_a_computing_worker_is_not(mm):
|
||||
"""The false positive that matters: telling someone with a genuinely slow
|
||||
machine that they found a bug would send them to the issue tracker instead
|
||||
of to the shorter-text/lighter-engine advice that would actually help."""
|
||||
assert not mm._stack_shows_a_wedge(COMPUTING_STACK)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("stack", ["", None])
|
||||
def test_no_stack_means_no_claim(mm, stack):
|
||||
"""Unreadable stacks keep the old wording. Claiming a hang we cannot see is
|
||||
the same mistake pointing the other way."""
|
||||
assert not mm._stack_shows_a_wedge(stack)
|
||||
|
||||
|
||||
def test_a_lock_the_worker_already_left_does_not_count(mm):
|
||||
"""Only the deepest frames decide. A compute job's callers routinely
|
||||
include a lock acquisition it has since returned from — reading the whole
|
||||
stack would flag almost every job."""
|
||||
stack = (
|
||||
' File "/usr/lib/python3.11/threading.py", line 604, in acquire\n'
|
||||
" self._block.acquire()\n"
|
||||
) + COMPUTING_STACK * 6
|
||||
assert not mm._stack_shows_a_wedge(stack)
|
||||
|
||||
|
||||
# ── the message ────────────────────────────────────────────────────────────
|
||||
|
||||
def test_a_wedge_does_not_blame_the_machine(mm):
|
||||
msg = mm._timeout_guidance("TTS generate", 300.0, wedged=True)
|
||||
assert "too heavy for the available compute" not in msg
|
||||
assert "not a limit of your machine" in msg
|
||||
# None of the hardware remedies: they cannot speed up a job that never ran.
|
||||
for useless in ("shorter text", "lighter engine", "VRAM", "Flush caches"):
|
||||
assert useless not in msg or "won't help" in msg
|
||||
# It must give the one thing that does help.
|
||||
assert "restart the backend" in msg.lower()
|
||||
assert "github.com/debpalash/VoiceStudio/issues" in msg
|
||||
|
||||
|
||||
def test_a_genuinely_slow_job_keeps_its_advice(mm):
|
||||
"""No regression for the case the message was written for."""
|
||||
msg = mm._timeout_guidance("TTS generate", 300.0, wedged=False)
|
||||
assert "too heavy for the available compute" in msg
|
||||
assert "shorter text" in msg or "lighter engine" in msg
|
||||
|
||||
|
||||
def test_the_default_is_the_old_wording(mm):
|
||||
"""Every existing caller that hasn't been taught about wedges keeps its
|
||||
behaviour — the new branch is opt-in from the one site that has evidence."""
|
||||
assert mm._timeout_guidance("TTS generate", 300.0) == mm._timeout_guidance(
|
||||
"TTS generate", 300.0, wedged=False
|
||||
)
|
||||
|
||||
|
||||
def test_an_app_function_named_wait_is_not_a_wedge(mm):
|
||||
"""`in wait` / `in result` say nothing on their own — plenty of application
|
||||
code has functions by those names, and flagging them would tell a user with
|
||||
a genuinely slow machine that they had found a bug (CodeRabbit)."""
|
||||
stack = (
|
||||
' File "/app/backend/services/dub_pipeline.py", line 88, in wait\n'
|
||||
" self.poll_until_done()\n"
|
||||
)
|
||||
assert not mm._stack_shows_a_wedge(stack)
|
||||
|
||||
|
||||
def test_a_third_party_result_function_is_not_a_wedge(mm):
|
||||
stack = (
|
||||
' File "/app/.venv/lib/python3.11/site-packages/somelib/api.py", '
|
||||
'line 12, in result\n'
|
||||
" return self._compute()\n"
|
||||
)
|
||||
assert not mm._stack_shows_a_wedge(stack)
|
||||
|
||||
|
||||
def test_a_stdlib_frame_that_is_not_a_blocking_call_is_not_a_wedge(mm):
|
||||
stack = (
|
||||
' File "/usr/lib/python3.11/threading.py", line 1002, in _bootstrap\n'
|
||||
" self._bootstrap_inner()\n"
|
||||
)
|
||||
assert not mm._stack_shows_a_wedge(stack)
|
||||
|
||||
|
||||
def test_windows_stdlib_paths_are_recognised(mm):
|
||||
"""The reporters are on Windows and macOS; a POSIX-only path match would
|
||||
quietly never fire for half of them."""
|
||||
stack = (
|
||||
' File "C:\\Python311\\Lib\\threading.py", line 327, in wait\n'
|
||||
" waiter.acquire()\n"
|
||||
)
|
||||
assert mm._stack_shows_a_wedge(stack)
|
||||
|
||||
|
||||
def test_unparseable_text_is_not_a_claim(mm):
|
||||
assert not mm._stack_shows_a_wedge("something that is not a stack at all")
|
||||
Reference in New Issue
Block a user