Commit Graph
9 Commits
Author SHA1 Message Date
Ettore Di Giacinto bbd5488871 fix(distributed): make backend.stop report what it stopped
backend.stop was the one lifecycle subject a worker never answered. The
controller published and returned nil as soon as the local publish
succeeded, so a stop that killed nothing, and a stop that failed
outright, were indistinguishable from one that worked.

The unload endpoint calls model.unload and then StopBackend. Only the
first is acknowledged, so the endpoint answered 200 while the backend
kept running and held its VRAM, and its own "backend stop failed" branch
could never run. The worker logged the failure and nobody saw it.

Give the subject a reply. The worker now enumerates the process keys it
terminated and reports any per-process error, so StopBackend fails when
the stop failed. Resolving to nothing stays a success: stopping a backend
that is not running leaves the caller in the state it asked for, and
eviction paths stop already-gone models routinely. The empty list is what
says nothing matched, and ReportsStoppedProcesses is what makes that
emptiness trustworthy, the same way BackendDeleteReply handles it.

A worker built before this reply still receives the request and still
stops the backend, it only stays silent, so a timeout degrades to the old
assumption rather than failing every stop on a fleet mid-upgrade. Only
silence degrades: a transport error is still reported, because
UnloadRemoteModel skips its registry cleanup for a node it could not
reach and needs to keep hearing about that.

Assisted-by: Claude:claude-opus-5 golangci-lint
2026-09-11 22:44:11 +00:00
mudler's LocalAI [bot]andEttore Di Giacinto 82c191afad fix(distributed): keep model replicas config-consistent (#11664)
* docs: design configurable copy buffering

Document the context-aware copy buffer option and its validation plan.

Assisted-by: Codex:gpt-5

* docs: design durable distributed staging operations

Assisted-by: Codex:gpt-5

* docs: design distributed model config revisions

Assisted-by: Codex:GPT-5 [apply_patch] [exec_command]

* feat(config): add stable model revisions

Hash typed model configuration and effective protobuf options deterministically for distributed revision comparisons.

Assisted-by: Codex:GPT-5 [apply_patch] [exec_command]

* feat(worker): acknowledge exact model stops

Assisted-by: Codex:GPT-5 [apply_patch] [exec_command]

* feat(nodes): track model config revisions

Assisted-by: Codex:GPT-5 [apply_patch]

* fix(distributed): retry quarantined model cleanup

Stop quarantined replicas by exact process identity, retain failed cleanup as durable capped retries, and compare-and-delete only the claimed registry row. Process one sufficiently leased row at a time so multiple frontends cannot duplicate slow cleanup work.

Assisted-by: Codex:gpt-5

* fix(distributed): bind loads to config revisions

Assisted-by: Codex: GPT-5 [OpenAI Codex]

* fix(modeladmin): apply config revisions consistently

Route model edits, patches, state changes, deletion, and peer refreshes through the same revision lifecycle. Quarantine stale replicas before exact cleanup and report durable pending cleanup without failing successful config writes.

Assisted-by: Codex: GPT-5 [OpenAI Codex]

* feat(distributed): expose model config revision state

Document replica revision observability and durable cleanup behavior. Keep pending cleanup explicit in model mutation responses and verify endpoint contracts expose revision state without serialized load options.

Assisted-by: Codex:GPT-5 [OpenAI Codex]

* test(distributed): cover model revision convergence

Exercise cross-frontend quarantine, stale replay rejection, exact cleanup retry, worker re-registration, and current-generation replica convergence against the distributed PostgreSQL harness.

Assisted-by: Codex:gpt-5

* fix(distributed): pass config revision CI checks

Keep configured gallery sources out of authoritative runtime snapshots only after validating their real schema, and harden rollback snapshots against symlink races and non-regular files.

Assisted-by: Codex: GPT-5 [OpenAI Codex]

