mirror of
https://github.com/mudler/LocalAI.git
synced 2026-09-12 22:33:54 -04:00
Ettore Di Giacinto
a8ac2af167
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>