Phase 2 — Resilience: crash & failure handling #273

Activity

1. thcp

   added

   reliabilityStability, resource management, race conditionsStability, resource management, race conditions

   on Jul 16, 2026
2. thcp commented on Jul 16, 2026

   

   thcp

   on Jul 16, 2026

   CollaboratorAuthor

   More actions

   Implementation plan — Phase 2: Resilience

   Findings covered: #276, #279, #281, #282, #284 (#285 is folded into Phase 3's PR-3A where the version counter that fixes it lives). 5 PRs. Depends on Phase 1's app/pipeline/errors.py (classifier + SeparationError) — land PR-1C first.

   ---

   PR-2A — GPU→CPU automatic fallback (#276) ★ flagship

   app/pipeline/separate.py refactor: extract the current Popen/watchdog/progress/cancel loop (lines 32–127) into

   def _run_demucs(job, source, job_dir, device) -> tuple[int, list[str]]:
       """One demucs attempt. Returns (returncode, stderr_tail). Raises JobCancelled."""

   separate() becomes the policy layer:

   1. device = get_demucs_device(); job.compute_device = device.
   2. rc, tail = _run_demucs(..., device); if rc == 0 → verify stems_root exists, return.
   3. If cancelled → JobCancelled (existing behavior).
   4. If device != "cpu": log logger.warning("[%s] demucs failed on %s (%s); retrying on CPU. tail:\n%s", job.id, device, classify_failure("\n".join(tail)), ...); clean partial output _rmtree(job_dir / DEMUCS_MODEL); _set(job, progress=0.0, stage="GPU failed — retrying on CPU (slower)..."); job.compute_device = f"cpu (fallback from {device})"; job.gpu_fallback = True (new Job field, persisted → visible in metadata + Library); rerun _run_demucs(..., "cpu").
   5. If the CPU attempt also fails (or device was already cpu): raise SeparationError(...) carrying both tails so the Phase 1 quarantine records the full story.

   Design choices (decided):

   • Fallback fires for every failure class on a GPU device, not just OOM — an unknown GPU failure is exactly the case field reports hit (Mac Mini). The classifier result is recorded, not used as a gate.
   • Fallback also applies when the user forced cuda/mps in Settings. The [Bug]: NVIDIA build still runs on CPU on machines with an NVIDIA GPU #247 lesson was against silent fallback; this one is loud three ways: stage message during the run, gpu_fallback/compute_device in metadata and job state, WARNING with full stderr tail in the log. A dead job with no diagnostics is strictly worse for a user who forced a device.
   • Progress resets to 0 on retry — honest, since CPU restarts from scratch.
   • The stall watchdog (TIMEOUT_DEMUCS_STALL) stays per-attempt.

   Testing seam: module-level _demucs_cmd(device, source, job_dir) -> list[str] that builds the current command; tests monkeypatch it to a stub script:

   • stub exits 1 printing "RuntimeError: MPS backend out of memory" when -d≠cpu, and on cpu writes <job_dir>/htdemucs_6s/<stem>/vocals.wav etc. then exits 0.
     Tests: fallback path produces stems + gpu_fallback=True + stage message seen; cpu-first failure raises without retry; cancel mid-first-attempt raises JobCancelled without a second attempt; double failure raises SeparationError with both tails.

   ---

   PR-2B — Download metadata probe retry + socket timeouts (#279)

   app/pipeline/download.py:

   • Extract the existing attempt/backoff loop (lines 268–289) into _with_retries(job, fn, *, what: str) reusing _MAX_RETRIES/_RETRY_BACKOFF/_is_retriable and the cancel-aware re-raise.
   • Wrap the metadata probe (line 228–231) in it: meta = _with_retries(job, lambda: ydl.extract_info(url, download=False), what="metadata") — construct the YoutubeDL inside the lambda (fresh instance per attempt).
   • Add "socket_timeout": 30 to all three ydl_opts dicts (metadata probe, audio download, _download_video_track).

   Tests: fake extractor raising a retriable error twice then succeeding → 3 calls, job not failed; non-retriable ("Private video") → immediate raise, 1 call.

   ---

   PR-2C — Registry persist race (#281)

   app/core/registry.py — persist(): two belts:

   1. Unique tmp name: path.with_name(f".registry.{uuid.uuid4().hex}.tmp") (mirrors _ensure_cached_mp3's pattern in stems.py).
   2. Extend the _lock scope to cover write+os.replace (payload is small; contention is nil). Wrap in try/except OSError: logger.warning(...) — persistence is already documented as best-effort elsewhere (settings.py:74); a failed persist must not kill the pipeline task. finally: tmp.unlink(missing_ok=True).

   Tests: 8 threads × 50 persist() calls with concurrent register()s → no exception, final file parses, no stray .tmp files left.

   ---

   PR-2D — Windows-correct graceful shutdown (#282)

   app/main.py:142: os.kill(os.getpid(), signal.SIGTERM) → signal.raise_signal(signal.SIGTERM).
   Rationale: on Windows os.kill(pid, SIGTERM) is TerminateProcess (hard kill, no uvicorn shutdown); signal.raise_signal triggers the in-process Python handler uvicorn installed via signal.signal, identical semantics on POSIX. One-line root-cause fix, comment updated to say why.
   Tests: monkeypatch signal.raise_signal, drive _desktop_parent_watchdog with a dead PID → assert called once. (Existing _process_exists tests unchanged.)

   ---

   PR-2E — Recover metadata-less completed jobs (#284)

   app/core/registry.py — _recover_done_job: when stems/ has WAVs but metadata.json is missing (the crash window between status=done and _write_metadata), recover instead of returning None:

   • title = f"Recovered track {job_dir.name[:6]}", other meta fields None, created_at from dir mtime (existing behavior).
   • Immediately write a minimal metadata.json (same shape as _write_metadata) so the next restart takes the normal path — self-healing, not a permanent special case.
     Ordering note: keep the existing rule that a dir with no stems at all is skipped; this only relaxes the metadata requirement.
     Tests: stems-without-metadata dir → recovered with placeholder title AND metadata.json now exists; empty dir still ignored.

   ---

   Sequencing & estimates

   2A (1d) is the priority — it closes the Mac Mini class of reports. Then 2B (0.5d), 2C (0.5d), 2D+2E (0.5d combined).
   Manual verification for 2A: on the NVIDIA machine, set demucs_device=cuda with CPU-only torch env (.venv-win today is exactly this) → job must complete on CPU with the fallback stage visible and gpu_fallback: true in /api/jobs/{id}.
3. thcp commented on Jul 17, 2026

   

   thcp

   on Jul 17, 2026

   CollaboratorAuthor

   More actions

   All 5 findings shipped and merged to main:

   PR	Closes	Commit
   #299	#276 (GPU→CPU fallback)	5355a93
   #300	#279	4a3ba0f
   #301	#281, #284	666005e
   #302	#282	1050789

   Phase 2 (Resilience) is complete. Closing.
4. thcp

   closed this as completedon Jul 17, 2026
5. thcp

   self-assigned this

   on Oct 1, 2026