---------

Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
2026-08-22 22:44:03 +02:00
Ettore Di Giacinto 2383726d6d Revert "chore(tests): Avoid network, sleep and more during tests" (#11601)
Revert "chore(tests): Avoid network, sleep and more during tests (#11050)"

This reverts commit cb3bf7af3f.
2026-08-19 16:39:39 +02:00
Richard Palethorpeandlocalai-org-maint-bot cb3bf7af3f chore(tests): Avoid network, sleep and more during tests (#11050)
* test: make coverage failures observable

Keep per-root logs, reject concurrent coverage runs, and avoid relying on /bin/sleep in the worker timeout test.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: parallelize coverage without remote fixtures

Assisted-by: Codex:gpt-5 [apply_patch] [exec_command]
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: add offline resource infrastructure

Introduce versioned resource manifests, a checksum-verified CAS preparer, offline test wrappers, and a guarded network transport. Replace live Hugging Face, GitHub, and OCI cases with deterministic fixtures and inject fixture metadata into importer discovery.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: enforce offline resource replay

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: harden offline resource refresh

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: expose slow coverage waits

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: eliminate avoidable wall-clock waits

Inject a clock into Hugging Face retry handling, reuse a process-scoped PostgreSQL container with per-spec schemas in the nodes suite, and poll local import jobs promptly.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: remove repeated fixture startup waits

Share PostgreSQL fixtures across parallel endpoint and agent suite workers, and make the worker Free deadline injectable so the wedged-backend test does not spend five seconds on wall-clock time.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: fix offline resource CI portability

Normalize Docker archive metadata before content addressing, derive archive checksums during explicit refreshes, make network lint portable to macOS, and prepare distributed images before running their offline suite.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* ci: cache Go modules before offline tests

Warm the complete module graph before the Linux and macOS test jobs enter offline replay mode, so tool dependencies such as Ginkgo are not fetched through the guarded proxy.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: drop the static network lint in favour of real isolation

The offline test suite already prevents tests from reaching the network
twice over: run-test-linux-offline.sh puts the test process in a cgroup
and REJECTs egress outside the private ranges, and HardenedTransport
installs testnetwork.LocalGuard to refuse dials that resolve to a public
address. Both fail the test with a precise error at the moment of the
dial.

test-network-lint.sh added neither. Its diff stage defaulted to a HEAD
base, so on a clean checkout it compared the tree against itself and
inspected nothing; the branch's own commits were never examined. It only
produced output when an earlier job step dirtied the tree, and then it
matched a bare https?:// against whatever changed. make react-ui runs
npm install rather than npm ci, so CI rewrote
core/http/react-ui/package-lock.json and the lint reported an npm
registry URL as forbidden test network access:

  +      "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.25.tgz",

Its fingerprint stage was self-defeating in a quieter way: hashing the
whole tree's network-mechanism inventory meant every rebase onto a master
that touched any _test.go needed a manual baseline bump, so the check
mostly caught its own staleness.

Remove the script, its make target and the two prerequisite edges, along
with the test-network: fixture markers that existed only to suppress it.
The isolation itself is untouched.

Assisted-by: Claude:claude-opus-5 [go vet]
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* ci: keep hidden files in the offline test bundle artifact

Cherry-picked from 15a37b0ac on the remote branch. The offline bundle lives
under .cache/, which actions/upload-artifact skips by default, so the Linux
job packed an artifact missing the very file the next step restores.

The other half of 15a37b0ac moved test-network-lint out of the `test` and
`test-coverage` prerequisite lists into a recipe line, so parallel make could
not fingerprint the tree while generated fixtures were still changing. That
is dropped: the preceding commit removes the lint entirely, and the race it
worked around is one more reason a whole-tree fingerprint was the wrong
mechanism.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* refactor: share bounded exponential backoff

Use overflow-safe saturating arithmetic for retry delays across model import polling, downloads, registration, node operations, and model loading. Keep model import status checks responsive initially while capping their interval at 500ms.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* ci: mirror Jetson Python wheels

Keep the CUDA aarch64 wheel subset in GHCR and serve it as a local PEP 503 index during L4T backend builds, preserving last-known-good packages through upstream outages.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* docs(agents): index the Jetson wheels mirror

Mention the GHCR-hosted L4T wheel mirror in the CI caching guide summary so maintainers can find its outage and cache documentation.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* ci: add defensive build network proxy

Record build destinations and byte counts, retry observable idempotent HTTP downloads, and isolate explorer database tests that race under coverage.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(kokoros): implement updated backend trait

Return unimplemented for image upscaling, matching the backend's other unsupported modalities after the protobuf API update.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(ci): clear recovered proxy errors

Do not mark a request failed when a later safe retry succeeds.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* ci: require HTTPS build interception

Inject a short-lived proxy CA into BuildKit and Dockerfile RUN steps, reject plain HTTP and opaque tunnels, and retain method/status/byte telemetry for verified HTTPS traffic.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(ci): preserve system trust in unproxied builds

Mount the generated interception CA at a dedicated secret path and add it to the trust bundle only in proxy-aware dependency stages. This prevents optional secret mounts from masking the system CA bundle in ordinary backend test builds.

Install the requested Go toolchain before starting the proxy and satisfy cleanup error checks found by CI lint.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(ci): persist build proxy trust

Install the generated proxy CA through the system-managed local certificate directory so ca-certificates upgrades retain it. Avoid turning canceled matrix jobs into proxy cleanup failures.

Assisted-by: Codex:gpt-5

Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(ci): trust proxy in nested build scripts

Install the build proxy CA before nested source fetches, route the DS4 package setup through the HTTPS mirror helper, and avoid repeated OCI setup in gallery behavior tests.

Assisted-by: Codex:gpt-5

Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(ci): use HTTPS apt sources for Bonsai

Rewrite ARM64 package sources before installing GCC and check gallery fixture cleanup errors so the optimized tests satisfy errcheck.

Assisted-by: Codex:gpt-5

Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(privacy-filter): trust build proxy CA

Install the mounted build proxy certificate before privacy-filter's make target fetches its HTTPS sources, for both source and prebuilt builder paths.\n\nAssisted-by: Codex:gpt-5

Signed-off-by: Richard Palethorpe <io@richiejp.com>

* test: fail on hidden offline egress

Count cgroup-scoped firewall rejects and fail the offline test harness with bounded aggregate diagnostics. Inject the gen-audio GGUF probe so fixture-backed importer tests do not attempt real network access.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

* fix(ci): preserve system CA trust

Build a combined runner certificate bundle instead of replacing public roots with the generated proxy CA. Centralize additive container installation in the shared proxy CA helper.

Assisted-by: Codex:gpt-5
Signed-off-by: Richard Palethorpe <io@richiejp.com>

---------

Signed-off-by: Richard Palethorpe <io@richiejp.com>
Co-authored-by: localai-org-maint-bot <306269227+localai-org-maint-bot@users.noreply.github.com>
2026-08-19 10:59:31 +02:00
mudler's LocalAI [bot]andEttore Di Giacinto 878a0d00a1 fix(distributed): reaper reaps live backends, ghost model stubs, in_flight leak, sidecar staging runaway (#11142)
* fix(distributed): stop the probe reaper from orphaning busy backends

The reconciler's liveness probe is a 1s gRPC HealthCheck, and a single
failed probe deleted the model's node_models row. A backend that is
merely busy cannot answer it: single-threaded Python backends (video and
avatar generation) block for minutes inside one request, so the reaper
was deleting registry rows for backends that were alive and mid-request.

The model then vanished from the nodes page while it was still
generating, and because the row was gone the in-flight decrement had
nothing to decrement ("DecrementInFlight: no matching row or already
zero"). Every subsequent request re-routed and re-staged the full model
from scratch.

Two guards:

  - Replicas with in-flight requests are excluded in SQL. A row that is
    actively serving is proof of life, and the running request is
    exactly what stops the backend from answering the probe.

  - Idle replicas must miss three CONSECUTIVE probes before removal, so
    a transient blip cannot orphan a live replica. A successful probe
    resets the streak.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude:claude-opus-5 [Claude Code]

* fix(distributed): drop the local model stub when its last replica goes

In distributed mode every routed model leaves an in-process stub in the
frontend's ModelLoader, and DistributedModelStore.Range reports local
stubs UNION the registry rows. Every registry removal path deletes only
the DB row, so the stub outlived the replica and the model was reported
as loaded forever.

That is the "loaded on the home page, absent from every node" ghost:
/system reads the union and still sees the stub, while /api/nodes/models
reads the registry and correctly sees nothing. It never self-healed,
and both frontend replicas showed it independently.

The replica-removed chokepoint could not fix this as it stood, because
it held a SINGLE hook that the prefix cache already owned, and it was
registered only when the prefix cache was enabled. Registering a second
listener would have silently displaced the first.

  - Turn replicaRemovedHook into a list (AddReplicaRemovedHook), so
    independent subsystems can each register without displacing others.
  - Add NewLocalStubInvalidator, which drops the local stub once no
    healthy replica of the model remains anywhere in the cluster, and
    wire it unconditionally in startup.

The stub is kept while another node still serves the model: the
frontend is right to consider it loaded, and each request re-routes
through SmartRouter to pick a live replica anyway.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude:claude-opus-5 [Claude Code]

* fix(distributed): stop staging checksum sidecars back to workers

The file transfer server writes a "<file>.sha256" sidecar next to every
file it accepts. The sender walked the model directory with no filter,
so it staged those sidecars too, and the receiver duly wrote a sidecar
for each sidecar. Every staging pass multiplied the tree:

  config.json -> config.json.sha256 -> config.json.sha256.sha256 -> ...

One LongCat snapshot had grown to 498 files, 466 of them chained, up to
29 levels deep, and the staged file count climbed on every pass. This
inflates each transfer and grows disk without bound on both ends.

Skip hash sidecars in stageDirectory, and mirror the skip in
countStageableFiles so the progress bar still reaches 100%. The check is
"a sidecar sitting next to a real file" rather than a blanket suffix
ban, so a model that genuinely ships a .sha256 payload with no
corresponding base file is still transferred.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude:claude-opus-5 [Claude Code]

* fix(distributed): classify the liveness probe instead of gating on in_flight

The previous commit excluded replicas with in-flight requests from the probe
reaper. That was the wrong guard, and could invert the bug it fixed.

in_flight has no decrement guarantee: track() balances its increment with a
defer, but a frontend killed mid-request never runs it, and the load-time
reservation is released only when the first inference completes. Nothing
resets a leaked counter. Gating the reaper on it therefore meant a leaked
counter would shield a genuinely dead replica from ever being reaped.

Nor was patience alone a fix: three misses at the default interval is ~90s of
silence, while the generation that triggered this blocks for 15+ minutes.

The real conflation was in the probe itself. A gRPC HealthCheck against the
backend's serving port measures "is it idle enough to answer", not "does the
process exist", and probeLoadedModels discarded the error that tells them
apart. Because the gRPC client is lazy, the status code is decisive:

  - DeadlineExceeded: transport fine, nothing serviced the RPC. Busy.
  - Unavailable: nothing is listening. Gone.

ModelProber now returns a ProbeOutcome, and only ProbeUnreachable counts
toward the reap threshold. ProbeBusy clears the streak: it is evidence of
life. A blackholed network reads as busy too, deliberately, since whole-node
failure is the health monitor's job.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude:claude-opus-5 [Claude Code]

* feat(distributed): reconcile replicas against worker-reported processes

Probing a backend's own serving port cannot distinguish "busy" from "gone"
without inferring it from an error code. The worker can answer directly: it
spawned the process, holds the handle, and its reply is not blocked by
whatever that backend is doing.

Adds a models.running request-reply subject. The worker answers out of its
in-memory process table, reporting each live process as (modelID,
replicaIndex, address) — the supervisor's process keys are `modelID#replica`,
which is isomorphic to a NodeModel row, so the reconciler can diff the two
directly.

reconcileNodeProcesses runs before the port probe and reaps rows for models
the worker is not running. Models the worker vouches for get updated_at
bumped, which takes them out of the port prober's stale set entirely: that is
what keeps a backend deep in a long generation away from the probe in the
first place, rather than relying on classifying its silence after the fact.

A worker that does not answer is skipped, not assumed empty. A messaging
failure says nothing about the processes, and assuming the worst would delete
a node's rows on a transient NATS blip; the port probe stays as the fallback
for those nodes. Rows younger than probeStaleAfter are ignored so a freshly
created row is never judged against a process table that has not caught up.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude:claude-opus-5 [Claude Code]

* fix(distributed): stop in_flight leaking and pin replicas against eviction

A leaked in_flight counter is not cosmetic. FindLRUModel,
FindGlobalLRUModelWithZeroInFlight and the router's eviction query all require
in_flight = 0, so a replica whose counter never came back is pinned and its
VRAM is unreclaimable for the lifetime of the process.

Two halves.

The source: routing reserves in_flight = 1 at load time so a freshly loaded
replica is not evicted out from under the request that caused the load. That
reservation was released ONLY by the first inference completing, so a route
torn down before any inference ran (client disconnect, handler error, failure
between load and the backend call) stranded it. newRouteResult now wires the
reservation to a sync.Once fired by whichever comes first, the first inference
or route teardown, and replaces three copies of the old wiring.

The backstop: a sweeper for counters leaked by paths that cannot run a defer
at all, such as a frontend killed mid-request.

Identifying a leak by elapsed time alone is unsafe. IncrementInFlight stamps
last_used at request START and nothing moves it while the request runs, so a
long generation is indistinguishable from a leak by age, and resetting there
would expose a serving model to eviction. The probe supplies the missing bit:
a backend that answers a health check promptly is not inside a request,
because that is precisely what a busy one cannot do. Requiring the row to also
be idle for 30 minutes covers backends that serve in parallel and can answer
while working, since those keep last_used fresh through each new increment.

Two existing tests asserted the old behaviour ("No decrement on Release").
That assertion was the leak, so both now pin the release instead.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude:claude-opus-5 [Claude Code]

---------

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
2026-07-27 18:36:31 +02:00
mudler's LocalAI [bot]andEttore Di Giacinto f735cb24c0 fix(worker): reap deleted backends and stop models that live on a worker (#10956)
* fix(worker): reap deleted backends and stop models that live on a worker

Three related backend-lifecycle defects, all reachable from the same
production incident on a Jetson/Thor worker: a deleted backend's gRPC
process survived ~40 minutes with its directory removed from disk, a later
model load was routed to that orphan and failed with a certifi path pointing
into the deleted directory, and the admin could not stop the model because
the frontend reported it as not loaded.

1. backend.delete orphaned the process it claimed to delete
------------------------------------------------------------
s.processes is keyed by `modelID#replicaIndex` (buildProcessKey), so the
backend name never appeared in a key and was recorded nowhere on the
process. backend.delete resolved its target via isRunning/stopBackend, whose
prefix path only matches a bare *modelID* - a delete keyed on a backend name
resolved to zero keys, the stop silently no-op'd, and the files were removed
out from under a live process.

The install fast path then handed that orphan back out: it returns any live
process for the (model, replica) slot without checking which backend started
it, so a reinstalled variant inherited the deleted backend's port.

- Record backendName on backendProcess, threaded installBackend ->
  startBackend.
- Add resolveProcessKeysForBackend, matching the recorded name and resolving
  alias <-> concrete via ListSystemBackends *before* DeleteBackendFromSystem
  erases the metadata that carries the alias. Alias resolution failure
  degrades to name-only matching so a delete never fails on it.
- backend.stop goes through resolveStopTargets, which accepts a backend
  name, a model name, or an exact modelID#replica key. Its payload field is
  named "backend" but is published with all three meanings: the admin UI
  sends a backend name, UnloadRemoteModel sends a model name, and the
  router's abandoned-load reap (#10948) sends an exact replica key.
  Narrowing it to backend names alone would strand the latter two.
  backend.delete stays strict - its identifier is unambiguously a backend.
- Gate the install fast path on processMatchesBackend so a slot held by a
  different backend is restarted rather than reused. Processes with no
  recorded name (pre-upgrade) are accepted, so rollout does not restart
  every running backend.
- stopBackendExact reports a real stop failure - the process still being
  alive afterwards, which is precisely what finishBackendStop already
  detects to keep the entry and its port reserved - and backend.delete no
  longer replies success when it knew about a process and could not kill it.
  "No process was running" stays a success but is logged, so the orphan case
  is visible rather than silent.

2. /backend/shutdown reported a running model as missing
---------------------------------------------------------
ModelLoader.deleteProcess short-circuits on a miss in this replica's
in-memory store. In distributed mode the authoritative record of "is this
model loaded" is the shared node registry: a frontend replica that never
served the model itself (load balancer picked a peer, or the replica
restarted) has no local entry. The remote unload path that pkg/model
documents ("when ShutdownModel is called for a model with no local process,
UnloadRemoteModel is called") sat behind that short-circuit, unreachable in
exactly the case it exists for. #10865 reworked this function but kept the
short-circuit at the top, so the gap survived that refactor.

- deleteProcess consults the remote unloader on a local-store miss, via a
  shared unloadRemote helper so this branch and the existing
  no-local-process branch both prefer #10865's RemoteModelContextUnloader,
  preserving force propagation across the distributed boundary.
- UnloadRemoteModelContext reports ErrRemoteModelNotLoaded when no node has
  the model; it previously returned nil, making a no-op stop
  indistinguishable from a real one. The converse case (nodes have it, none
  could be stopped) already errors since #10865 joined the per-node
  failures, so that half of the original fix was dropped as redundant.
- Only when the model is absent locally AND cluster-wide does the endpoint
  report not-found, now 404 naming both scopes rather than a bare 500.
- modelNotFoundErr becomes the exported ErrModelNotFound so the HTTP layer
  can map it without string matching; watchdog's identity comparison becomes
  errors.Is.

3. Coverage for the bounded Free() that #10865 shipped untested
----------------------------------------------------------------
The original branch also bounded the pre-stop Free(), but #10865 landed that
fix first (workerBackendFreeTimeout, applied in both stopBackendExact and
handleModelUnload). That production change is therefore DROPPED here as
superseded - master's version is strictly better, since it also releases the
supervisor mutex across the call and keeps the port reserved until
termination completes.

What #10865 did not ship is a test, and the bound is load-bearing: the
router-side reap in #10948 sends backend.stop for an abandoned load, and
against a wedged backend an unbounded Free would swallow that stop before it
reached the process. Nothing failed if the bound regressed.

The spec stands up a real gRPC backend server whose Free handler never
returns - what a Python backend looks like when its single worker thread
(PYTHON_GRPC_MAX_WORKERS=1 on 37 backends) is occupied by a stuck LoadModel.
A stub socket is not sufficient and was tried first: without a completed
HTTP/2 handshake, gRPC's own ~20s connect timeout ends the call, so that
version passed against the very bug it targets. With the connection READY,
only the caller's deadline can end it, so the spec hangs to its 60s limit if
the timeout is removed and passes with it.

Its fixture process is deliberately never started. go-processmanager v0.1.1
writes Process.pid from readPID() without synchronization, so a live process
races its own monitor goroutine under -race - reproducible with a bare
Run()+Stop() and unrelated to this spec. Since
scripts/model-lifecycle-conformance.sh runs this package with -race and is
fail-closed, starting one would turn that gate red on an upstream defect. An
unstarted process still proves the point: the stop is reached and the slot
released, which is exactly what an unbounded Free prevents.

Verified: make lint (new-from-merge-base origin/master) reports 0 issues;
scripts/model-lifecycle-conformance.sh passes all three stages including the
FizzBee liveness check (1458 states, IsLive: true).

Assisted-by: Claude:claude-opus-4-8 golangci-lint
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* fix(distributed): keep remote unload idempotent, ask presence separately

2035a4d25 made UnloadRemoteModel return ErrRemoteModelNotLoaded when no node
holds the model, so ShutdownModel could answer 404 instead of a misleading
500. That narrowed a shared adapter contract to serve one caller and broke
the documented idempotent-unload guarantee, which CI caught on PR #10956:

  [FAIL] Node Backend Lifecycle (NATS-driven) > NATS backend.stop events
         should be no-op for models not on any node [Distributed]
         Expected success, but got: model not loaded on any node

The spec name states the contract outright. The matching unit assertion was
updated in that commit; this e2e one was missed because it lives under
tests/e2e/ with no build tags and does not run in package-scoped test runs.

Caller audit - who breaks when an idempotent unload becomes an error:

- pkg/model/watchdog.go:902 (LRU memory reclaimer) is the serious one. It
  untracks a model ONLY when shutdown returns nil or ErrModelNotFound. A new
  error type means the model is never untracked, so the reclaimer keeps
  re-selecting the same entry and never reclaims - a live wedge whenever a
  local store entry outlives the remote model.
- core/services/galleryop/managers_local.go:43 (DeleteModel) would warn on
  every deletion of an already-unloaded model.
- core/services/modeladmin/{state,config,remote_sync}.go stop instances
  best-effort against models that are frequently not loaded.
- deleteProcess itself: the no-local-process branch returns the unload result
  directly, so a stale local entry for a model no longer on any node turned a
  previously-successful cleanup into a failure.

Only ShutdownModel wants the distinction, and only on the local-store-miss
path. So the distinction moves to the caller instead of the contract:

- UnloadRemoteModel/UnloadRemoteModelContext return nil again when no node
  has the model, and ErrRemoteModelNotLoaded is removed.
- New optional RemoteModelPresenceChecker (HasRemoteModel) answers the
  question directly. deleteProcess consults it BEFORE unloading, because an
  idempotent unload cannot report afterwards whether anything was stopped.
  Absent locally AND cluster-wide is the only case that reports 404.
- A failed registry lookup is surfaced rather than reported as absence: an
  unreachable registry is not evidence a model is gone, and answering a
  confident 404 off a failed lookup is how an operator gets told a running
  model does not exist.
- Unloaders that predate the extension keep working - deleteProcess attempts
  the unload rather than refusing it - and compile-time assertions in the
  nodes package now pin all three optional interfaces, since both are
  consumed by runtime type assertion where drift degrades behavior silently
  instead of failing the build.

The contract is now pinned at both levels that disagreed, each spec pointing
at the other: "with no nodes returns nil" in unloader_test.go and "should be
no-op for models not on any node" in node_lifecycle_test.go.

Verified: full distributed e2e suite 233 passed / 0 failed (the suite that
failed 232/1 in CI); pkg/model and core/services/nodes green; make lint
new-from-merge-base reports 0 issues.

Assisted-by: Claude:claude-opus-4-8 golangci-lint
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* fix(distributed): drop replica rows when a worker stops a backend

A worker returns a stopped backend's gRPC port to its allocator as soon as
the process is confirmed dead, and hands it to the next backend that starts.
The controller's NodeModel row for the old address survives, and both
SmartRouter.probeHealth and the HealthMonitor per-model probe verify
liveness, not identity, so once an unrelated backend binds the recycled port
the stale row passes every check and the request is served by the wrong
backend instead of failing.

backend.delete is newly able to trigger this: before #10956 a delete never
actually stopped a process, so it never recycled a port. backend.upgrade has
the identical gap and always did — upgradeBackend force-stops every process
using the binary and starts none back up, while
DistributedBackendManager.UpgradeBackend never removes rows. model.unload is
the one path that gets this right today: it calls RemoveAllNodeModelReplicas
straight after StopBackend.

Report the process keys the worker terminated on the delete and upgrade
replies, and drop the matching rows in RemoteUnloaderAdapter, which already
holds a ModelLocator with RemoveNodeModel. All three call sites funnel
through that adapter, so no new interface, DB migration, or proto change is
needed. A key is reported only once its process is confirmed gone, so the
list stays trustworthy on the partial-failure replies too.

Old workers never populate the new fields. ReportsStoppedProcesses tells
"stopped nothing" apart from "does not report", so an old worker's silence
falls back to the pre-existing probe-based staleness recovery instead of
being mistaken for a completed cleanup.

Quarantine released ports for a short window as an interlock covering the
NATS round-trip between the worker freeing the port and the controller
dropping the row. It is deliberately not derived from HealthCheckInterval:
that cadence is operator-tunable and the per-model reaper can be disabled
outright, so coupling a worker-local constant to it would be silently wrong
on some clusters. Eager row removal is the fix; the delay only closes the
handoff gap.

Identity verification in probeHealth was considered and rejected: Health and
Status carry no backend identity, so it needs a proto change plus an
implementation in 36 Python and 4 C++ Health servicers, it is fail-open for
any backend not yet rebuilt, and the probeCache short-circuit means it would
not even execute during the 30s window where the misroute happens.

Fixes #10952
Refs #10954, #10956

Assisted-by: Claude:claude-opus-4-8 golangci-lint
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* chore(deps): bump go-processmanager, assert real backend termination

go-processmanager wrote Process.PID from readPID() with no synchronization
while its own monitor goroutine cleared the same field on exit, so a bare
Run()+Stop() tripped the race detector without any concurrent access from
the caller. LocalAI hit this on every backend stop.

Upstream fixed it in a94e2b7 by guarding PID with a mutex and adding
CurrentPID() as a race-safe accessor. The exported field was kept to avoid
a breaking change but is now deprecated: a direct read still races the
monitor. No tag carries the fix yet, so pin the pseudo-version.

GetGRPCPID reads through CurrentPID() instead of the field. The accessor
returns the same string under an RLock, so the empty-PID and strconv error
paths are unchanged; it is the only direct field read in the tree.

With the race gone, the Free-timeout spec no longer has to leave its
fixture process unstarted. It now runs a real child and asserts the child
genuinely exits, which is exactly what the earlier workaround gave up: the
spec could show the stop was reached and the slot released, but not that
SIGTERM ever landed. Termination is observed through Done(), which closes
only once the library has waited on the child. The pidfile-based liveness
helpers cannot serve here, because Stop() deletes the pidfile while
releasing the handle and so reports "not alive" even if no signal was sent.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
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>
2026-07-20 01:09:22 +02:00
Richard Palethorpe 9c43b2da8f fix(model): make backend shutdown model-scoped (#10865)
Avoid holding the global loader lock across backend lifecycle waits and propagate forced shutdown through distributed workers. Track parallel requests with in-flight counters and reserve worker ports until process termination.

Add focused race tests and an authoritative FizzBee lifecycle model with a fail-closed conformance target.

Assisted-by: Codex:GPT-5 [FizzBee] [Ginkgo]

Signed-off-by: Richard Palethorpe <io@richiejp.com>
2026-07-19 08:43:17 +02:00
Ettore Di Giacinto 3568b2819d fix(gallery): keep auto-upgrade off non-dev backends when -development is installed (#9736)
A `-development` backend variant (e.g. `cuda12-llama-cpp-development`)
shares its `alias` with the stable counterpart and is meant to be a
drop-in replacement via ListSystemBackends alias resolution. Two paths
in the auto-upgrade flow let the stable variant slip back in on top of
the user's explicit dev pick:

1. ListSystemBackends emits a synthetic alias row keyed by the alias
   name that re-uses the chosen concrete's metadata pointer. In
   distributed mode, the worker's handleBackendList serialised that
   row over NATS as `{Name: <alias>, URI: <dev URI>, Digest: <dev>}`
   — the frontend can't reconstruct the alias relationship, and the
   wire-rebuilt row then carried `Metadata.Name = <alias>` and
   resolved against an unrelated gallery entry on the next upgrade
   check.
2. CheckUpgradesAgainst happily iterated the synthetic row in
   single-node too. Today the duplicate gallery lookup is harmless
   because both rows share the same `Metadata.Name`, but any gallery
   change that gives a meta backend a version, or any concrete
   sharing its alias with a dev counterpart, would surface a phantom
   non-dev upgrade and auto-upgrade would install it — shadowing the
   dev one through alias-token preference.

Two layered fixes:

- `core/services/worker/lifecycle.go` (`handleBackendList`): drop
  rows where the map key differs from `b.Metadata.Name`. Concrete
  and meta entries always have `key == Metadata.Name`; only synthetic
  aliases violate it. Workers now report only what's actually on disk;
  the per-node UI listing and CheckUpgrades both stop seeing phantoms.
- `core/gallery/upgrade.go` (`CheckUpgradesAgainst`): iterate by key,
  skip rows where `key != Metadata.Name` (belt-and-suspenders for any
  caller-supplied installed set), and apply the dev-aware rule —
  build a set of installed `Metadata.Name`s and drop any non-dev
  candidate `X` whose `X-<devSuffix>` counterpart is installed. Uses
  the configured dev suffix from `getFallbackTagValues(systemState)`.

Manual `POST /api/backends/upgrade/<name>` is unaffected: it goes
straight through `bm.UpgradeBackend(name)` without consulting the
suppression list, so users who genuinely want the stable variant
upgraded can still trigger it explicitly.

Tests in core/gallery/upgrade_test.go cover three cases under
"CheckUpgradesAgainst (distributed)": dev-only installed → only the
dev surfaces; both variants installed → dev still wins; synthetic
alias row is ignored. Generic backend names are used to avoid the
capability filter dropping cuda-prefixed entries on a CPU-only host.

Assisted-by: Claude:claude-opus-4-7

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-05-09 18:20:00 +02:00
LocalAI [bot]andEttore Di Giacinto e5d7b84216 fix(distributed): split NATS backend.upgrade off install + dedup loads (#9717)
* feat(messaging): add backend.upgrade NATS subject + payload types

Splits the slow force-reinstall path off backend.install so it can run on
its own subscription goroutine, eliminating head-of-line blocking between
routine model loads and full gallery upgrades.

Wire-level Force flag on BackendInstallRequest is kept for one release as
the rolling-update fallback target; doc note marks it deprecated.

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* feat(distributed/worker): add per-backend mutex helper to backendSupervisor

Different backend names lock independently; same backend serializes. This
is the synchronization primitive used by the upcoming concurrent install
handler — without it, wrapping the NATS callback in a goroutine would
race the gallery directory when two requests target the same backend.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* fix(distributed/worker): run backend.install handler in a goroutine

NATS subscriptions deliver messages serially on a single per-subscription
goroutine. With a synchronous install handler, a multi-minute gallery
download would head-of-line-block every other install request to the
same worker — manifesting upstream as a 5-minute "nats: timeout" on
unrelated routine model loads.

The body now runs in its own goroutine, with a per-backend mutex
(lockBackend) protecting the gallery directory from concurrent operations
on the same backend. Different backend names install in parallel.

Backward-compat: req.Force=true is still honored here, so an older master
that hasn't been updated to send on backend.upgrade keeps working.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* feat(distributed/worker): subscribe to backend.upgrade as a separate path

Slow force-reinstall now lives on its own NATS subscription, so a
multi-minute gallery pull cannot head-of-line-block the routine
backend.install handler on the same worker. Same per-backend mutex
guards both — concurrent install + upgrade for the same backend
serialize at the gallery directory; different backends are independent.

upgradeBackend stops every live process for the backend, force-installs
from gallery, and re-registers. It does not start a new process — the
next backend.install will spawn one with the freshly-pulled binary.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* feat(distributed): add UpgradeBackend on NodeCommandSender; drop Force from InstallBackend

Master now sends to backend.upgrade for force-reinstall, with a
nats.ErrNoResponders fallback to the legacy backend.install Force=true
path so a rolling update with a new master + an old worker still
converges. The Force parameter leaves the public Go API surface
entirely — only the internal fallback sets it on the wire.

InstallBackend timeout drops 5min -> 3min (most replies are sub-second
since the worker short-circuits on already-running or already-installed).
UpgradeBackend timeout is 15min, sized for real-world Jetson-on-WiFi
gallery pulls.

Updates the admin install HTTP endpoint
(core/http/endpoints/localai/nodes.go) to the new signature too.

router_test.go's fakeUnloader does not yet implement the new interface
shape; Task 3.2 will catch it up before the next package-level test run.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* test(distributed): update fakeUnloader for new NodeCommandSender shape

InstallBackend lost its force bool param (Force is not part of the public
Go API anymore — only the internal upgrade-fallback path sets it on the
wire). UpgradeBackend gained a method. Fake records both call slices and
provides an installHook concurrency seam for upcoming singleflight tests.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* test(distributed): cover UpgradeBackend's new subject + rolling-update fallback

Task 3.1 changed the master to publish UpgradeBackend on the new
backend.upgrade subject; the existing UpgradeBackend tests scripted the
old install subject and so all 3 began failing as expected. Updates them
to script SubjectNodeBackendUpgrade with BackendUpgradeReply.

Adds two new specs for the rolling-update fallback:
  - ErrNoResponders on backend.upgrade triggers a backend.install
    Force=true retry on the same node.
  - Non-NoResponders errors propagate to the caller unchanged.

scriptedMessagingClient gains scriptNoResponders (real nats sentinel) and
scriptReplyMatching (predicate-matched canned reply, used to assert that
the fallback path actually sets Force=true on the install retry).

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* fix(distributed): coalesce concurrent identical backend.install via singleflight

Six simultaneous chat completions for the same not-yet-loaded model were
observed firing six independent NATS install requests, each serializing
through the worker's per-subscription goroutine and amplifying queue
depth. SmartRouter now wraps the NATS round-trip in a singleflight.Group
keyed by (nodeID, backend, modelID, replica): N concurrent identical
loads share one round-trip and one reply.

Distinct (modelID, replica) keys still fire independent calls, so
multi-replica scaling and multi-model fan-out are unaffected.

fakeUnloader gains a sync.Mutex around its recording slices to keep
concurrent test goroutines race-clean.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* test(e2e/distributed): drop force arg from InstallBackend test calls

Two e2e test call sites still passed the trailing force bool that was
removed from RemoteUnloaderAdapter.InstallBackend in 9bde76d7. Caught
by golangci-lint typecheck on the upgrade-split branch (master CI was
already green because these tests don't run in the standard test path).

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* refactor(distributed): extract worker business logic to core/services/worker

core/cli/worker.go grew to 1212 lines after the backend.upgrade split.
The CLI package was carrying backendSupervisor, NATS lifecycle handlers,
gallery install/upgrade orchestration, S3 file staging, and registration
helpers — all distributed-worker business logic that doesn't belong in
the cobra surface.

Move it to a new core/services/worker package, mirroring the existing
core/services/{nodes,messaging,galleryop} pattern. core/cli/worker.go
shrinks to ~19 lines: a kong-tagged shim that embeds worker.Config and
delegates Run.

No behavior change. All symbols stay unexported except Config and Run.
The three worker-specific tests (addr/replica/concurrency) move with
the code via git mv so history follows them.

Files split as:
  worker.go        - Run entry point
  config.go        - Config struct (kong tags retained, kong not imported)
  supervisor.go    - backendProcess, backendSupervisor, process lifecycle
  install.go       - installBackend, upgradeBackend, findBackend, lockBackend
  lifecycle.go     - subscribeLifecycleEvents (verbatim, decomposition is
                     a follow-up commit)
  file_staging.go  - subscribeFileStaging, isPathAllowed
  registration.go  - advertiseAddr, registrationBody, heartbeatBody, etc.
  reply.go         - replyJSON
  process_helpers.go - readLastLinesFromFile

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

* refactor(distributed/worker): decompose subscribeLifecycleEvents into per-event handlers

The 226-line subscribeLifecycleEvents method packed eight NATS subscriptions
inline. Each grew context-shaped doc comments mixed with subscription
plumbing, making it hard to read any one handler without scrolling past the
others. Extract each handler into its own method on *backendSupervisor; the
subscriber becomes a thin 8-line dispatcher.

No behavior change: each method body is byte-equivalent to its corresponding
inline goroutine + handler. Doc comments that were attached to the inline
SubscribeReply calls migrate to the new method godocs.

Adding the next NATS subject is now a 2-line patch to the dispatcher plus
one new method, instead of grafting onto a monolith.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>

---------

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
2026-05-08 16:24:54 +02:00