Commit Graph
4 Commits
Author SHA1 Message Date
Ettore Di Giacinto 793c4e211f fix(distributed): read a departed agent tunnel as the routing fact it is
Task 4 gave agent workers tunnels and deliberately left the NodeType skip in
HealthMonitor.tunnelDeparted, with a spec asserting that an agent node whose
presence reader answers PresenceGone is NOT marked unhealthy. That spec was
scaffolding. It was true while an agent worker took its jobs and its verbs over
the message bus: a departure row for one said nothing about whether it could
work, and an early bug in the new tunnel client could otherwise have demoted a
fleet of healthy agent workers.

There is no bus. An agent worker is reachable through its tunnel and through
nothing else, so a departed agent tunnel means exactly what a departed backend
tunnel means: no live replica holds it, the departure has outlived the reconnect
grace, and that is a routing fact the scheduler and a reaper may act on. The
skip would now hide the only symptom an unreachable agent worker has. This is
the deliberate removal Task 4's M6 predicted, and task-4-report.md is where that
mutation already stands recorded red against the spec this commit deletes.

The skip existed at ONE site. router_liveness.go has none: its candidates come
from queries that already filter node_type = 'backend'. The two skips in
managers_distributed.go stay, because an agent worker still runs no backend
processes, so it has no backend to list and no backend op to apply.

Two node types can depart now, which is why the second half exists. Before this,
one type could depart and every per-node cache a departure left stale was
dropped from wherever its owner happened to notice, so a reader could not tell
which caches a demotion invalidated by reading the demotion path. Departure gets
ONE notification point. DepartureNotifier is edge triggered, because the monitor
runs on a ticker and a departed node stays departed; its subscribers are NAMED,
because what has to be caught is a forgotten cache and a count can say only that
one of four is missing; and NewHealthMonitor takes it as a required positional
argument, so a caller that does not pass one fails to compile.

Four caches subscribe: prefix-cache affinity in every model, probe freshness at
every address, in-flight staging operations, and the per-node breakdown of every
open gallery operation. The prefix-cache one is registered only when
prefix-cache routing is enabled, so --distributed-prefix-cache=false stays a
true no-op. The notification carries the node's name as well as its id, because
the staging tracker keys on the name and the other two key on the id, and a
subscriber should not have to read the registry from inside an eviction hook.

A departure notification is an act on absence, so it fires only on the routing
fact. A tunnel lost inside the grace, a worker that never dialled, a presence
query that failed and a stale heartbeat all announce nothing, asserted per node
type. The stale-heartbeat branch is excluded on purpose: it already marks the
node offline, which deletes its rows and runs the registry's replica-removed
hooks, so firing there too would double-evict and make the notification mean two
different things at its subscribers.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-09-27 03:05:13 +00:00
Ettore Di Giacinto 118eea6931 fix(distributed): let a worker's own refusal be evidence about its backend
A worker that refuses a stream has answered, and cluster.Dial keeps the three
tunnelproto sentinels out of the ErrNoRoute umbrella precisely so a consumer
can act on that. No consumer did. Since workers stopped listening, a backend
process that crashed on a healthy worker is no longer a dead listener's
codes.Unavailable: the worker refuses the stream with
ErrStreamTargetUnavailable, gRPC flattens it into Unavailable anyway, and
nodes.unroutable reported the whole thing as "this frontend has no route".
Every reap path then answered ProbeUnknown and left the row, so the replica
slot never freed and at the default MaxReplicasPerModel=1 the only cleanup
left was LRU eviction of models that were working.

isWorkerAnswer is exported as cluster.IsWorkerAnswer, so the errors the dialer
keeps out of the umbrella are by construction the errors the consumers treat
as the worker answering. nodes.unroutable and pkg/model's transportFailure
both use it; ConnectionEvictingClient, the site reached during inference, goes
through transportFailure rather than asking the transport directly. A reply
code this frontend does not recognise is still not an answer, so a newer
worker's vocabulary costs a retry and not a replica.

The reap guards keep the allow-list rather than requiring ErrNoRoute: an
unrecognised dial error must mean "no route", never "the backend is gone".

Also in this final pass over the branch:

- Docs: recommend upgrading FRONTENDS first, with the symptom of each order.
  Workers-first fails now that a 4xx registration is a verdict rather than an
  outage, so an old frontend's "address is required for backend workers" makes
  each restarted worker exit and drains the fleet a node per restart.
- Docs: LOCALAI_WORKER_TUNNEL=false is a fatal startup error, not a degraded
  mode, in both places that described it; and a frontend rollback needs every
  worker restarted, because re-registration force-clears the address columns.
- A replica with no advertised address now says so every five minutes and
  names the workers only it can reach, instead of one startup warning for a
  cost paid for the life of the process.
- callerRanOut's rule now holds at all three siblings, so an expired caller
  deadline stops reading as a broken tunnel; probeHealth's withdrawn reason
  for using the raw client is corrected; the dead DoOrCached is deleted and
  its coverage kept on DoOrCachedResult; sweepLeakedInFlight enumerates the
  outcomes that reach it.
- The peer route's self-declared id is recorded as a phase-3 deferral, in the
  handler, in the isolation claim it narrows, and in the operator docs.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-09-27 03:05:12 +00:00
Ettore Di Giacinto 4945e44595 fix(cluster): make "no route" a condition of its own, and let it out of the package
Review round 1 on task 6. Five blocking findings, all with the same root: the
conditions the dialer kept apart were erased one layer out, because every one of
them arrived at core/services/nodes as a gRPC codes.Unavailable, which is also
what a backend process that died produces. Four call sites acted on that by
deleting a replica row, one of them after a single failed probe.

The fifth condition is ErrNoRoute: this replica could not get a request to a
worker's backend, and no claim at all about the worker. A worker's presence is
its HEARTBEAT, which nodes owns; a route is a separate fact that cluster owns,
and the two now differ. They differ in normal operation, not exotically: a
worker that has not dialled its tunnel yet after a frontend-first upgrade is
unroutable on every request while it heartbeats and serves.

Two properties, both mutation-tested. Every failure to resolve or open a route
carries ErrNoRoute, so a consumer has one check to make. No failure carries an
absence sentinel: routeFailure is the single place that rule lives, and it keeps
ErrNoConnection and ErrInstanceNotFound in the message and out of the unwrap
chain, the guarantee unreachableError already made for peers. Everything else
stays matchable, so ErrNotOwner and ErrPeerUnreachable are unchanged for anyone
who can act on them. A worker's own refusal carries no umbrella, because a
worker that answers has demonstrated it is there and that is the only real
evidence on the path.

Crossing the boundary needed a value, not a code. NewClientWithDialer wraps the
dialer and records each outcome; LastDialError hands it back behind a narrow
interface, and nodes.unroutable turns it into ErrWorkerUnroutable with the
cluster sentinels still in the chain. A spec asserts a dial failing with
ErrNoRoute plus ErrPeerUnreachable arrives matching all three and matching
neither absence sentinel.

The sweep found a fourth site the review had not named: pkg/model checkIsLoaded
evicts a remote model on a connection error, and a tunnel dial failure is one.
Four other reap sites were cleared with reasons - inflight and the worker
authoritative pass reap only on semantic answers, scale-down is driven by
last_used, abandoned loads decide on the node's heartbeat. Every fixed site also
grew the opposite spec, so the new check cannot pass by never reaping.

probeCache carries the reason through singleflight rather than a closed-over
variable. A variable is only written by the goroutine that runs the probe, so
the leader would correctly decline to reap while every joiner reaped on the
leader's own observation; a mutation reproduces exactly that.

The docs sentence promising LOCALAI_WORKER_TUNNEL=false restores direct dialling
is gone. There is no such path, so it said the operator could take a worker dark
and call it a rollback. Replaced with the upgrade order that is actually safe.

The deadline spec the reviewer found vacuous now waits on the dial context's own
Done channel before touching the stream, so the armed deadline has really
expired; the mutation that survived for the reviewer reddens it.

Nine mutations, each reddening a named spec, including both halves of
isAbsenceClaim independently.

Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
2026-09-27 03:05:12 +00:00
LocalAI [bot]andEttore Di Giacinto 8bbe89a537 fix(distributed): route per request across loaded replicas + cache probeHealth (#9968)
* refactor(distributed): extract PickBestReplica from FindAndLockNodeWithModel

Lifts the replica-selection policy (in_flight ASC, last_used ASC,
available_vram DESC) out of the SQL ORDER BY into a pure Go function in
the new replicapicker.go. The SQL clause keeps its FOR UPDATE atomicity
and remains the production path used by SmartRouter; PickBestReplica is
the canonical implementation that the future per-frontend rotating
replica cache (TODO referenced from pkg/model) will call against an
in-memory snapshot without paying a DB round-trip per inference.

A new registry_test mirror spec seeds a multi-tier scenario and asserts
both layers pick the same replica, so any future tweak to either side
fails the test until the other side is updated.

No behavior change.

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

* fix(distributed): route per inference request and cache probeHealth

Two related fixes that together restore load balancing across loaded
replicas of the same model.

1. ModelLoader.Load and LoadModel bypass the local *Model cache when
   modelRouter is set. The cached *Model wraps an InFlightTrackingClient
   bound to a single (nodeID, replicaIndex) — reusing it pinned every
   subsequent request to whichever node won the very first pick, so
   FindAndLockNodeWithModel's round-robin never got a chance to run
   even after the reconciler scaled the model out to a second node. In
   distributed mode SmartRouter.Route now runs per request, and
   PickBestReplica picks the least-loaded replica each time.

   SmartRouter has its own coalescing (advisory DB lock for first-time
   loads + singleflight on backend.install RPC) so concurrent first
   requests for a not-yet-loaded model still produce a single worker
   side install.

2. SmartRouter.probeHealth memoizes successful gRPC HealthCheck results
   in a new probeCache (probe_cache.go) with a 30s TTL. With per-request
   routing every inference call hits probeHealth, and llama.cpp-style
   backends serialize HealthCheck behind active Predict — so a burst of
   incoming requests stalled on the probe to a node already mid-stream,
   tripping the 2s timeout and falling through to the install path.
   singleflight collapses N concurrent first-time probes for the same
   (node, addr) into one round-trip, failed probes invalidate the entry
   so the staleness-recovery path still triggers, and the TTL matches
   pkg/model/model.go's healthCheckTTL so the single-process and
   distributed paths share a staleness budget. The background
   HealthMonitor still reaps actually-dead backends within ~45s.

The bypass introduces one short FindAndLockNodeWithModel transaction per
inference. A TODO in pkg/model/loader.go documents the future per modelID
rotating-replica cache that would reuse PickBestReplica against an
in-memory snapshot and skip the DB round-trip for hot paths.

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

---------

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