Skip to content

fix(downloader): stall timeout, resume-safe cancel, and stale-partial reaping - #10406

Merged
mudler merged 3 commits into
masterfrom
fix/downloader-stall-resume
Jun 19, 2026
Merged

fix(downloader): stall timeout, resume-safe cancel, and stale-partial reaping#10406
mudler merged 3 commits into
masterfrom
fix/downloader-stall-resume

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

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 pending with no progress for 20+ min, and a history of installs dying at 2-4 GiB with context canceled / stale operation reaped, leaving ~90 GB of orphaned .partial files 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's IdleConnTimeout doesn't cover an in-flight body read. A silently-dropped TCP connection (no FIN/RST) blocks the body Read indefinitely, 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.Canceled the 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 .partial on cancel so the next attempt resumes via Range. (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 .partial files accumulated and could fill the models volume.
→ Add CleanupStalePartialFiles and reap partials older than 24h on startup.

Tests

TDD throughout. New specs in pkg/downloader (all behavioral, no network):

  • stall mid-stream aborts instead of hanging; slow-but-steady is not aborted; .partial survives a stall
  • .partial survives context cancellation and a second attempt resumes to completion with a valid SHA
  • CleanupStalePartialFiles removes stale partials recursively while keeping fresh partials and completed files

go test -race ./pkg/downloader/... clean; core/gallery + core/services/galleryop suites green; lint clean.

Assisted-by: Claude:claude-opus-4-8 [Claude Code]

… 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]
Comment thread pkg/downloader/partial.go Fixed
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]
@localai-bot

Copy link
Copy Markdown
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:

  • The gallery op's context is cancelled with downloader.ErrUserCancelled (context.WithCancelCause) on a deliberate user cancel → the downloader discards the .partial.
  • An incidental cancellation (process shutdown / pod restart) carries no such cause → the .partial is kept for resume.

One subtlety handled along the way: an HTTP request cancelled with a cause surfaces the cause error (not context.Canceled) from the client, so cancellation is now detected via the context (ctx.Err() / context.Cause(ctx)) rather than by matching the returned error. New downloader spec covers the deliberate-cancel-discards-partial path; the keep-on-restart and resume specs still pass.

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]
@mudler
mudler merged commit 2e734bf into master Jun 19, 2026
59 checks passed
@mudler
mudler deleted the fix/downloader-stall-resume branch June 19, 2026 19:35
@localai-bot localai-bot added the bug Something isn't working label Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants