mirror of
https://github.com/ollama/ollama.git
synced 2026-10-14 03:23:33 +00:00
transfer: don't retry blob directory and file creation errors
os.MkdirAll's error was discarded, and os.Create failures were returned as ordinary errors, so a missing or unusable destination was retried like a network failure. Return both as destError and stop on the first attempt. Co-authored-by: Ganesh <ganeshmaheshwaram2005@gmail.com>
This commit is contained in:
parent
b933ca379d
commit
bb189e08ea
2 changed files with 80 additions and 6 deletions
|
|
@ -26,8 +26,8 @@ var (
|
|||
errSlow = errors.New("download too slow")
|
||||
)
|
||||
|
||||
// destError is a write to the blob destination. It is not a transient
|
||||
// network failure, so the download is not retried.
|
||||
// destError is a local failure creating or writing the blob destination.
|
||||
// It is not a transient network failure, so the download is not retried.
|
||||
type destError struct{ err error }
|
||||
|
||||
func (e *destError) Error() string { return e.err.Error() }
|
||||
|
|
@ -219,8 +219,8 @@ func (d *downloader) download(ctx context.Context, blob Blob) error {
|
|||
case errors.Is(err, context.Canceled), errors.Is(err, context.DeadlineExceeded):
|
||||
return err
|
||||
case isDestWriteError(err):
|
||||
// Disk full and other local write failures will not succeed on retry,
|
||||
// and a later network error would hide them.
|
||||
// Creating the destination or writing the blob will not succeed on
|
||||
// retry, and a later network error would hide the local failure.
|
||||
return err
|
||||
case errors.Is(err, errStalled):
|
||||
if stallRetries++; stallRetries >= maxTransientRetries {
|
||||
|
|
@ -312,7 +312,9 @@ func (d *downloader) downloadOnce(ctx context.Context, blob Blob) (int64, error)
|
|||
func (d *downloader) save(ctx context.Context, cancel context.CancelCauseFunc, blob Blob, r io.Reader, existingSize int64) (int64, error) {
|
||||
dest := filepath.Join(d.destDir, digestToPath(blob.Digest))
|
||||
tmp := dest + ".tmp"
|
||||
os.MkdirAll(filepath.Dir(dest), 0o755)
|
||||
if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
|
||||
return 0, &destError{err: err}
|
||||
}
|
||||
|
||||
h := sha256.New()
|
||||
|
||||
|
|
@ -341,7 +343,7 @@ func (d *downloader) save(ctx context.Context, cancel context.CancelCauseFunc, b
|
|||
if existingSize == 0 {
|
||||
f, err = os.Create(tmp)
|
||||
if err != nil {
|
||||
return 0, err
|
||||
return 0, &destError{err: err}
|
||||
}
|
||||
setSparse(f)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -478,6 +478,78 @@ type errWriter struct{ err error }
|
|||
|
||||
func (w errWriter) Write([]byte) (int, error) { return 0, w.err }
|
||||
|
||||
// Directory and file creation failures are local. They must surface on the
|
||||
// first attempt instead of being ignored or retried as network errors.
|
||||
func TestDownloadLocalCreateFailureNotRetried(t *testing.T) {
|
||||
payload := []byte("blob-bytes")
|
||||
sum := sha256.Sum256(payload)
|
||||
digest := fmt.Sprintf("sha256:%x", sum)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
setup func(t *testing.T) string
|
||||
wantOp string
|
||||
}{
|
||||
{
|
||||
name: "parent path is a file",
|
||||
setup: func(t *testing.T) string {
|
||||
root := t.TempDir()
|
||||
parent := filepath.Join(root, "not-a-dir")
|
||||
if err := os.WriteFile(parent, []byte("x"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return filepath.Join(parent, "blobs")
|
||||
},
|
||||
wantOp: "mkdir",
|
||||
},
|
||||
{
|
||||
name: "blob temp path is a directory",
|
||||
setup: func(t *testing.T) string {
|
||||
dir := t.TempDir()
|
||||
// Non-empty so the retry cleanup cannot remove it and succeed later.
|
||||
tmp := filepath.Join(dir, digestToPath(digest)+".tmp")
|
||||
if err := os.Mkdir(tmp, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(tmp, "keep"), []byte("x"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return dir
|
||||
},
|
||||
wantOp: "open",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
var hits atomic.Int32
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
hits.Add(1)
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write(payload)
|
||||
}))
|
||||
t.Cleanup(server.Close)
|
||||
|
||||
err := Download(context.Background(), DownloadOptions{
|
||||
Blobs: []Blob{{Digest: digest, Size: int64(len(payload))}},
|
||||
BaseURL: server.URL,
|
||||
DestDir: tt.setup(t),
|
||||
})
|
||||
if err == nil || errors.Is(err, errMaxRetriesExceeded) {
|
||||
t.Fatalf("Download error = %v, want a local create error without retry", err)
|
||||
}
|
||||
var pe *os.PathError
|
||||
if !errors.As(err, &pe) || pe.Op != tt.wantOp {
|
||||
t.Fatalf("Download error = %v, want *os.PathError op %q", err, tt.wantOp)
|
||||
}
|
||||
// resolve + one body GET. Retrying would hit the server again.
|
||||
if got := hits.Load(); got != 2 {
|
||||
t.Fatalf("server hits = %d, want 2 (one attempt)", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpload(t *testing.T) {
|
||||
// Create test blobs
|
||||
clientDir := t.TempDir()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue