Repository navigation
fix(studio-server): a copy another process holds without progress ends in a named error - #6918
Conversation
…s in a named error
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head 1ffd9dd0
- important: the
Commentsworkflow fails on this head:proxyLock.tscomment share goes from 1.4% to 3.7% (3 comment lines added). It isn't a required check, but it's red because of this PR. Cut the new doc lines or move the explanation into names.
Scope
Read the full diff and the transcoder's owner write paths (transcodeToCache, runFfmpeg, encodeInChunks, concatArgs). The fails without the fix: line is present. I reproduced the blocker against proxyLock.ts at this head with a standalone Node probe: a live lock, an old lock mtime, and a .tmp-<uuid>-.part-<stem>-0.mp4 appended every 100 ms with stallMs = 1000 threw CopyLockStalledError instead of waiting. No prior reviews on this PR. Required checks were still running when I reviewed.
Edit accuracy: accurate 271 (base branch 271), smooth 103 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (2)
|
|
Fixed at 8dfa121. Blocker: the in-progress piece of a long source is written as .tmp--.part--.mp4, which my activity probe skipped; it now counts (a .tmp- file containing -.part--), so an owner still encoding a slow piece is waited for. New test 'keeps waiting while the owner writes a piece of a long source' fails with only that clause removed. Important: the doc line above lastActivityMs and the try/catch comment are gone (statSync throwIfNoEntry instead) and acquireCopyLock's doc stays one line, so the PR adds no comment lines to proxyLock.ts. 78 tests green x3, fmt/tsc clean. |
There was a problem hiding this comment.
Verdict: APPROVE — 2 resolved, 0 open, 0 new · 1ffd9dd0 → 8dfa1217. Pieces still being written count as activity ("keeps waiting while the owner writes a piece of a long source" fails at 1ffd9dd0 and passes here; proxyLock + proxyTranscoder 78/78 locally), and Comments is green. One note, not blocking: throwIfNoEntry: false ignores only ENOENT, whereas the old catch ignored every error. On Windows, a stat during delete-pending returns EPERM, which would now fail that one copy with a raw error; the next request retries it.
What
A browser-playable video copy that another process holds the lock for, and that has changed nothing for the transcode stall window (3 min,
HYPERFRAMES_PROXY_STALL_MS), now ends withProxyTranscodeError: another process holds the copy of <cache file> and has made no progress for 180s.checkshows it asCould not make a browser-playable copy of <video file>: ...(a warning that names the file) instead of waiting for the lock forever.Why
checkstopped at the "making video copies" phase with no result, and asnapshotbehind it waited for the check to finish. That phase runs beforecheck's idle window by design, and the ffmpeg run inside it already ends after 3 minutes without progress. The one wait with no bound is the copy lock:acquireCopyLockpolls while the lock's owner process is alive and never gives up. A lock left by a hung process, or one whose recorded pid has been reused by an unrelated live process (likely on a long-running Windows machine), makes every later copy of that file wait forever.I could not reproduce the reporter's machine (4-core Windows); the unbounded wait is the one I found by reading the copy path, and the new test reproduces it directly: a lock held by a live process with nothing changing for an hour.
How
acquireCopyLocktakes the stall window. While the owner is alive it measures the newest modification among the lock file, the owner's.tmp-*-<copy>file and its.part-<stem>-*pieces; if that is older than the window it throwsCopyLockStalledError, which the transcoder turns into the existingProxyTranscodeError. An owner that is still writing keeps being waited for.Not done:
check's own deadline still does not cover the copies phase (it ends only through this stall window and the existing ffmpeg one); I did not change the Desktop queue.Test plan
fails without the fix:
proxyLock.test.ts"gives up on a live owner that has made no progress for the stall window, naming the copy" (times out at main).proxyLock.test.tsandproxyTranscoder.test.ts: 77 passed, 3 runs in a row (devbox); the new test fails with the lock and transcoder at main; a second test pins that an owner still writing is waited for.Size
About 85 changed lines: one lock, its caller and a test file.