fix(captions): resume model downloads instead of restarting from zero - #2043
fix(captions): resume model downloads instead of restarting from zero#2043DPS0340 wants to merge 1 commit into
Conversation
A dropped connection anywhere in the Parakeet download failed the whole operation and deleted the staging directory, so a user on an unreliable link could never finish: every attempt restarted at 0 bytes and had to survive ~650 MB in one unbroken transfer. Each part now tracks how much it has written and retries with a Range header, up to 5 attempts, with exponential backoff. Only mid-transfer drops are retried -- an attempt that received nothing (404, DNS, refused) returns immediately rather than burning five attempts on a hard failure. Two cases the naive version gets wrong: - A server that ignores Range answers 200 with the whole body. Appending that onto the prefix already on disk silently corrupts the file, so the response status is checked and the part rewound when it is not 206. - One entry in PARAKEET_TDT_FULL_MODEL_FILES is split across two URLs appended to the same file, so the rewind target is this part's start offset (captured with stream_position) rather than 0. Rewinding to 0 would discard the previous part. Verified by extracting the loop into a standalone binary and running it against a local server that drops connections mid-body: drops 2x, honours Range -> 400000/400000 bytes, byte-identical drops then ignores Range -> rewinds, byte-identical, no duplicate prefix 404 / connection refused -> fails in 0.018s, no retries always drops -> gives up after 5 attempts 2-part file, part 2 drops -> 'restarting part from 200000', both parts intact Relates to CapSoftware#1791 (that reporter's 30s timeout is already fixed; this is the remaining failure mode on a flaky connection).
| Ok(()) => break, | ||
| Err(e) => { | ||
| // Only a mid-transfer drop is worth resuming. Give up | ||
| // immediately when nothing new arrived, otherwise a |
There was a problem hiding this comment.
Nit: the comment says "nothing new arrived", but the code only checks whether any bytes have ever been downloaded for this part. Might be worth tightening the wording so it’s clear we fail fast only when part_downloaded == 0.
| // immediately when nothing new arrived, otherwise a | |
| // Only retry once we have a partial file on disk. If we haven't downloaded | |
| // anything yet, fail fast so hard failures (404, DNS, refused) don't burn attempts. |
| let chunk = chunk_result | ||
| .map_err(|e| format!("Download error for {filename}: {e}"))?; | ||
| file.write_all(&chunk) | ||
| .await |
There was a problem hiding this comment.
This retry loop will also back off/retry on local IO errors (e.g. disk full, permission issues) since they’re surfaced the same way as network drops. Might be worth bailing immediately on those so we don’t sleep/retry when the failure is clearly non-transient.
|
Closing for queue size — cutting my nine open PRs to five, keeping the ones that close user-filed issues. This one is honest about its own scope in the description already: it relates to #1791 but is explicitly not a fix for that reporter's problem — their 30s timeout is already fixed on The problem is real and I verified it rather than reasoning about it — I extracted the loop into a standalone binary and drove it against a server that drops the connection mid-transfer, checking five scenarios down to byte-identity of the resulting file. The Parakeet download has no resume, so one dropped connection anywhere fails the whole transfer and deletes the staging directory, and the user starts a multi-hundred-MB download from zero. But +117/-31 of retry-and-resume logic in Test harness is kept, so if you want this later it is a reopen and not a rewrite. |
Relates to #1791. Not a fix for that reporter's problem — their 30s timeout is already fixed on
main; this is the failure mode that remains once it is.The problem
The Parakeet download has no resume. One dropped connection anywhere in the transfer fails the whole operation and deletes the staging directory:
The int8 encoder alone is 652 MB (
content-lengthfrom HuggingFace). On an unreliable link every attempt restarts at 0, so the user has to win a ~650 MB unbroken transfer or never finish at all.The fix
Each part tracks what it has written and retries with a
Rangeheader, 5 attempts, exponential backoff. Only mid-transfer drops are retried — an attempt that received nothing (404, DNS, refused) returns immediately rather than spending five attempts on a hard failure.Two cases the naive version gets wrong, both of which I hit while testing:
Rangeanswers200with the whole body. Appending that onto the prefix already on disk silently corrupts the file. The status is checked and the part rewound when it isn't206.PARAKEET_TDT_FULL_MODEL_FILESis split across two URLs appended to the same file. So the rewind target is this part's start offset (captured viastream_position), not0— rewinding to0would discard the previously completed part. I wrote it the wrong way first and caught it by checking the table.Confirmed HuggingFace supports this before writing anything:
accept-ranges: bytes, and a live range request returns206.Verification
cargo checkcan't run here — the build script needsbinaries/cap-muxer-*andtarget/native-deps/Spacedrive.framework, and it fails identically on unmodifiedmain(verified by stashing). So instead of reasoning about the loop I extracted it verbatim into a standalone binary and ran it against a local server that drops connections mid-body:Range400000/400000bytes, byte-identical to sourceRange404/ connection refusedrestarting part from 200000, both parts intactByte-equality was checked against the generated payload, not just the length — a resumed download that stitches at the wrong offset produces a correctly-sized, corrupt file, which is the failure worth catching.
The harness is a faithful copy of the loop rather than the real crate, so it proves the resume logic and not the surrounding integration. A build on CI would be worth having before merge.