fix(downloader): stall timeout, resume-safe cancel, and stale-partial reaping - #10406
Merged
Conversation
… reaping Large model installs would hang forever or never finish. Three defects in the HTTP download path, all hit by big GGUF pulls over a slow or flaky link: 1. No stall timeout. The shared download client sets no body deadline (correct for streaming) but also no read-idle timeout, and the transport's IdleConnTimeout does not cover an in-flight body read. A silently-dropped TCP connection (no FIN/RST) blocked the body Read forever, freezing an install at N bytes until an external reaper killed it. Add an idle-timeout reader that closes the body after a window of zero progress (DownloadStallTimeout, default 60s), turning an indefinite hang into a fast, retryable error. A read that returns data resets the clock, so a slow-but-steady transfer is unaffected. 2. Cancellation deleted the partial. On context.Canceled the code removed the .partial file, so any frontend restart (deploy, OOM) mid-download wiped all progress and the retry restarted from zero. At slow egress, files larger than the restart interval never completed. Keep the .partial on cancel so the next attempt resumes via Range. 3. Partials leaked. Cleanup only ran on the context-cancel path, never on a stall or a SIGKILL/OOM, so abandoned .partial files accumulated and could fill the models volume. Add CleanupStalePartialFiles and reap partials older than 24h on startup. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Review follow-up. The previous commit kept the .partial on every cancellation so restarts could resume, but that also left a dangling partial when a user *intentionally* cancelled an install — the file lingered until the 24h reaper. Distinguish the two: cancel the gallery operation's context with a cause (downloader.ErrUserCancelled) so the download layer can tell a deliberate abort (discard the partial) from an incidental one such as a shutdown/restart (keep it for resume). Detect cancellation via the context rather than the returned error, because an HTTP request cancelled with a cause surfaces the cause error, not context.Canceled. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
Collaborator
Author
|
Follow-up (commit 0a3add2): the original "keep the .partial on cancel" was too blunt — a deliberate user cancel would also leave a dangling partial until the 24h reaper swept it. Now the two cases are distinguished via the cancellation cause:
One subtlety handled along the way: an HTTP request cancelled with a cause surfaces the cause error (not |
CI's code-scanning (gosec) flagged G122 (symlink TOCTOU) for the os.Remove call inside the filepath.WalkDir callback. Collect the stale paths during the walk and delete them afterwards instead of mutating the tree from inside the callback. Behavior is unchanged; the existing specs still pass. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-8 [Claude Code]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Large model installs (big GGUFs) on a slow/distributed deployment would hang forever or never finish. Installation is a frontend-side download (no worker needed), and three defects in the HTTP download path compound on a slow or flaky link:
Observed in a live distributed instance: ops stuck
pendingwith no progress for 20+ min, and a history of installs dying at 2-4 GiB withcontext canceled/stale operation reaped, leaving ~90 GB of orphaned.partialfiles on the models volume.Root cause + fixes
1. No stall timeout → frozen forever. The shared download client (
pkg/httpclient) intentionally sets no body deadline (correct for SSE streaming), but also has no read-idle timeout, and the transport'sIdleConnTimeoutdoesn't cover an in-flight body read. A silently-dropped TCP connection (no FIN/RST) blocks the bodyReadindefinitely, so the install freezes at N bytes until an external reaper kills it.→ Add an idle-timeout reader that closes the body after a window of zero progress (
DownloadStallTimeout, default 60s), turning an indefinite hang into a fast, retryable error. A read that returns data resets the clock, so a slow-but-steady transfer is unaffected.2. Cancellation deleted the partial → no resume across restarts. On
context.Canceledthe downloader removed the.partial, so any frontend restart (deploy, OOM) mid-download wiped all progress and the retry restarted from zero. At slow egress, files larger than the restart interval never completed.→ Keep the
.partialon cancel so the next attempt resumes viaRange. (The downloader already supports resume; this just stops defeating it.)3. Partials leaked → disk fills. Cleanup only ran on the context-cancel path, never on a stall or a SIGKILL/OOM, so abandoned
.partialfiles accumulated and could fill the models volume.→ Add
CleanupStalePartialFilesand reap partials older than 24h on startup.Tests
TDD throughout. New specs in
pkg/downloader(all behavioral, no network):.partialsurvives a stall.partialsurvives context cancellation and a second attempt resumes to completion with a valid SHACleanupStalePartialFilesremoves stale partials recursively while keeping fresh partials and completed filesgo test -race ./pkg/downloader/...clean;core/gallery+core/services/galleryopsuites green; lint clean.Assisted-by: Claude:claude-opus-4-8 [Claude Code]