Files
LocalAI/pkg/modelartifacts/partial_test.go
mudler's LocalAI [bot] f01038f479 fix(modelartifacts): stage each writer's artifact in its own partial tree (#10995)
Every writer used to stage into the same `.artifacts/.partial/<cacheKey>`.
That was safe only because the artifact lock held: two writers that both
believed they had it opened the same blob with O_APPEND and interleaved
their bytes into one file, while the resume probe read the other writer's
in-flight size. SHA verification caught the damage only after both had
burned the entire download.

#10986 restored the lock's precondition on CIFS but left the dependency in
place. Suffix the staging tree with a writer identity drawn once per
process run, so concurrent writers cannot corrupt each other whatever the
lock does. The lock stops being a correctness dependency and becomes a
pure efficiency optimisation: a lock failure now costs a duplicated
download, not a corrupted one.

Commit stays an atomic rename. The loser of a commit race reconciles onto
the winner's tree instead of surfacing a bare ENOTEMPTY for work that
actually succeeded, since the artifact is content-addressed and both trees
hold the same verified bytes.

Writer-unique staging means a crashed writer's tree is no longer
overwritten by its successor, so two things are added to keep it from
becoming a disk leak and a resume regression:

- A sweep reclaims trees whose contents have been untouched for 24h,
  matching the window the startup reaper already uses for stray *.partial
  files. It reads the newest mtime anywhere inside the tree, because
  writing a blob never touches an ancestor, and refuses any name this
  package did not write. A live download writes continuously, and the
  downloader's stall watchdog aborts a silent one long before it could
  look abandoned.

- Adoption lets a restarted process claim a dead predecessor's tree for
  the same artifact and resume from its bytes, which a tens-of-gigabytes
  repo depends on. The claim is an atomic rename, so racing adopters
  cannot both win. It runs only under the artifact lock - which is
  released exactly when the owning process dies - and only on a tree idle
  for 5 minutes as a second line of defence for when the lock does not
  exclude.


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

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
2026-07-20 23:07:43 +02:00

233 lines
9.6 KiB
Go

package modelartifacts_test
import (
"context"
"crypto/sha256"
"encoding/hex"
"fmt"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"strings"
"sync/atomic"
"time"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
hfapi "github.com/mudler/LocalAI/pkg/huggingface-api"
"github.com/mudler/LocalAI/pkg/modelartifacts"
)
const (
testCacheKey = "b79e23e0b9c50af094d627582df30109eff8637438864172d64be07dfc5a98f9"
testWriterID = "00112233445566aa"
otherWriterID = "ffeeddccbbaa9988"
)
// backdate rewinds every mtime in tree so it looks abandoned. The sweep and the
// adoption scan both read the newest mtime anywhere inside the tree, not the
// directory's own, so every entry has to move.
func backdate(tree string, age time.Duration) {
when := time.Now().Add(-age)
Expect(filepath.Walk(tree, func(path string, _ os.FileInfo, err error) error {
if err != nil {
return err
}
return os.Chtimes(path, when, when)
})).To(Succeed())
}
func partialRoot(modelsPath string) string {
return filepath.Join(modelsPath, ".artifacts", ".partial")
}
// stageOrphan writes a partial tree as if a writer had died holding it, with
// blob carrying however many bytes of file it had managed to fetch.
func stageOrphan(modelsPath, writerID, hubPath string, blob []byte) string {
tree := filepath.Join(partialRoot(modelsPath), testCacheKey+"."+writerID)
nameSum := sha256.Sum256([]byte(hubPath))
downloads := filepath.Join(tree, ".downloads")
Expect(os.MkdirAll(downloads, 0o750)).To(Succeed())
Expect(os.MkdirAll(filepath.Join(tree, "snapshot"), 0o750)).To(Succeed())
Expect(os.WriteFile(filepath.Join(downloads, hex.EncodeToString(nameSum[:])+".partial"), blob, 0o600)).To(Succeed())
return tree
}
var _ = Describe("writer-unique artifact partial trees", func() {
It("gives each writer its own staging path and rejects a malformed identity", func() {
spec := modelartifacts.Spec{
Source: modelartifacts.Source{Type: "huggingface", Repo: "owner/repo"},
Resolved: &modelartifacts.Resolved{Endpoint: "https://huggingface.co", Revision: "0123456789abcdef0123456789abcdef01234567", CacheKey: testCacheKey},
}
layout, err := modelartifacts.LayoutFor("/models", spec)
Expect(err).NotTo(HaveOccurred())
Expect(layout.Partial).To(BeEmpty(), "a layout without a writer identity must not name a staging tree")
mine, err := layout.WithWriter(testWriterID)
Expect(err).NotTo(HaveOccurred())
theirs, err := layout.WithWriter(otherWriterID)
Expect(err).NotTo(HaveOccurred())
Expect(mine.Partial).To(Equal(filepath.Join("/models", ".artifacts", ".partial", testCacheKey+"."+testWriterID)))
Expect(mine.Partial).NotTo(Equal(theirs.Partial))
// A writer identity reaches the filesystem as a path component, so
// anything that is not the exact drawn shape is refused rather than
// sanitised.
for _, bad := range []string{"", "../escape", "not-hex", strings.Repeat("a", 17)} {
_, err := layout.WithWriter(bad)
Expect(err).To(MatchError(ContainSubstring("invalid artifact writer id")), "accepted writer id %q", bad)
}
})
Describe("reclaiming abandoned trees", func() {
var modelsPath string
BeforeEach(func() {
modelsPath = GinkgoT().TempDir()
Expect(os.MkdirAll(partialRoot(modelsPath), 0o750)).To(Succeed())
})
It("removes a tree nothing has touched for the whole window", func() {
tree := stageOrphan(modelsPath, otherWriterID, "model.safetensors", []byte("half"))
backdate(tree, 48*time.Hour)
removed, err := modelartifacts.SweepStalePartialTrees(modelsPath, modelartifacts.PartialOrphanTTL, "")
Expect(err).NotTo(HaveOccurred())
Expect(removed).To(Equal(1))
Expect(tree).NotTo(BeADirectory())
})
// This is the property that keeps the fix from becoming a data-loss
// bug: a running download writes continuously, so the freshness of the
// bytes inside the tree - not the directory's own mtime, which nothing
// updates - is what proves the writer is still alive.
It("never removes a tree whose contents are still being written", func() {
tree := stageOrphan(modelsPath, otherWriterID, "model.safetensors", []byte("half"))
// An old directory with a fresh blob inside is exactly what a
// long-running download looks like.
backdate(tree, 48*time.Hour)
now := time.Now()
blob := filepath.Join(tree, ".downloads",
hex.EncodeToString(func() []byte { sum := sha256.Sum256([]byte("model.safetensors")); return sum[:] }()))
Expect(os.Chtimes(blob+".partial", now, now)).To(Succeed())
removed, err := modelartifacts.SweepStalePartialTrees(modelsPath, modelartifacts.PartialOrphanTTL, "")
Expect(err).NotTo(HaveOccurred())
Expect(removed).To(BeZero())
Expect(tree).To(BeADirectory())
})
It("never removes the caller's own tree, however old it looks", func() {
tree := stageOrphan(modelsPath, testWriterID, "model.safetensors", []byte("half"))
backdate(tree, 48*time.Hour)
removed, err := modelartifacts.SweepStalePartialTrees(modelsPath, modelartifacts.PartialOrphanTTL, tree)
Expect(err).NotTo(HaveOccurred())
Expect(removed).To(BeZero())
Expect(tree).To(BeADirectory())
})
It("never removes a directory it did not create", func() {
stranger := filepath.Join(partialRoot(modelsPath), "not-an-artifact")
Expect(os.MkdirAll(stranger, 0o750)).To(Succeed())
backdate(stranger, 48*time.Hour)
removed, err := modelartifacts.SweepStalePartialTrees(modelsPath, modelartifacts.PartialOrphanTTL, "")
Expect(err).NotTo(HaveOccurred())
Expect(removed).To(BeZero())
Expect(stranger).To(BeADirectory())
})
It("treats a missing partial root as nothing to do", func() {
removed, err := modelartifacts.SweepStalePartialTrees(GinkgoT().TempDir(), modelartifacts.PartialOrphanTTL, "")
Expect(err).NotTo(HaveOccurred())
Expect(removed).To(BeZero())
})
})
// Writer-unique staging would otherwise cost the resume a restarted replica
// depends on to ever finish a tens-of-gigabytes repo: its predecessor's
// bytes sit under a writer id it will never generate again.
It("adopts an abandoned tree for the same artifact and resumes from its bytes", func() {
payload := []byte(strings.Repeat("artifact-bytes-", 512))
sum := sha256.Sum256(payload)
already := len(payload) / 2
var servedFrom atomic.Int64
servedFrom.Store(-1)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Accept-Ranges", "bytes")
if r.Method == http.MethodHead {
w.WriteHeader(http.StatusOK)
return
}
start := int64(0)
if _, err := fmt.Sscanf(r.Header.Get("Range"), "bytes=%d-", &start); err != nil {
start = 0
}
servedFrom.Store(start)
w.Header().Set("Content-Range", fmt.Sprintf("bytes %d-%d/%d", start, len(payload)-1, len(payload)))
w.WriteHeader(http.StatusPartialContent)
_, _ = w.Write(payload[start:])
}))
DeferCleanup(server.Close)
modelsPath := GinkgoT().TempDir()
spec := modelartifacts.Spec{Source: modelartifacts.Source{Type: "huggingface", Repo: "owner/repo"}}
resolver := &fakeSnapshotResolver{snapshot: hfapi.Snapshot{
Endpoint: "https://huggingface.co", Repo: "owner/repo",
RequestedRevision: "main", ResolvedRevision: "0123456789abcdef0123456789abcdef01234567",
Files: []hfapi.SnapshotFile{{
Path: "model.safetensors", Size: int64(len(payload)),
LFSOID: hex.EncodeToString(sum[:]), URL: server.URL + "/model",
}},
}}
key, err := modelartifacts.CacheKey(modelartifacts.Spec{
Source: spec.Source,
Resolved: &modelartifacts.Resolved{Endpoint: "https://huggingface.co", Revision: "0123456789abcdef0123456789abcdef01234567"},
})
Expect(err).NotTo(HaveOccurred())
Expect(key).To(Equal(testCacheKey), "the staged orphan must belong to the artifact under test")
orphan := stageOrphan(modelsPath, otherWriterID, "model.safetensors", payload[:already])
// Only a tree idle for the adoption window is claimable: the artifact
// lock already proves a peer that respects it is not writing, and this
// is the second line of defence for when the lock does not exclude.
backdate(orphan, time.Hour)
result, err := modelartifacts.NewManager(resolver).Ensure(context.Background(), modelsPath, spec)
Expect(err).NotTo(HaveOccurred())
Expect(servedFrom.Load()).To(Equal(int64(already)), "the adopted partial's bytes were re-downloaded instead of resumed")
Expect(os.ReadFile(filepath.Join(modelsPath, filepath.FromSlash(result.RelativePath), "model.safetensors"))).To(Equal(payload))
Expect(orphan).NotTo(BeADirectory(), "adoption must move the tree, not copy it")
})
It("leaves a freshly written tree for its owner instead of adopting it", func() {
payload := []byte("weight-bytes")
sum := sha256.Sum256(payload)
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write(payload)
}))
DeferCleanup(server.Close)
modelsPath := GinkgoT().TempDir()
// No backdating: this stands for a peer that is mid-download right now.
orphan := stageOrphan(modelsPath, otherWriterID, "model.safetensors", []byte("partial"))
resolver := &fakeSnapshotResolver{snapshot: hfapi.Snapshot{
Endpoint: "https://huggingface.co", Repo: "owner/repo",
RequestedRevision: "main", ResolvedRevision: "0123456789abcdef0123456789abcdef01234567",
Files: []hfapi.SnapshotFile{{
Path: "model.safetensors", Size: int64(len(payload)),
LFSOID: hex.EncodeToString(sum[:]), URL: server.URL + "/model",
}},
}}
_, err := modelartifacts.NewManager(resolver).Ensure(context.Background(), modelsPath,
modelartifacts.Spec{Source: modelartifacts.Source{Type: "huggingface", Repo: "owner/repo"}})
Expect(err).NotTo(HaveOccurred())
Expect(orphan).To(BeADirectory(), "a live peer's staging tree must survive another writer's materialization")
})
})