mirror of
https://github.com/mudler/LocalAI.git
synced 2026-07-30 01:48:06 -04:00
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>
131 lines
5.0 KiB
Go
131 lines
5.0 KiB
Go
package modelartifacts_test
|
|
|
|
import (
|
|
"context"
|
|
"crypto/sha256"
|
|
"encoding/hex"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"sync"
|
|
"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"
|
|
)
|
|
|
|
// bypassedLocker grants the artifact lock to everyone who asks. It reproduces
|
|
// the state a real cluster was in during #10981: flock(2) on CIFS returned
|
|
// EACCES, both controller replicas concluded the lock was unusable, and both
|
|
// proceeded to materialize the same artifact at the same time. The lock is
|
|
// meant to prevent that, but a correctness property must not depend on a
|
|
// primitive that a network filesystem can silently take away.
|
|
type bypassedLocker struct{}
|
|
|
|
func (bypassedLocker) TryLock() (bool, error) { return true, nil }
|
|
func (bypassedLocker) Unlock() error { return nil }
|
|
|
|
// rendezvousServer serves payload to every GET, but holds each response until
|
|
// concurrent GETs have arrived, then dribbles the body out in two chunks. That
|
|
// makes the two writers provably overlap inside the download rather than
|
|
// relying on scheduling luck, so the spec is deterministic rather than flaky.
|
|
func rendezvousServer(payload []byte, concurrent int32) *httptest.Server {
|
|
var arrived atomic.Int32
|
|
gate := make(chan struct{})
|
|
var once sync.Once
|
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
if r.Method != http.MethodGet {
|
|
// The resume probe is a HEAD. Answering without Accept-Ranges keeps
|
|
// this server non-resumable, which is the harsher case: a writer
|
|
// that finds a foreign partial discards it outright.
|
|
w.WriteHeader(http.StatusOK)
|
|
return
|
|
}
|
|
if arrived.Add(1) >= concurrent {
|
|
once.Do(func() { close(gate) })
|
|
}
|
|
select {
|
|
case <-gate:
|
|
case <-time.After(10 * time.Second):
|
|
}
|
|
half := len(payload) / 2
|
|
_, _ = w.Write(payload[:half])
|
|
w.(http.Flusher).Flush()
|
|
time.Sleep(50 * time.Millisecond)
|
|
_, _ = w.Write(payload[half:])
|
|
}))
|
|
DeferCleanup(server.Close)
|
|
return server
|
|
}
|
|
|
|
var _ = Describe("concurrent materialization without a working lock", func() {
|
|
// The partial tree is shared mutable state. Before writer-unique partial
|
|
// paths it was safe only because the lock held: two writers opened the same
|
|
// blob with O_APPEND and interleaved into one file, and the resume probe
|
|
// read the other writer's in-flight size. SHA verification caught the
|
|
// damage only after both had burned the whole download.
|
|
It("lets two writers materialize the same artifact concurrently without corrupting each other", func() {
|
|
payload := make([]byte, 128*1024)
|
|
for i := range payload {
|
|
payload[i] = byte(i % 251)
|
|
}
|
|
sum := sha256.Sum256(payload)
|
|
server := rendezvousServer(payload, 2)
|
|
|
|
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",
|
|
}},
|
|
}
|
|
modelsPath := GinkgoT().TempDir()
|
|
spec := modelartifacts.Spec{Source: modelartifacts.Source{Type: "huggingface", Repo: "owner/repo"}}
|
|
|
|
type outcome struct {
|
|
result modelartifacts.Result
|
|
err error
|
|
}
|
|
outcomes := make([]outcome, 2)
|
|
var group sync.WaitGroup
|
|
for writer := range outcomes {
|
|
group.Add(1)
|
|
go func() {
|
|
defer GinkgoRecover()
|
|
defer group.Done()
|
|
// Separate managers stand in for separate processes: each one
|
|
// draws its own writer identity at construction.
|
|
manager := modelartifacts.NewManager(&fakeSnapshotResolver{snapshot: snapshot},
|
|
modelartifacts.WithLocker(func(string) modelartifacts.Locker { return bypassedLocker{} }))
|
|
result, err := manager.Ensure(context.Background(), modelsPath, spec)
|
|
outcomes[writer] = outcome{result: result, err: err}
|
|
}()
|
|
}
|
|
group.Wait()
|
|
|
|
for writer, got := range outcomes {
|
|
Expect(got.err).NotTo(HaveOccurred(),
|
|
"writer %d must not be poisoned by its peer; two writers sharing a partial path is exactly the corruption this guards against", writer)
|
|
Expect(os.ReadFile(filepath.Join(modelsPath, filepath.FromSlash(got.result.RelativePath), "model.safetensors"))).
|
|
To(Equal(payload), "writer %d resolved to a snapshot whose bytes are not the artifact", writer)
|
|
}
|
|
|
|
// Exactly one snapshot is committed: the loser of the commit race
|
|
// reconciles onto the winner's tree rather than publishing a rival.
|
|
committed, err := filepath.Glob(filepath.Join(modelsPath, ".artifacts", "huggingface", "*"))
|
|
Expect(err).NotTo(HaveOccurred())
|
|
Expect(committed).To(HaveLen(1))
|
|
|
|
// Both writers finished, so neither may leave a partial tree behind.
|
|
leftovers, err := filepath.Glob(filepath.Join(modelsPath, ".artifacts", ".partial", "*"))
|
|
Expect(err).NotTo(HaveOccurred())
|
|
Expect(leftovers).To(BeEmpty(), "a completed writer must not leak its partial tree onto a shared volume")
|
|
})
|
|
})
|