commit cd65b7c6384ae2d9e5cf433409ab3294c40e2ae1
parent 2a25799aeecfae4ccabc3b11673eced949576b34
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Thu, 25 Jun 2026 13:38:11 -0400
Fix "Drain all" hang: stop the auto-runner's drain-wait from spinning the event loop
A second, distinct Drain hang survived the earlier parked-unit fix
(64db913). When an auto-transcribe/-download unit was actively RUNNING
(worker leased, transcription in progress — not merely parked) at the
moment a soft drain fired, "Drain all" hung the whole server.
Root cause: the runner's internal waitNext() short-circuited to an
immediately-resolved promise whenever a signal was ALREADY aborted
(`pendingWake || signal.aborted || ctx.drainSignal.aborted`). A soft
drain aborts ctx.drainSignal for the rest of the run, so every
in-flight-wait thereafter returned with no delay. That turned the
runner's poll loops into a timer-less microtask spin that starves the
Node event loop. A running transcription that drain deliberately lets
finish completes via a child-process `exit` event — a macrotask — which
the spin never let the loop reach, so inFlight never hit zero, a CPU
core pegged, and the app appeared frozen.
The spin was reached first in the MAIN fill loop's at-capacity wait
(once draining drops the target to 0 while a unit is still in flight),
before the terminal `while (inFlight) await waitNext()` was ever hit —
so a fix confined to the terminal loop wouldn't have helped. The
earlier fix only covered PARKED units, which settle via microtasks and
so cleared even under the spin; a genuinely running unit depends on a
macrotask and didn't.
Fix: waitNext() now only fast-paths a real pending wake(); an
already-aborted signal falls through to a real timer. All three wait
sites (the two main-loop idle waits and the terminal drain-wait) now
pace on a timer instead of spinning, while still being woken promptly
by a finishing unit (wake()) or by an abort firing mid-wait (the abort
event listeners, which fire on the abort transition). The whisper-all
batch was never affected — it awaits Promise.all with no poll loop.
Adds an e2e regression staging a genuinely-running auto-transcribe unit
and draining mid-run; it asserts the runner finalizes and the in-flight
unit is allowed to complete (soft drain, not a kill). Verified to hang
(30s timeout) before this fix and to pass (~8s) after. All 11
auto-queue e2e tests pass; common + editor typecheck clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Diffstat:
3 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/common/controller/autoRunner.ts b/common/controller/autoRunner.ts
@@ -254,9 +254,21 @@ async function runLoop(
pendingWake = true;
}
};
+ // Wait up to maxMs, woken early by wake() (a finishing unit) or by a hard
+ // cancel / drain firing during the wait (the abort listeners below). An
+ // ALREADY-aborted signal must NOT short-circuit to an immediately-resolved
+ // promise: after a drain (or hard cancel) the signal stays aborted for the
+ // rest of the run, so the loops that wait for in-flight units to clear
+ // (`while (inFlight) await waitNext()`, and the at-capacity wait once
+ // target == 0 while draining) would become a timer-less microtask spin that
+ // starves the event loop — a still-running unit's child-process exit (a
+ // macrotask) would then never be delivered, inFlight would never reach zero,
+ // and a CPU core would peg forever (the "Drain all hangs the app" bug). So we
+ // only fast-path a real pending wake here; an already-aborted signal falls
+ // through to a real timer, and a finishing unit still wakes us promptly.
const waitNext = (maxMs: number): Promise<void> =>
new Promise<void>((resolve) => {
- if (pendingWake || signal.aborted || ctx.drainSignal.aborted) {
+ if (pendingWake) {
pendingWake = false;
resolve();
return;
diff --git a/editor/CHANGELOG.md b/editor/CHANGELOG.md
@@ -1,6 +1,7 @@
# Changelog
## [Unreleased]
+- **"Drain all" no longer hangs the server when an auto-transcribe/-download unit is actively running.** A second, distinct drain hang remained after the earlier parked-unit fix: the runner's internal `waitNext()` helper short-circuited to an *immediately-resolved* promise whenever a signal was *already* aborted — so once a soft drain fired (and `drainSignal` stays aborted for the rest of the run), every wait while in-flight units were still finishing returned with no delay. That turned the runner's poll loops into a timer-less **microtask spin** that starved the Node event loop (the spin was reached first in the main fill loop's at-capacity wait once the drain target dropped to 0, before the terminal drain-wait was ever hit). A transcription that drain deliberately lets finish completes via a child-process `exit` event — a *macrotask* — which the spin never let run, so the in-flight count never reached zero, a CPU core pegged, and the whole app appeared frozen. (The earlier fix only covered *parked* units, which settle via microtasks and so cleared even under the spin; a genuinely *running* unit depends on a macrotask and didn't.) `waitNext()` now only fast-paths a real pending `wake()`; an already-aborted signal falls through to a real timer, so all three wait sites pace instead of spinning while still being woken promptly by a finishing unit or by an abort firing mid-wait. The `whisper-all` batch was never affected (it `await Promise.all(...)` with no manual poll loop). See `common/controller/autoRunner.ts` (`waitNext`) and the new running-unit drain regression test in `editor/e2e/auto-queue.spec.ts`.
- **First-class video-persistence UI (phase 5, the final phase): a Saved Videos area, per-channel retention controls, and per-video persist/unpersist.** The video-persistence subsystem built up over phases 1–4 is now driveable end to end from the editor. A new top-level **Saved videos** page (`/saved-videos`, in the Pool nav) summarizes the whole saved-video store — total count and size, per-channel breakdown (count, size, how many carry a backup checksum), the default store location, and the last backup time — and hosts the **backup configuration** (destination, scheduled on/off, interval) plus **Back up now** / **Verify backup** buttons. Each channel's **Cleanup stage** gains a **Retention & persistence** section (shown whenever keep-latest is on or the channel has saved videos) with live counts and three buttons: **Check kept videos** (re-probe the window for deleted-from-source videos and pin them), **Persist kept now** (a new bulk catch-up pass that re-fetches the source container for any in-window video whose source isn't saved yet — `persistKeptAction` / `common/controller/persistKept.ts`), and **Back up saved videos**. The **channel settings form** adds a Retention & persistence section: **keep latest** (window size), **extraction mode** (yt-dlp vs app-side ffmpeg), and a per-channel **saved-video store dir** override. Each **video page** gains a **Source video** card showing persisted status (file, size, stored time, keep reason, sha256, location) with an **Unpersist** control that moves the container back into the data dir, or a **Persist source video** button (re-fetch + archive) when it isn't saved yet. New job-kind label for `persist-kept`; `persist-kept` is re-runnable from bookmarks. The saved-video actions moved from `editor/app/savedVideos/` to `editor/app/saved-videos/` to match the route. Covered by `editor/e2e/saved-videos.spec.ts` and `common/controller/persistKept.test.ts`. (Deferred: surfacing kept-check/persist as Actionable-page rows, and streaming the player directly from the store — unpersist brings the container back to the data dir to play it.)
- **Backups for the saved-video store: rsync mirror + per-backup manifest + drift verification (phase 4 of the video-persistence subsystem; backend + scheduler, UI lands later).** The (large, often irreplaceable) saved source videos can now be **backed up to a configured destination**. A backup walks every saved-video pointer across all channels (so per-channel store overrides are covered automatically) and **`rsync`-mirrors each container** into `<dest>/<slug>/<videoId>/` — incremental and resumable (`-a --partial`), additive (no deletes), so re-running only transfers changed or new files. It writes a **`backup-manifest.json`** at the destination root recording each container's canonical location, byte size, and a **streamed sha256**, and caches that hash back onto the live pointer. A **verify** step reads the manifest back and reports drift in four buckets — `missing`, `sizeMismatch`, `checksumMismatch` (re-hashing each present file), and `extra` (containers at the destination the manifest doesn't know about). New global settings block **`savedVideoBackup`** (`{ enabled, dest, intervalMinutes }`; a blank `dest` forces `enabled` off) plus a **`RSYNC_BIN`** env override. When enabled with a destination, the **sync scheduler** runs the backup automatically on its own cadence (a global, not per-channel, job — suppressed during quiet hours, tracked via `lastSavedVideoBackupAt`). Backups can also be run/verified manually via `backupSavedVideosAction` / `verifySavedVideoBackupAction` (managed jobs on a dedicated `saved-videos` queue). The destination is treated as a local filesystem path (a mounted backup disk). See the new `common/lib/savedVideoBackup.ts` (manifest types/parse), `common/controller/{backupSavedVideos,savedVideoInventory}.ts` (+ tests), `common/lib/paths.ts` (`rsyncBin`), `common/lib/settings.ts` (`savedVideoBackup`), `common/jobs/syncSchedulerState.ts`, `editor/app/savedVideos/backupActions.ts`, and `editor/app/scheduler/runTick.ts`.
- **Saved-video store: persisted source videos move to a separate dir/disk, with retention pruning (phase 3 of the video-persistence subsystem; backend, UI lands later).** When the per-download persistence rule (phase 2) keeps a source video, the downloaded container is now **moved out of the per-video data dir into a separate saved-video store** — leaving only a small `saved-video.json` pointer behind — so the main data volume holds just audio + transcripts while the (large) source videos can live on another disk. The store root defaults to `<transcripts>/saved-videos`, is overridable globally via the **`SAVED_VIDEOS_DIR`** env var, and can be further overridden **per channel** (`savedVideosDir` in `config.json`); a video's container lands under `<root>/<slug>/<videoId>/`. The move is **cross-device-safe** (rename within a disk, copy-to-temp + atomic rename + unlink across disks) and **best-effort** — a failed move leaves the container in the data dir as `source-media.<ext>` (still persisted, just not relocated) rather than failing the download. **Transcription resolves from the store**: when no extracted `audio.*` exists, the transcribe fallback follows the pointer to the stored container (returned as a path relative to the video dir so both the local engine and the remote uploader read it correctly), so a kept-but-cleaned or archive-only video still transcribes. **Retention pruning** (the phase-2 follow-up) now bounds the store: the Clean-audio sweep also evicts any *keep-latest* container that has rolled out of the window — but **never** a manually-archived (`override`) or pinned/irreplaceable (`pin`/do-not-clean) one, distinguished by a `keepReason` recorded on each pointer. Reversible helpers ship for the upcoming UI: `unpersistSavedVideo` (move the container back) and `dropSavedVideo` (delete it). A reusable `checkDiskSpaceFor(dir, …)` lands so disk gating can target the store filesystem (used by the UI/backup phases). Note: source-video persistence is still skipped for audio-check channels (deferred), and saved-store counts aren't yet surfaced in the channel snapshot (lands with the phase-5 UI). See the new `common/lib/savedVideo.ts` (+ `savedVideo-server.ts` + tests), `common/controller/pruneSavedVideos.ts` (+ tests), `common/lib/paths.ts` (`savedVideosDir`), `common/lib/channelConfig.ts` (`savedVideosDir`), `common/lib/diskSpace.ts`, `common/ytdlp/{persistencePlan,downloadOneManaged}.ts`, `common/controller/{transcribeOne,cleanAudioFromTranscribed}.ts`.
diff --git a/editor/e2e/auto-queue.spec.ts b/editor/e2e/auto-queue.spec.ts
@@ -599,6 +599,59 @@ test("Drain completes when an auto-transcribe unit is parked behind a busy worke
}
});
+// Regression: draining the runner while one of its units is actively RUNNING (a
+// worker leased and a transcription in progress, not merely parked) used to hang
+// the whole server. The runner's terminal `while (inFlight) await waitNext()`
+// loop short-circuited to an immediately-resolved promise once drainSignal was
+// aborted, turning it into a timer-less microtask spin that starved the event
+// loop — so the running unit's child-exit (a macrotask) never fired, inFlight
+// never emptied, and a CPU core spun forever. (The earlier parked-unit fix only
+// covered units that settle via microtasks.) Drain must let the running unit
+// finish and the runner must finalize.
+test("Drain completes (and lets the unit finish) when an auto-transcribe unit is running", async ({
+ page,
+ request,
+}) => {
+ await resetData(null);
+ // A single slowop video the auto-runner picks: the fake-whisper fixture runs
+ // ~7s of real wall-time for a "slowop" dir, a wide window to drain mid-run.
+ // The worker is free, so the runner leases it and the unit RUNS (not parks).
+ await makeChannel("alpha", ["slowop1"]);
+ await writeSettings(IDLE_SETTINGS(ALPHA_ROOT));
+
+ // 1) Start the runner; it picks slowop1, leases the free worker, and starts
+ // transcribing. Wait until the pick is recorded (unit launched/running) and
+ // the transcript isn't written yet (still mid-run), so the drain lands while
+ // the unit is genuinely in flight.
+ await startRunner(request);
+ await expect
+ .poll(
+ async () =>
+ (await getStatus(request)).transcription.picks.some(
+ (p) => p.videoId === "slowop1",
+ ),
+ { timeout: 30_000 },
+ )
+ .toBe(true);
+ expect(
+ await pathExists("test-transcripts/channels/alpha/data/slowop1/transcript.json"),
+ ).toBe(false);
+
+ // 2) Drain the runner. Pre-fix this hung forever (event-loop starvation);
+ // post-fix the loop paces on a real timer, the running unit finishes, and
+ // the runner finalizes well within the timeout.
+ await page.goto("/auto-queue");
+ const section = page.locator("section", {
+ has: page.getByRole("heading", { name: "Auto-transcribe" }),
+ });
+ await section.getByRole("button", { name: "Drain Auto-transcribe" }).click();
+ await expect.poll(() => runnerRunning(request), { timeout: 30_000 }).toBe(false);
+
+ // 3) Drain is a soft-cancel: the in-flight unit was allowed to COMPLETE, not
+ // killed — so its transcript now exists.
+ expect(await allTranscribed("alpha", ["slowop1"])).toBe(true);
+});
+
test("download: prioritizes channels across the per-platform queue", async ({
request,
}) => {
