mirror of
https://github.com/mudler/LocalAI.git
synced 2026-09-15 15:52:31 -04:00
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>
114 lines
4.1 KiB
Go
114 lines
4.1 KiB
Go
package nodes
|
|
|
|
import (
|
|
"sync"
|
|
"time"
|
|
|
|
"golang.org/x/sync/singleflight"
|
|
)
|
|
|
|
// probeCache memoizes recent successful gRPC HealthCheck results for
|
|
// (nodeID, addr) tuples so SmartRouter.probeHealth doesn't pay a round-trip
|
|
// on every inference request.
|
|
//
|
|
// Why this exists: with per-request routing (see pkg/model/loader.go), every
|
|
// inference call goes through SmartRouter.Route, which probes the backend
|
|
// before returning a client. Many gRPC backends (notably llama.cpp's server)
|
|
// serialize HealthCheck against active Predict on a shared goroutine, so a
|
|
// burst of new requests can stall behind a single long-running stream —
|
|
// exactly the "queue stalling" symptom observed in distributed clusters.
|
|
//
|
|
// The background HealthMonitor (perModelHealthCheck) is still the cluster-wide
|
|
// source of truth that reaps actually-dead backends within ~45s; this cache
|
|
// only saves the per-request hot path from re-asking when nothing has changed.
|
|
//
|
|
// TTL matches healthCheckTTL in pkg/model/model.go so the single-process
|
|
// IsRecentlyHealthy path and this distributed-mode path share the same
|
|
// staleness budget.
|
|
type probeCache struct {
|
|
ttl time.Duration
|
|
mu sync.Mutex
|
|
seen map[string]time.Time // key → last successful probe
|
|
flight singleflight.Group // coalesces concurrent probes for the same key
|
|
}
|
|
|
|
// newProbeCache returns a probeCache with the given TTL. Zero TTL disables
|
|
// caching: every call to DoOrCached invokes the probe.
|
|
func newProbeCache(ttl time.Duration) *probeCache {
|
|
return &probeCache{
|
|
ttl: ttl,
|
|
seen: make(map[string]time.Time),
|
|
}
|
|
}
|
|
|
|
// IsFresh reports whether key was successfully probed within TTL.
|
|
func (c *probeCache) IsFresh(key string) bool {
|
|
if c.ttl <= 0 {
|
|
return false
|
|
}
|
|
c.mu.Lock()
|
|
defer c.mu.Unlock()
|
|
last, ok := c.seen[key]
|
|
return ok && time.Since(last) < c.ttl
|
|
}
|
|
|
|
// markFresh records key as successfully probed at the current time.
|
|
func (c *probeCache) markFresh(key string) {
|
|
c.mu.Lock()
|
|
defer c.mu.Unlock()
|
|
c.seen[key] = time.Now()
|
|
}
|
|
|
|
// Invalidate drops any cached freshness for key. Used after a probe failure
|
|
// (or any other signal that the backend may not be alive) so the next call
|
|
// will re-probe instead of trusting stale state.
|
|
func (c *probeCache) Invalidate(key string) {
|
|
c.mu.Lock()
|
|
defer c.mu.Unlock()
|
|
delete(c.seen, key)
|
|
}
|
|
|
|
// DoOrCached returns true if key is fresh; otherwise it runs probe (coalescing
|
|
// concurrent callers via singleflight) and caches a successful result. Failed
|
|
// probes invalidate the cache, so a transient miss doesn't pin every
|
|
// subsequent request to a re-probe.
|
|
func (c *probeCache) DoOrCached(key string, probe func() bool) bool {
|
|
alive, _ := c.DoOrCachedResult(key, func() (bool, error) { return probe(), nil })
|
|
return alive
|
|
}
|
|
|
|
// DoOrCachedResult is DoOrCached with a second result: the reason the probe
|
|
// never reached the backend, or nil when it did.
|
|
//
|
|
// The second result travels through the SINGLEFLIGHT, which is the whole reason
|
|
// it is not simply a variable the caller closes over. A closed-over variable is
|
|
// only written by the goroutine that actually runs the probe; every other
|
|
// caller coalesced into that flight adopts the leader's boolean and sees its own
|
|
// unset variable, so the leader would correctly decline to reap while its
|
|
// joiners reaped on the very same observation. Carrying it in singleflight's
|
|
// error slot hands every joiner the leader's reason as well as its answer.
|
|
//
|
|
// A probe that could not reach the backend is NOT cached either way. Caching it
|
|
// as fresh would hide a genuinely dead backend behind a network blip, and
|
|
// caching it as a failure is what Invalidate already does.
|
|
func (c *probeCache) DoOrCachedResult(key string, probe func() (bool, error)) (bool, error) {
|
|
if c.IsFresh(key) {
|
|
return true, nil
|
|
}
|
|
v, unreached, _ := c.flight.Do(key, func() (any, error) {
|
|
// Double-check after potentially waiting: another caller in this
|
|
// flight may have just populated the cache.
|
|
if c.IsFresh(key) {
|
|
return true, nil
|
|
}
|
|
ok, unreached := probe()
|
|
if ok {
|
|
c.markFresh(key)
|
|
} else {
|
|
c.Invalidate(key)
|
|
}
|
|
return ok, unreached
|
|
})
|
|
return v.(bool), unreached
|
|
}
|