mirror of
https://github.com/mudler/LocalAI.git
synced 2026-07-30 01:48:06 -04:00
fix(downloader): hash the partial file before issuing the resume request (#11099)
The stall watchdog arms as soon as the response body exists, but the downloader then re-hashed the entire existing .partial before reading a single byte from the network. On slow models storage (a CIFS share reading at ~117MB/s) hashing a multi-GB partial outlasts the 60s stall window, so the watchdog aborted every healthy resume with 'download stalled: no data received for 1m0s'. The partial never grew, so every retry re-paid the same hash and failed identically, wedging the install permanently (any partial over ~7GB on such storage). Open the partial and hash it before the HTTP request instead: the watchdog now only measures actual network idle time, and the origin no longer sits on an idle connection while the hash runs. Assisted-by: Claude:claude-fable-5 [Claude Code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
committed by
GitHub
parent
977f663cb0
commit
90d93c71cd
@@ -2,6 +2,7 @@ package downloader_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
@@ -99,6 +100,54 @@ var _ = Describe("Download stall timeout", func() {
|
||||
Expect(info.Size()).To(BeNumerically(">", 0))
|
||||
})
|
||||
|
||||
It("does not count partial-file hashing time against the stall window when resuming", func() {
|
||||
// A resumed download re-hashes the existing .partial before the copy
|
||||
// loop starts. On slow storage a multi-GB partial takes longer to hash
|
||||
// than the stall window, and since no network read happens while
|
||||
// hashing, a watchdog armed before the hash aborts a healthy resume.
|
||||
// Every retry then re-pays the same hash and fails identically, so the
|
||||
// install wedges permanently (observed with a 7.9GB partial on a CIFS
|
||||
// models share). The sparse file keeps disk usage at zero while still
|
||||
// forcing the hash to churn through every byte.
|
||||
const partialSize = int64(2) << 30
|
||||
tail := []byte("remaining-bytes")
|
||||
total := partialSize + int64(len(tail))
|
||||
|
||||
partial, err := os.OpenFile(filePath+".partial", os.O_CREATE|os.O_WRONLY, 0600)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(partial.Truncate(partialSize)).To(Succeed())
|
||||
Expect(partial.Close()).To(Succeed())
|
||||
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == "HEAD" {
|
||||
w.Header().Set("Accept-Ranges", "bytes")
|
||||
w.WriteHeader(http.StatusOK)
|
||||
return
|
||||
}
|
||||
if r.Header.Get("Range") == "" {
|
||||
// A resume must ask for a range; anything else would re-download
|
||||
// (and here, concatenate) the whole body.
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", partialSize, total-1, total))
|
||||
w.WriteHeader(http.StatusPartialContent)
|
||||
_, _ = w.Write(tail)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
DownloadStallTimeout = 150 * time.Millisecond
|
||||
|
||||
err = URI(server.URL).DownloadFileWithContext(
|
||||
context.Background(), filePath, "", 1, 1,
|
||||
func(s1, s2, s3 string, f float64) {})
|
||||
Expect(err).ToNot(HaveOccurred(), "a promptly-answered resume must not be aborted by hash time")
|
||||
|
||||
info, statErr := os.Stat(filePath)
|
||||
Expect(statErr).ToNot(HaveOccurred())
|
||||
Expect(info.Size()).To(Equal(total))
|
||||
})
|
||||
|
||||
It("does not abort a slow-but-steady download", func() {
|
||||
// One byte every 100ms keeps the idle clock from ever expiring even
|
||||
// though the total transfer outlasts the stall timeout.
|
||||
|
||||
@@ -674,6 +674,32 @@ func (uri URI) DownloadFileWithContext(ctx context.Context, filePath, sha string
|
||||
return fmt.Errorf("failed to check partial download file %q: %w", tmpFilePath, statErr)
|
||||
}
|
||||
|
||||
// Create parent directory
|
||||
err = os.MkdirAll(filepath.Dir(filePath), 0750)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create parent directory for file %q: %v", filePath, err)
|
||||
}
|
||||
|
||||
// Open the partial and hash its existing bytes BEFORE issuing the request.
|
||||
// The stall watchdog arms the moment the response body exists, and nothing
|
||||
// reads that body while the partial is hashed; on slow storage a multi-GB
|
||||
// partial takes longer to hash than the stall window, so hashing after the
|
||||
// request aborts every resume, and each retry re-pays the same hash and
|
||||
// fails identically, wedging the install permanently. Hashing first also
|
||||
// keeps the origin from idling out the connection during the hash.
|
||||
outFile, err := os.OpenFile(tmpFilePath, os.O_APPEND|os.O_RDWR|os.O_CREATE, 0600)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create / open file %q: %v", tmpFilePath, err)
|
||||
}
|
||||
defer func() { _ = outFile.Close() }()
|
||||
if err := outFile.Chmod(0600); err != nil {
|
||||
return fmt.Errorf("failed to restrict partial file %q permissions: %v", tmpFilePath, err)
|
||||
}
|
||||
hash, err := calculateHashForPartialFile(outFile)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to calculate hash for partial file")
|
||||
}
|
||||
|
||||
var source io.ReadCloser
|
||||
var contentLength int64
|
||||
if _, e := os.Stat(uri.ResolveURL()); strings.HasPrefix(string(uri), LocalPrefix) || e == nil {
|
||||
@@ -743,25 +769,6 @@ func (uri URI) DownloadFileWithContext(ctx context.Context, filePath, sha string
|
||||
}
|
||||
defer source.Close()
|
||||
|
||||
// Create parent directory
|
||||
err = os.MkdirAll(filepath.Dir(filePath), 0750)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create parent directory for file %q: %v", filePath, err)
|
||||
}
|
||||
|
||||
// Create and write file
|
||||
outFile, err := os.OpenFile(tmpFilePath, os.O_APPEND|os.O_RDWR|os.O_CREATE, 0600)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create / open file %q: %v", tmpFilePath, err)
|
||||
}
|
||||
defer outFile.Close()
|
||||
if err := outFile.Chmod(0600); err != nil {
|
||||
return fmt.Errorf("failed to restrict partial file %q permissions: %v", tmpFilePath, err)
|
||||
}
|
||||
hash, err := calculateHashForPartialFile(outFile)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to calculate hash for partial file")
|
||||
}
|
||||
progress := &progressWriter{
|
||||
fileName: tmpFilePath,
|
||||
total: contentLength,
|
||||
|
||||
Reference in New Issue
Block a user