materializeLocked built a download task for every file in the resolved
snapshot unconditionally. A completed file is promoted from
.downloads/<hash> into snapshot/<path> and its blob deleted, so on any
re-entry (a controller pod roll, a resubmit, a crash) the new pass built a
task whose .downloads blob no longer existed, re-downloaded the whole file
from Hugging Face, and its AfterDownload even removed the already-complete
snapshot copy first. The only resume that worked was the downloader's
per-file .partial resume for a file caught mid-transfer; completed files
were never skipped.
Production consequence: installing longcat-video-avatar-1.5 (~35 GB after
allow_patterns) on a cluster whose controller rolls hourly (Flux image
automation) never converged across ~14 hours. Each roll restarted from the
first shard; the completed bytes on disk were repeatedly deleted and
re-fetched, and the artifact never promoted. curl of the same files from
inside the pod ran fine, proving the loss was the materializer re-fetching,
not the network.
Before building a task, check whether the file is already materialized and
verified in this staging tree's snapshot/ and, if so, keep it and count it
complete instead of downloading. "Materialized" means a regular file of the
expected size that passes the same verifyDownloadedFile check the download
path uses, so the kept manifest entry is byte-for-byte identical to a fresh
one and integrity is re-checked. The manifest requires a SHA-256 for every
file and non-LFS files carry none to borrow, so a hash is unavoidable for
the manifest anyway; a full re-hash of local disk is still orders of
magnitude cheaper than re-downloading, and the downloader re-verifies any
file it does fetch. Manifest entries are now written at their snapshot index
rather than appended in completion order, so a mix of skipped and downloaded
files keeps the resolved order that committedResult and staging read. The
unconditional root.Remove(destination) now runs only on the fresh-download
path; a kept file survives. Skips are logged at INFO with count and bytes so
an operator can see resume working.
This is the resume-side counterpart to the sibling defects on this path:
read/write error conflation and transient retry (#10985), hash-verify
progress accounting and silent success on an expired deadline (#11026), and
the response-header hang (#11053). The download machinery resumed a single
in-flight file; the materializer above it still threw away every completed
file on restart. It also makes orphan-partial adoption worth its cost:
an adopted tree's completed files were re-downloaded anyway until now.
Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
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>
flock(2) on CIFS/SMB returns EACCES when another client holds the lock:
the kernel maps STATUS_LOCK_NOT_GRANTED and STATUS_FILE_LOCK_CONFLICT to
-EACCES and never produces EWOULDBLOCK on that path. gofrs/flock only
recognises EWOULDBLOCK as contention, so TryLockContext returned a bare
"permission denied" and Ensure aborted. Both replicas then fell back to
legacy loading, which makes the worker download the whole repo in-band
inside LoadModel and blow the remote-load deadline.
Replace TryLockContext with an explicit wait loop over a new Locker
interface, classifying EWOULDBLOCK/EAGAIN/EACCES/EBUSY as contention.
EACCES is ambiguous at the syscall boundary but not here: the lock file
is already open O_CREATE|O_RDWR, so a real permission problem would have
failed the open with an *fs.PathError, and flock(2) documents no EACCES
on Linux at all. The wait is bounded (DefaultLockWait, overridable via
WithLockWait), so even a misclassification degrades to a delay. On
timeout the committed result is re-checked before reporting the new
ErrLockContended, so a peer that finished the work still wins.
Locker also exists so the contention path is testable without a network
filesystem: nothing in CI can make flock(2) return EACCES on demand.
Raise the fallback to error for a managedArtifactBackends backend, via a
shared config.LogArtifactFallback used by both call sites. For those
backends the legacy path is not graceful degradation, and the operator
otherwise sees only a timeout with no causal link. The fallback stays
non-fatal.
Drop the os.Chmod(layout.Lock, 0o600) after acquisition: flock.New
already creates the file 0600, and the chmod was gratuitous risk on a
nounix mount that ignores modes.
Fixes#10981
Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
fix(model-artifacts): load single-file HF snapshots from the file, not the dir
The managed Hugging Face artifact materializer (#10825) always pointed
backends at the snapshot *directory*
(.artifacts/huggingface/<key>/snapshot). For a single-file model
reference such as huggingface://nomic-ai/nomic-embed-text-v1.5-GGUF/nomic-embed-text-v1.5.f16.gguf,
the GGUF lives *inside* that directory, so llama.cpp was handed a
directory and failed with "gguf_init_from_reader: failed to read magic".
This has kept the tests-aio job red on master since the feature merged
(the embeddings e2e tests could not load text-embedding-ada-002).
Record the single file of a one-file snapshot as Resolved.PrimaryFile and
have ModelFileName() resolve to snapshot/<PrimaryFile> when it is set.
Multi-file snapshots (e.g. transformers repos consumed as a directory)
keep pointing at the snapshot directory. PrimaryFile is derived from the
resolved contents and is deliberately excluded from the artifact cache
key. estimateModelSizeBytes now derives the snapshot directory from the
cache key instead of ModelFileName(), so its manifest lookup is unaffected
by the file-vs-directory resolution.
Assisted-by: Claude:opus-4.8 [Claude Code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
