mirror of
https://github.com/mudler/LocalAI.git
synced 2026-09-16 00:00:58 -04:00
4e4597dfc2e43fc0eb560be57b64739633036a8e
176
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 localai-org-maint-bot
|
8403d9da05 |
Merge master into test/distributed-e2e-ci
Preserve heartbeat checkpoints and backend readiness across the tunnel transport changes. Update incoming tests for the renamed worker address fields and health monitor arguments. Assisted-by: Codex:gpt-6 |
||
|
Ettore Di Giacinto
|
4b9cd31dd3 |
fix(worker): check a gRPC port is free before handing it out
The backend port allocator allocated from its own bookkeeping alone. That bookkeeping records what this worker did with a port, and the collision it cannot see is with something this worker never did: the default base port is 50051, inside Linux's default ephemeral range of 32768 to 60999, so the kernel hands ports in this range to outbound connections and to anything that binds port 0. A backend handed one of those dies on bind, and the frontend sees a backend that will not start. Every candidate is now probed by binding the exact address the backend will listen on, in all four allocation branches: the key's own port, the free pool, a grown port and a stolen one. Probing the free pool matters as much as probing a grown port, because a port this worker released is exactly as available to the kernel as one it never used. A candidate that fails the probe is quarantined rather than blacklisted, since whatever holds it is usually an ephemeral connection that gives it back, and its affinity claim is dropped so an unbindable port does not stay reserved for the key that last held it. Exhaustion now says how many candidates were skipped, which is what tells an operator "something else is in my range" from "my range is too narrow". This does not remove the race and cannot: between the probe and the child's bind the kernel can still give the port away. It removes the far larger window in which the allocator hands out a port the kernel gave away minutes ago, which was the whole of the observed one-in-three harness flake. The e2e harness comment that recorded the missing check is corrected, and the docs say how to move the range out of the ephemeral one entirely. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
a1b5c177bc |
feat(cluster): make a peer prove which replica it is
GET /api/cluster/peer authenticated with the deployment's shared registration token and took the dialling replica's id from ?id= on trust. Every worker holds that token, so anything holding it could open a peer link as any replica: relay through it to every worker tunnel that replica owns, displace a real replica's inbound link by declaring its id, and point the roughly 31 GiB per-session receive window at one replica. Validating the id against the instances table does not fix this, because the attack declares a real replica's id. So the route now checks two credentials and needs both. The shared token still says the dialler belongs to this deployment; a new per-replica credential says which replica it is. The credential follows the per-node worker credential rather than inventing a second mechanism: crypto/rand.Text, stored only as a hex SHA-256, compared in constant time, with no fallback to the shared token. It differs in the stronger direction. A worker's credential is minted by the frontend and handed over once; a replica writes its own instances row, so it mints its own secret, publishes only the hash in the same statement that publishes its address, and never sends the plaintext anywhere but the peer dial. A peer that presents no credential is refused, not waved through. An old replica and an attacker holding the shared token send the same request, so accepting the first accepts the second; there is no safe downgrade here, only a quiet one. The refusal is made loud instead, on both sides, naming the upgrade rather than the network. On the documented frontend-first order a new replica still dials an old one; an old replica cannot dial a new one, which costs relayed requests that land on a not-yet-restarted replica and surfaces as no route, never as absence. A rejected peer gets its own sentinel, ErrPeerRejected, whose unwrap chain carries ErrPeerUnreachable as well and no absence sentinel at all. Keeping the older sentinel means no existing consumer changes behaviour; the cause stays out of the chain, so absence cannot escape through it and nothing can read an authorization failure as a worker that went away. One consequence beyond the fix: a replica with no advertised address has no instances row, so it now cannot dial out either. It was already unreachable inward. The startup error and the docs say so. Registry.Register, NewMembership, NewPeerPool, PeerHandler and RegisterClusterRoutes all gained required arguments, so the identity cannot be dropped without a compile failure. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
348b0860dc |
test(distributed): prove the fan-out carrier between two real replicas
Removing the broker left one thing carrying every broadcast family in the product: PostgreSQL LISTEN/NOTIFY, in core/services/pgbus. It is covered thoroughly in process by test-e2e-distributed, and it was covered nowhere at all by real binaries: grepping the six Cluster spec files for pgbus, bus_messages, LISTEN and NOTIFY returned zero hits. Registration, model staging over the tunnel and inference through both the owner and the relay paths were already proven by real processes; the carrier that now carries everything else was not, so a deployment whose replicas each published to themselves and heard nobody would have left every suite green. Two specs, both on two frontends and no workers against one PostgreSQL, publishing at frontend 0 and reading at frontend 1. 1. A gallery operation admitted at one replica, read out of the other, with the queued state observed before the terminal one. 2. A broadcast of about 9.3 kilobytes, which PostgreSQL refuses as a notification payload, making the round trip byte for byte through the bus_messages spill table. The family is a gallery operation for one property nothing else on this carrier has: the answer a peer gives is held in memory ALONE. GET /models/jobs/<id> reads galleryop's statuses map, which on a peer is filled by the gallery.*.progress subscriber and by nothing else, because the only other filler, Hydrate, runs once at startup and every operation here is created long afterwards. Every other family has a durable table behind it that a peer would converge through anyway, and a spec on one of those cannot separate "the broadcast arrived" from "the row was read". That is then made checkable rather than argued. The gallery_operations row is written when the gallery worker DEQUEUES an operation, so an operation still waiting in the queue has NO row, and both specs assert zero rows while the peer is already answering with the operation's own bytes. Both also read the instances table and require the reading replica to be a different live instance from the publishing one, so "the other replica" cannot decay into a spelling of "this replica". Holding the queue is what cluster.Options.Galleries is for. The gallery worker runs one operation at a time on an unbuffered channel, so an install parked inside a gated index fetch parks everything behind it; without that the admission broadcast and the terminal one are separated by two database round trips and no HTTP poller could see between them. The option also turns the startup estimate warmer off, because a second fetcher filling the process-wide index cache would leave the operation never blocking and the spec passing on an ordering nothing enforced. The spill spec is written against a failure this branch has shipped three times: a size-limit spec that cannot fail. The oversized body is an ordinary element name that the real consumer decodes and surfaces, so it is not a body the decoder would have refused at any size. The size is ABSOLUTE at 9000 bytes rather than derived from the cap, and a one-byte control operation in the same run is required to leave no spill row, so moving the 8000-byte cap in either direction reddens the spec. pgbus.FitsInline, which shares its encoder and its comparison with Publish, is asked about both payloads and must answer differently. The spilled row is then decoded and its element name compared byte for byte against what frontend 1 answers. The terminal assertion in spec 1 does not re-check the element name: a terminal status does not carry one, because updateError in galleryop.Start builds a fresh OpStatus holding only the error. It asserts the two fields that status does carry, in the relation that one place writes them. Attacks run, each alone, each reverted, each behaving as predicted. Neutering the pg_notify in pgbus.Publish so every replica knows only what it did itself reddens both specs at frontend 1, which answers 500 for an operation it was never told about; the bus_messages row assertion still passes under it, which is right, since the row is written before the notification. Releasing the queue gate reddens spec 1 at the gallery_operations count, because the operation is dequeued and the row appears. Shrinking the oversized name to 100 bytes reddens spec 2 at FitsInline; inverting that guard so the run reaches the row check reddens it there instead, with no bus_messages row written, which is what makes the row a statement about size. test-e2e-cluster is 26 specs in 933.8 seconds of Ginkgo time, 15m37s wall. The two additions cost 7.0 seconds together, 5.0s and 2.0s: they start no workers, so they pay for no registration, and what they wait on is a broadcast rather than a threshold. test-e2e-distributed is unchanged at 223 plus 8 specs, 130.6 seconds. The budget comment and .agents/building-and-testing.md move from 24 specs at 897 to 907 seconds to 26 at 933.8. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
afe7741773 |
test(distributed): prove the busless cluster on two frontends and two workers
Tasks 1 to 17 are proven by unit and integration specs and by two e2e passes taken mid-flight. This is the pass that boots the real binaries with every carrier in place and none of the old one, and it does so on the topology the feature was built for rather than on the one-worker shape the rest of the cluster suite uses. Two frontends and two workers is the configuration that matters. With each worker's tunnel landing on a different replica, the owner path and the relay path are live at the same instant against one roster, one scheduler and one health monitor, so a routing mistake has somewhere to show up instead of hiding. It is also the only shape in which "killing a replica re-homes only ITS worker" can be stated at all. Three scenarios, all 2x2: 1. Both workers served from both replicas. No broker as a property of the ARTIFACT (debug/buildinfo reports no github.com/nats-io module, with the module count asserted non-zero so a stripped binary cannot pass vacuously), no broker in either worker's live /proc environment, and no advertised address on either worker. One completion over the owner path and one over the relay, plus the mirror image through the other replica, plus four control-plane listings covering both paths for both workers. 2. The replica owning worker 0's tunnel is killed with that tunnel blocked. Leg 1 asserts nothing and only waits for the killed instance to leave the live set, because before that it still reads as a live owner and the scenario is not yet about absence. Leg 2 then holds a window inside the reconnect grace requiring that nothing acted on the absence. Leg 3 requires the re-home and inference again. Worker 1 keeps serving throughout. 3. The suite-wide negative control. Both tunnel dials refused while registration and heartbeats flow, both workers refused at both replicas naming the routing fact and not a departure, nothing reaped and both heartbeats fresh. Then ONE tunnel is restored and exactly one worker recovers while the other stays refused. Which worker served is read back from node_models rather than assumed: the two models are pinned to one worker each through PUT /api/nodes/:id/labels and POST /api/nodes/scheduling, and every assertion requires the model to be on the expected node AND absent from the other. That the relay hit a non-owner is read from the production Owner query before the request and re-read after it. Attacks run, each alone, each reverted, each behaving as predicted: hand a worker a broker URL reddens scenario 1's environment leg; point the module check at gorm.io reddens its artifact leg; start one worker instead of two reddens all three at the topology guard; delete the relay in WorkerDialer.Dial reddens scenario 1 on exactly the request sent to the non-owner while 2 and 3 stay green; a one-nanosecond reconnect grace reddens scenario 2's leg 2 on the demotion while 1 and 3 stay green; lifting both blocks at scenario 3's differential reddens its "still unreachable" half. The brief's "restore the NatsURL validation" attack cannot be applied: DistributedConfig has no such field left to validate. Label-orphan arithmetic, counting non-skipped It nodes from --dry-run: all 256, dist 231, cluster 24, vllm 1, and 231 + 24 + 1 = 256, so no spec is orphaned by the label filters. Three test-e2e-cluster runs: 897.0s, 897.8s and 906.6s of Ginkgo time, 24 specs, 15 minutes wall. The only failure across the three was a pre-existing spec dying at cluster.Start with frontend-1 exiting status 2, which passed in the other two and is reported as a port-allocation flake rather than a regression. test-e2e-distributed is 223 plus 8 specs in 131.7s. The budget comment and .agents/building-and-testing.md move from 21 specs at 800 to 830 seconds to 24 specs at 897 to 907. The harness gains ProcessEnviron, which reads /proc for any of the three process families; WorkerEnviron and FrontendEnviron become wrappers rather than being deleted, so the specs that call them are not re-aimed for a rename. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude Opus 5 [claude-code] |
||
|
Ettore Di Giacinto
|
730d259976 |
chore(distributed): take the nats-io modules out of the build
Distributed mode has not dialled a message broker since the control plane moved onto the workers' own outward tunnels and every fan-out family moved onto PostgreSQL LISTEN/NOTIFY. What was left was the dependency itself, and the code that existed only to feed it. Dropped from go.mod: nats-io/jwt/v2, nats-io/nats.go, nats-io/nkeys, nats-io/nuid and testcontainers-go/modules/nats, along with the fourteen indirect requires that only the NATS testcontainer pulled in. go.sum carries no nats line either, so the removal is not the partial kind where the require goes and the checksum stays. Deleted with them: pkg/natsauth in full, the broker client's remaining options and TLS files, the per-node JWT minting on both the register and the approve path, and the natsauth.Config parameter threaded through the node routes. The credential manager is renamed and stripped rather than deleted, because it still holds the tunnel token that every re-registration rotates. The bus flags stay accepted and ignored, and are now hidden, on every command that had them, so an existing unit file, compose file or Helm values file still starts on the day of the upgrade. What is not kept is the validation that REQUIRED one: a distributed frontend started with no bus URL is no longer fatal. The TLS paths lose type:"existingfile" deliberately, so a certificate deleted along with the broker cannot fail a startup. One operator-visible behaviour change: --nats-require-auth no longer makes an agent worker wait through admin approval. Ask for that wait with --distributed-require-auth, which already implied it. It is documented in the migration section and pinned from both sides. A deployment now needs PostgreSQL and the frontends' own HTTP listener, and nothing else. coverage-baseline.txt moves from 54.2 to 62.0. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
d3dfad90b9 |
chore(distributed): stop telling an operator to run a NATS cluster
Every carrier had already moved and no process opened a bus connection, but the surface an operator reads still described a deployment with a broker in it: a compose service, a 220-line credential-generation script, two CI steps pulling a container nothing started, two flag tables offering --nats-url, an architecture diagram with a NATS box wired to the workers, a join-command generator in the Nodes page that emitted --nats-url for agent workers, and a test suite that stood a NATS server up for specs that no longer used it. That is the one way this programme could still fail invisibly. Every test passes, every binary works, and every production deployment goes on running and paying for infrastructure that carries nothing. Nothing in this repository starts a NATS server any more. The compose file is four services, the docs say to shut the broker down and what to keep, and the e2e suite runs on one PostgreSQL container. The three LOCALAI_NATS_*_TIMEOUT env vars are KEPT, and are now documented twice as being kept. They were never broker settings: each names a control-RPC budget the frontend applies to a worker, still read and still enforced. They carry the prefix only because they arrived with the bus, and renaming them would break every existing deployment for cosmetics. The agent worker's join command was the last surface still emitting the flag, two tasks after the agent worker stopped dialling. The Playwright spec that covered it asserted the opposite of what is now true, so it is inverted rather than deleted, and it reads the rendered command string rather than the component's variables: the variables are what the fix removes, so a spec reading them would have stopped compiling instead of failing, and a compile error is not evidence about what an operator is shown. nats_jwt_test.go and its helpers are deleted. They pinned a real server ENFORCING the minted permissions. The CONTENT of those allow lists is still pinned, untouched, by pkg/natsauth's own suites, including the spec that refuses to let the agent lists go empty, since an empty allow list in NATS means unrestricted. The enforcement half is retired rather than moved: enforcement is a property of a connection, and nothing opens one. The suite's own NATS container goes with them, which the brief left for the next task. Removing the pre-pull while BeforeSuite still ran the image would have defeated the step rather than cleaned it up, and this change removes the last reader of TestInfra.NC. agent_native_executor_test.go and mcp_ci_job_test.go are moved onto infra.Bus() instead of deleted: they were the last two specs building a bridge and a dispatcher on a client nobody uses, which is exactly the drift TestInfra.Bus's own comment warns about. cluster.Options.NatsURL is now fed a deliberately dead address rather than a live container's. Frontends and agent workers still receive LOCALAI_NATS_URL, because that is the coverage for the promise that an existing command line still starts; sourcing it from a running server would have let a regression that actually dialled it pass. The control in cluster_control_test.go keeps its assertion and loses its explanation, which claimed the deployment had a bus and no longer could. One latent spec race surfaced and is fixed: the background-run spec waited for a COUNT of events and then read a snapshot for the terminal status, which is the last event of a run and therefore always arrives after the count is met. Its immediate twin had already been fixed this way. Nothing in production changed. pkg/natsauth keeps its files. It is reachable from production only through the natsauth.Config parameter thread, and that thread is the next task's. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
a6b2d7c0ec |
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> |
||
|
Ettore Di Giacinto
|
3b4858851a |
feat(distributed): carry an agent cancel on the worker's own tunnel
agent.<name>.cancel was the last family on a message bus, and the only reason an agent worker dialled one. Its subscriber is the worker running the execution, and a worker has no database, so the family could not move to the PostgreSQL fan-out carrier: a cancel published there would reach no worker while reporting that it had been sent. It is a control verb now. An agent worker mounts workerctl.PathAgentCancel on the loopback control plane behind its tunnel and applies the cancel to the same registry the executor registers a run on. The frontend issues it through nodes.AgentControlClient.CancelAgentRun. That call is a FAN-OUT and not a pick, because nothing records which worker holds a given execution: the claim row names the claiming replica, and it is deleted when the run ends. Every agent worker a live replica can reach is asked over its own tunnel, relayed by the peer mesh when a peer holds it, and each worker answers only for itself. The answers stay apart, which is why this family was held back. A cancel a worker made is nil. A cancel some worker could not be asked is ErrAgentCancelUndelivered, which is neither a refusal nor a missing run. A cancel every reachable worker declined to own is ErrAgentRunNotOnAnyWorker. A deployment with no agent worker is ErrNoAgentWorker. Neither new sentinel wraps ErrWorkerUnroutable and neither is a worker answer, so nothing is reaped, demoted or evicted because of a cancel. A worker in the ABSENT CONNECTION condition, one whose tunnel was lost inside the reconnect grace, counts as undelivered. It is not retried in the call and not queued: a retry would spend a budget the caller did not choose, and a queue would need durable state whose only consumer is a run whose control stream went with the tunnel. A worker whose departure has outlived the grace is the one routing fact a caller may act on and is excluded, or a single retired agent node would make every cancel undelivered for ever. The fan-out reads a different node set from the pick. A draining worker takes no new work but is still finishing what it holds, so it is offered the cancel; a pending one is refused by the tunnel route on every dial and is not. With that, nothing in LocalAI connects to NATS. The agent worker's dial, its credential ladder and its refresh loop are gone, and so is the frontend's cancel carrier. LOCALAI_NATS_URL is accepted and ignored everywhere, and distributed mode no longer requires it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
b45076c5f7 |
refactor(distributed): delete MessagingClient and shrink the NATS client to fan-out
Nothing in the tree publishes, subscribes, queue-subscribes or requests through
the MessagingClient interface any more, so it is deleted rather than shrunk to
Broadcaster: two exported names for one method set in one package is an
invitation for the next author to pick whichever the surrounding file already
imported.
$ grep -rn 'messaging\.MessagingClient' --include='*.go' .
core/services/syncstate/syncstate.go:54: // It is messaging.Broadcaster rather than messaging.MessagingClient because
(one hit, a comment; no live referent. The naive grep in the plan also matches
prose and the local test type names fakeMessagingClient and
countingMessagingClient, so it can never be empty.)
*messaging.Client is shrunk to exactly Broadcaster plus its own lifecycle.
QueueSubscribe, QueueSubscribeReply, SubscribeReply, Request, Conn and the
package helpers QueueSubscribeJSON and RequestJSON go with it; none had a
production caller. Deleting the methods rather than only the call sites is what
makes putting a family back on this carrier a build error instead of a line that
compiles, publishes successfully, and is delivered onto a carrier the deployment
is being taken off. Conn is in that list because while it existed every other
name was one c.Conn().X() away; the flush-and-verdict that its real consumers
needed is now ConfirmRoundTrip, which keeps the NATS JWT permission specs armed.
The client, its options and its TLS plumbing are NOT deleted, and both processes
stay on the bus. agent.<name>.cancel is the one fan-out family that could not
move: its only subscriber is the agent worker, which has no database and cannot
join the PostgreSQL carrier at all, so a cancel published there would reach no
worker and be reported as sent. The frontend passes the client to
newFanoutBridges as its cancelCarrier and the worker subscribes on it, so
--nats-url stays required on agent-worker. Both go with the tunnel cancel verb.
The struct field is renamed Nats -> CancelCarrier to say what it is for, and
agentpool loses the messaging.Publisher it held only to be non-nil: it never
published on it, and it was gating whether a frontend runs agents distributed or
in an in-process pool. Retiring the bus would have flipped every replica back to
the in-process pool silently. The gate now reads the agent store, which is the
dependency the mode actually requires.
Also deletes four subject builders with no production publisher
(SubjectFineTuneProgress, SubjectFineTuneCancel, SubjectCacheInvalidateSkills,
SubjectCacheInvalidateCollection), the queue and request/reply halves of the
shared test double, and the e2e specs that were their only callers. Every
surviving subject is now pinned to its exact literal, because a subject is a
cross-version wire format and a rename that looks internal stops half a fleet
hearing the other half.
Docs: distributed-mode.md and cli-reference.md no longer claim NATS carries the
agent-worker job subjects, the frontend's cross-replica events, or an agent
worker's real work.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
|
||
|
Ettore Di Giacinto
|
44cb495169 |
feat(distributed): move the last nine fan-out families onto PostgreSQL
Gallery progress and cancel, the operation cache's start and end, the model
and backend cache invalidations, staging progress, and the prefix cache's
observations and invalidations now travel on the LISTEN/NOTIFY carrier. No
subject is published or subscribed on messaging.Client anywhere in the tree,
which is what makes retiring that package a deletion rather than a migration:
$ grep -rn 'natsClient\.Publish\|nats\.Publish\|\.Nats\.Publish\|QueueSubscribe\|SubscribeReply\|\.Request(' \
--include='*.go' core/ pkg/ | grep -v _test \
| grep -v 'c\.Request()\|ctx\.Request()\|Request()\.Context' \
| grep -v 'core/services/testutil/fakebus.go'
core/services/messaging/client.go:168,170,172,227,234,236,250,252,254,268,269,287
core/services/messaging/interfaces.go:21,22,23
Every remaining hit is inside core/services/messaging itself. The production
reads of the NATS client are now three, all of them the documented agent-worker
exception: Close on shutdown, the agent pool's publisher, and the agent-cancel
carrier passed to newFanoutBridges.
Prefix-cache observations publish like every other family rather than through a
method that refuses a message too large for a notification. The plan proposed
such a refusal on the reasoning that a long prompt makes a chain of thousands of
entries; ExtractChain caps a chain at Config.MaxDepth blocks, MaxDepth is a
constant with no operator knob, and the chain reaching Sync.Observe has one
source, the router's own extraction hook. A worst-case observation is a few
kilobytes against an 8000-byte cap, so the hot-path spill the refusal was
designed to avoid cannot occur, and shipping it would have added the programme's
only deliberate message drop to guard a condition that cannot arise. pgbus gains
FitsInline instead, a predicate that shares one size decision with Publish and
decides nothing, and core/application refuses at startup to wire a prefix cache
whose configured depth would put every observation over the cap.
The carrier choice is no longer stated at four sites. StagingTracker.SetPublisher
and SubscribeBroadcasts become one SetBroadcaster, so a tracker that publishes
where its peers are not listening cannot be spelled; prefixcache.Sync gains
SubscribeBroadcasts, which reads the carrier it publishes on; and the gallery
service and the operation cache are wired by methods on DistributedServices that
name no carrier at all, so the NATS client beside it cannot be handed over.
OpCache.SetMessagingClient and GalleryService.SetNATSClient are renamed to
SetBroadcaster so a missed call site fails to compile.
Two pre-existing defects that the two-real-carrier specs surfaced are fixed. A
progress tick published before a cancel and delivered after it cleared Cancelled
and left the operation reading as still running on that replica; mergeStatus now
drops a stale tick rather than merging it. GetStatus and GetAllStatus handed out
the stored OpStatus pointer while the broadcast subscribers mutated it in place,
so an /api/operations response could be marshalled mid-write; both now copy.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
|
||
|
Ettore Di Giacinto
|
5a95bb3c0a |
refactor(distributed): move job and agent fan-out onto the PostgreSQL carrier
Five of the six families whose subscriber is an open HTTP response rather than a process-lifetime cache now travel on pgbus: jobs.<id>.progress, jobs.<id>.result, jobs.<id>.cancel, agent.<name>.events.<user> and responses.<id>.cancel. Both ends of each move together, so there is no state where a publisher is on one carrier and its subscriber on the other. agent.<name>.cancel does NOT move, and the plan was wrong about why. Its only subscriber in the tree is the agent worker, which has no database and so cannot join the PostgreSQL carrier at all. Publishing that cancel on pgbus would have lost every cancel of a worker-run agent while returning nil, which reports a cancel that reached nobody as a cancel that was sent. EventBridge now names its cancel carrier separately, a frontend replica sets it to the carrier the worker reads, and it stays there until a cancel rides the worker's tunnel like every other verb addressed to a worker. The carrier drops at 256 rather than blocking, which is not safe on its own for a result: a lost result has no successor message. It is not the only path. The claiming replica persists the terminal line before it releases the claim, and an open progress stream re-reads the job row once after subscribing and then periodically, so a dropped terminal broadcast costs promptness and never the answer. Both per-request subscriptions close in a defer instead of on one return path, and pgbus grows Subscribers() so the leak they would otherwise cause can be asserted. It has no other symptom: only the first subscriber of a channel issues a LISTEN, so a leaked filter just adds one closure per notification for every stream the replica has ever served. Subscribe now issues its LISTEN before it registers, which makes that count a readiness signal rather than a figure to compare against itself. Two rules that were stated at several sites and pinned at none are now one each. The re-broadcaster is built beside the dispatcher and the bridge and handed to the dispatch loop, so no line is left that can point it at a carrier nobody subscribes to while every spec stays green. The set of statuses a job never leaves is one exported set that the SSE bridge and the store both read. The last hand-written subject filter in production code became messaging.SubjectAgentEventsWildcard. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
245010f2f6 |
feat(distributed): carry the state.*.delta families on PostgreSQL
syncstate.Config held one carrier field typed as the NATS client, so a pgbus.Bus could not be handed to a SyncedMap at all: it satisfies messaging.Broadcaster and not MessagingClient. The durable re-hydration path built for the responses map therefore had a NATS-only consumer and nothing in the build said so. The field becomes Bus messaging.Broadcaster, SubscribeJSON moves to its own file and relaxes its parameter to Broadcaster, and the four adopters fan out over PostgreSQL LISTEN/NOTIFY: fine-tune jobs, quantization jobs, agent tasks with their per-tenant children, and Open Responses metadata. A new spec proves it on a real database, over two Bus instances on two pinned listener connections: a Set and a Delete carry, a payload past the 8000-byte notification cap comes back byte identical through the spill row, two families sharing one LISTEN channel stay separate, and a terminated listener re-hydrates a row written while it was gone. The five sites that each chose a carrier for an adopter are collapsed into one DistributedServices.Broadcast() accessor. Five field reads were five chances to leave one family on NATS with nothing failing, because messaging.Client satisfies Broadcaster too. The accessor also refuses to hand out a nil pgbus.Bus wrapped in a non-nil interface, which every adopter would read as "broadcast" and dereference on the first Set. SetTaskSyncNATS and SetJobSyncNATS are renamed to SetTaskSyncBus and SetJobSyncBus so a missed wiring site fails to compile. The response metadata table gains a retention of its own, defaulting to 24 hours. It inherited the Open Responses store TTL, which defaults to 0 meaning no expiration. Zero is defensible for a map that dies with the process and is not for a table: the table grew for the life of the deployment and a restarting replica re-hydrated every response the cluster had ever created. A row that names its own expiry is still judged on that column alone, and "this row is dead" now has one SQL spelling that PurgeExpired deletes by and ListUnexpired is the negation of, so a hydrate cannot resurrect what a sweep has already retired. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
43f7a5d108 |
feat(distributed): give responses.metadata something to re-hydrate from
The responses.metadata SyncedMap had no durable Store, so its reconnect re-hydrate replaced nothing. That was survivable while responses converged through deltas on a broker that mostly stayed up. It is not survivable on a carrier whose listener is one pinned PostgreSQL session: every response created while the subscription was down stays invisible on that replica forever, and the symptom is a 404 from one replica and a 200 from another for the same response_id. State that must survive a gap now lives in a response_metadata table, and the notification only says it changed. The map writes through on a Set and reads the table on hydrate, on reconnect and on reconcile, so the gap closes instead of becoming permanent. The row carries the whole projection as JSON rather than one column per field. A column-per-field schema would be a second definition of what a peer may act on, and the two would drift the first time syncedResponse gained a field: the map would broadcast the new field and hydrate without it, so a replica that had reconnected would serve a different response body from one that had not, with nothing failing anywhere. Only PayloadJSON is ever decoded; owner_replica and owner are indexed copies for an operator reading the table by hand. A missing row and an unreachable database are different facts. Every store and adapter method returns a driver failure as an error and never as an empty result, and syncstate replaces nothing when its source errors, so an outage leaves the map holding what it had rather than blanking it into a cluster-wide 404. Liveness is the database's clock, spelled expires_at IS NULL OR expires_at > now(), because every replica hydrating from this table must agree on which rows are live and a Go-side cutoff makes that a property of whichever process asked. The test container shares the host clock, so no behavioural spec can tell the two apart; the statement shape is pinned instead. The constructor refuses a non-PostgreSQL handle, because an unguarded now() on the single-binary path reads as a missing migration. A ticker sweeps expired rows every five minutes on each replica, and Close waits for it rather than racing it. Note that the sweep removes nothing while LOCALAI_OPEN_RESPONSES_STORE_TTL is 0, which is the default: with no TTL nothing ever expires and the table grows for the life of the deployment. The docs say so plainly. EnableDistributed takes the store positionally and last, so a call site that forgets it fails to compile rather than silently restoring the deltas-only map this change exists to replace. A nil store there is refused by name: it is reached only from the distributed branch of route registration, so it is a wiring bug and not a deployment shape. What still never leaves the owning replica is unchanged: the resume buffer and the CancelFunc. The write-through is one row per response state change, not one per generated token. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
78f4ff7934 |
feat(distributed): dispatch queued work as a claim queue
The three NATS queue groups jobs.new, jobs.mcp-ci.new and agent.execute are gone. Dispatching work is now a row in a work_claims table, taken by one frontend replica with SELECT ... FOR UPDATE SKIP LOCKED and driven on an agent worker as a streaming control RPC over that worker's tunnel. Exactly-one delivery among competing consumers is a database problem, not a broker feature. An agent worker has no database, so it never claims; it executes what the claiming replica hands it. A claim must not outlive the replica that took it. The reap releases a claim whose owner is no longer a live replica in the instances table, on the database clock, and never asks how long the claim has been held. A job that legitimately runs for an hour on a heartbeating replica is left alone, while a claim whose owner stopped heartbeating becomes claimable again within one liveness window. A replica with no advertised address has no instances row at all, so it refuses to claim rather than have its work reaped out from under it mid-run. The settle rule is stated once, in settleClaim, and every exit path calls it. A transport failure releases the claim and never completes or discards it; only a decoded reply line completes it. That line is deliberately not cluster.IsWorkerAnswer, which accepts the stream refusals a worker's tunnel writes before any request body reaches its control server: completing on those would discard work that never ran. The terminal line is persisted before the claim is completed, so a store that refuses leaves the claim standing rather than leaving the job running for ever. That is the dropped-result defect fixed structurally rather than by retry. This also surfaces a pre-existing gap rather than causing one: no worker has ever served plain task jobs, and publishing them into an empty queue group left them running with no trace. Such a claim is now failed with a reason. Removes QueueWorkers, --agent-subject and --agent-queue, and narrows an agent worker's minted JWT by agent.execute and jobs.mcp-ci.new. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
e5ad2e18c4 |
test(e2e): build the cluster binary and drop a load-sensitive budget
Two things made every end-to-end result on this branch unreliable. make test-e2e-cluster did not rebuild ./local-ai. It only checked that a file by that name existed, so an edit that was never rebuilt ran the whole suite against an older build while reporting on the working tree, and a missing binary skipped every spec and exited 0 with "Test Suite Passed". The target now depends on a new e2e-binary target, which is the plain go build CI already used rather than make build, since that one also builds the React UI this suite never touches. The harness carries the other half: localAIBinary now FAILS, locally as well as under CI, when the binary is older than the newest non-test Go source in the tree, which covers a run started with LOCALAI_E2E_BINARY or by invoking ginkgo directly. Test files are excluded from that scan because they compile into the ginkgo suite and never into local-ai. The CI job drops its own build step and the env var so that one place owns the build and it happens after protogen-go. test-e2e-distributed was audited for the same hole and has none: ginkgo compiles that suite from the working tree on every run and it execs no prebuilt binary. build-mock-backend already rebuilt unconditionally. "Worker tunnel under load" bounded the worst probe inside the bulk transfer window against the worst probe under the empty-load window. A max over n samples is a biased estimator when the two n differ, and here they always do: the bulk window is by construction longer and draws several times as many chances at an unrelated scheduling outlier. Anything loading the box widens that gap, so the spec reddened on what else was running: 255ms against a 161ms budget with make lint beside it, 57ms alone. It now bounds the probe COMPLETION RATE instead, which is the statistic a serialised session actually moves and a mean over dozens of samples in both terms, so a uniform slowdown cancels in the ratio. Measured with make lint running: 0.97 direct and 0.60 relayed against a floor of 0.125. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
9d7a2457d9 |
feat(distributed): stop a backend on one route, whatever the worker is
nodes.<id>.backend.stop was the last worker-facing NATS subject, and it existed only because ONE publisher had not moved. An agent worker already mounted workerctl.PathBackendStop on the tunnel it holds, and a backend worker already took its stop there, so RemoteUnloaderAdapter branched on NodeType to pick a carrier for a verb both kinds of worker served the same way. The branch is gone, and with it nodeTypeOf and its NodeTypeBackend default, which removes one of the ten NodeType branches left to sweep. The adapter loses its messaging.MessagingClient outright rather than keeping an unused field: it now holds no publisher, so re-routing any verb back onto the bus is a change to the struct and to every caller of the constructor, and does not compile until all of them agree. messaging.SubjectNodeBackendStop and subjectNodePrefix are deleted, the agent worker's subscription with them. pkg/natsauth drops the per-node backend.stop grant from the agent SUB list. That is a narrowing of eleven entries to ten, never to nothing: NATS reads an EMPTY allow list as unrestricted, so the coverage spec asserts both that the retired subject is no longer covered and that the queue subjects an agent worker lives on still are. The e2e half proves it against a real enforcing server: one spec subscribes successfully on an agent-minted JWT, the next is refused the retired subject on a JWT minted the same way. Both halves of the old split were pinned, so both pins are re-aimed rather than deleted, and the two node types are asserted separately rather than as one parameterised case, because only two cases can show that the two used to differ. Three assertions that the adapter published nothing are deleted instead: with no publisher to hold, no change could ever redden them. The CLI's handler set moves into agentWorkerControlHandlers so a spec can stand it up and post to it. That wiring was a bare literal no spec pinned, and deleting the subscription made it the ONLY carrier for backend.stop: a dropped field would have been a 404 the frontend reads as a worker too old to serve the verb, and nothing in the repo would have noticed. Mutations: the agent branch restored off the control route reddens two specs; the backend branch restored, separately, reddens five; PathBackendDelete in place of PathBackendStop reddens nine across both node types; dropping the CLI wiring line reddens the new wiring table; re-adding the allow-list entry reddens the unit spec and the JWT e2e spec; and restoring the publisher for real does not compile. Four comments this change falsified are fixed, in core/cli, pkg/model and the distributed-mode docs, which now say both kinds of worker serve POST /v1/control/backend/stop and what each does with it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
localai-org-maint-botandEttore Di Giacinto
|
7a234473e8 |
fix(ci): unbreak the e2e build and the darwin vllm-metal pin (#11849)
Two independent breakages on master make every open pull request red, for reasons unrelated to the changes under review. The e2e backend suite stopped compiling. Reply.message is `bytes` in backend.proto, so res.GetMessage() returns []byte, and strings.ToUpper wants a string. Every other call site in the file already converts. tests/e2e-backends sits behind a build tag, so `go build ./...` never compiled it and the breakage reached master unnoticed. The darwin vllm build stopped resolving. Upstream vllm-metal deleted its old dev tags and re-versioned to track the vLLM release it targets, so the pinned wheel 404s. The coupled vLLM release also moved out of upstream's install.sh into .github/vllm-release-tag.commit, and the wheel's platform tag moved from macosx_11_0 to macosx_15_0. Read the wheel name from the release's own asset listing rather than composing it from a hardcoded platform segment, so a platform-tag change cannot silently 404 again, and resolve the vLLM version from the new metadata file with a fallback to the legacy installer. The bump script and the extractor learn the same two-source lookup, so the next nightly run converges on the pin checked in here instead of reintroducing the break. Assisted-by: Claude:claude-opus-5 Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
1a8384a8e1 |
test(distributed): name the replica endpoint the router now requires
Two specs in the distributed e2e suite have been red since |
||
|
Ettore Di Giacinto
|
5effa47527 |
feat(distributed): make MCP execution and discovery a selection
mcp.tools.execute and mcp.discovery were the only NATS subjects that combined a queue group with a reply, and no carrier in this design provides both. They never needed one: a queue group is a way of choosing a subscriber, and choosing is a query. The frontend now lists the approved, non-draining agent nodes, asks the node_connections table in one joined statement which of those tunnels a live replica holds, prefers one this replica holds so the call skips the relay hop, and issues an ordinary control RPC on the path task 4 already mounted. A peer-held tunnel is reached through the relay. That is a choice a broker's hidden balancing could not make. The selection reads presence and nothing else. It is filtered only on node type and on the two statuses an operator controls, never on a health verdict written on another clock, because refusing a worker that is connected and answering is the same defect as picking one that is gone. An empty fleet answers ErrNoAgentWorker, which is deliberately neither ErrWorkerUnroutable nor anything cluster.IsWorkerAnswer accepts: nothing was asked of any worker, so no reap guard may act on it. A reply carrying an Error is the worker's own answer and is returned unchanged; it is never offered to a second worker, which would turn "this MCP server rejected your arguments" into "the fleet is broken" and could run a tool twice. A call that never reached a worker is retried against a different pick, at most three times, and whatever error is finally returned is returned unwrapped so its identity survives the loop. MCP prompts and resources now answer 501 in distributed mode instead of an empty 200. They are served only from sessions the frontend holds, and in distributed mode it holds none. That gap predates the removal of the bus and is not closed by it; this only stops it being silent. Agent workers keep every other subject, including nodes.<id>.backend.stop. Their minted JWT loses the two MCP subjects and keeps a non-empty allow list, because NATS reads an empty one as no restriction at all. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
64059cd7d7 |
feat(distributed): give agent workers a tunnel of their own
Phase 2 gated agent nodes out of tunnel credentials at the mint site. That was right while nothing dialled into an agent worker: a credential would have replaced nothing, and the gate was structural rather than a second check that could drift. It is wrong now that the frontend needs to reach an agent worker by RPC. attachTunnelToken mints for backend and agent nodes and CLEARS for anything else, through one tunnelEligible predicate rather than two conditions that can be widened separately. ConnectHandler still never reads NodeType, so an empty hash is still what refuses an ineligible node. An agent worker now starts a loopback control server behind the same bearer check a backend worker uses, and holds one tunnel whose only stream tag is http: it runs no backend processes, so the grpc tag has nothing to route to and is not offered. Its MCP tool, MCP discovery and backend.stop verbs are served from ONE implementation reached by both the bus and the tunnel, so a frontend cannot get different bytes depending on which carrier delivered. The tunnel is an ADDITION. --nats-url is still required, and agent jobs, MCP execution, MCP CI jobs and nodes.<id>.backend.stop all still travel on the bus. Absence semantics are unchanged. An agent node now has a real node_connections row whose departure ages past the grace, so the node type check in HealthMonitor.tunnelDeparted stopped being an optimisation and became the rule; its comment says so, and the spec that pins it is shown red under a mutation that deletes the check. The scheduler needed no change: every placement query already filters node_type = backend, so an agent node never reaches nodeMayTakeWork. Shared rules moved to one site each. The request bounds, the POST-only check and the unknown-path 404 live in workerctl and are called by both worker packages; the bearer check that guards every extra route is one function in core/services/nodes used by both server constructors. workerctl.AllPaths splits into BackendPaths and AgentPaths, with AllPaths as their deduped union, because a backend worker does not mount the agent verbs and asserting otherwise would fail a correct worker. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Claudio Maradonna
|
9e831d7709 |
fix(ds4): build CUDA kernels for the target architecture (#11840)
* fix(ds4): build CUDA kernels for the target architecture The ds4 backend compiled its CUDA objects with no -arch. Upstream's Makefile leaves CUDA_ARCH empty and its `cuda` target refuses to build without one, offering `cuda-spark` (sm_121) and `cuda-generic` (native) instead. We invoke its object targets directly, which bypasses that guard, so nvcc fell back to its default architecture and the kernels ran as JIT'd PTX on the real GPU. On GB10 (sm_121) that silently corrupted inference: any prompt over roughly 128 tokens produced text unrelated to the input and never closed its thinking block, so content came back empty and the chat showed only reasoning; longer prompts failed with "cuda decode failed". It also cost close to two orders of magnitude of prefill throughput. Measured on one box, same model, same prompt, same GPU, upstream ds4 at the pinned commit, differing only in the nvcc flags: make -B ds4 (archless, as we build it) garbage output 4.21 t/s make cuda-spark (compute_121a/sm_121a) correct output 325.70 t/s Select an architecture list from CUDA_MAJOR_VERSION, which the backend matrix already declares for both ds4 cublas entries but Dockerfile.ds4 never forwarded. Upstream's CUDA_ARCH takes a single value, so it cannot express the fat binary these images need; NVCC_ARCH_FLAGS is overridden instead, since a command-line assignment wins over its `:=`. The lists are copied from vllm-cpp rather than invented so the two CUDA images cover the same GPUs, with l4t/arm64 covering Orin, Thor and GB10. An empty CUDA_MAJOR_VERSION keeps upstream's `native` behaviour for local developer builds, and no CI runner has a GPU to enumerate. DS4_CUDA_HAVE_MXF4 is deliberately left unset: upstream defines it only for single-arch sm_120/sm_121 builds and guards it with a plain #ifdef rather than __CUDA_ARCH__, so it cannot be combined with older archs. It gates an optional MXFP4 indexer fast path whose #ifndef branch returns 0 and falls back cleanly, so omitting it costs speed on GB10, not correctness. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Claudio Maradonna <git@codeshifter.xyz> * test(ds4): cover the multi-batch prefill regression The architecture fix has no automated guard: every existing e2e spec uses a short prompt, and the miscompiled backend answered short prompts correctly. The corruption only appears once a prompt spans more than one prefill batch, so the whole suite passed against a backend that produced garbage in normal use. Add an opt-in "long_prefill" capability to the backend e2e suite that sends a prompt well past one batch with a known needle and asserts the answer still reflects it, and document in the ds4 guide why the build must never omit an nvcc architecture, how to check which flags a configuration resolves to without compiling, and how to run the new spec. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Claudio Maradonna <git@codeshifter.xyz> --------- Signed-off-by: Claudio Maradonna <git@codeshifter.xyz> |
||
|
Ettore Di Giacinto
|
6717856212 |
test(distributed): prove phase 3 end to end, under real processes
Five cluster specs that run the binaries an operator runs, plus the repair
of eighteen specs phase 2 left red.
The eighteen were router_tracking and full_flow, failing since
|
||
|
Ettore Di Giacinto
|
dd9aff58ff |
feat(distributed): take the backend worker off NATS entirely
A local-ai worker no longer opens a bus connection. connectNATS and its
spec are gone; Run registers once, starts its tunnel, arms /readyz on that
tunnel, and heartbeats. The worker's bus credential flags (--nats-jwt,
--nats-user-seed, --nats-require-auth, the three TLS flags) and
Config.NatsAuthRequired go with it. --nats-url stays, accepted and
ignored, so an existing worker command line still parses.
/readyz was the thing most likely to wedge a tunnel-only worker: it
required a live NATS link, so a worker with no bus would have reported
itself unready forever. nodes.NATSReadiness becomes nodes.TunnelReadiness
over a local interface{ Connected() bool }, and worker.Tunnel gains
Connected(), backed by a mutex-guarded session field the loop publishes
and clears. A closed-but-not-yet-cleared session reads as disconnected:
the loop waits for every in-flight stream before it clears the field, and
the probe must answer not-ready through that wait.
The heartbeat gate is DELETED rather than re-pointed at the tunnel. The
heartbeat is the worker's own answer that its process is alive; whether
the frontend can reach it is a separate fact the frontend already holds
and ages against LOCALAI_WORKER_RECONNECT_GRACE. Withholding the
heartbeat would report an unreachable worker as an absent one on the one
path with no grace, where the health monitor marks it offline and its
pending backend ops are deleted behind it. heartbeatLoop is given no view
of the tunnel, so a gate cannot be added back without changing its
signature.
Removing the NATS credential manager from this path also removes a defect
it carried: its refresh loop re-registered on a timer to renew a JWT, and
Register CLEARS a node's NodeModel rows. Any backend worker running on
frontend-minted credentials had its replica rows deleted roughly every
18 hours.
Of core/cli/workerregistry, everything survives. The manager is still
used in full by core/cli/agent_worker.go, which still needs NATS: Acquire,
Provider, RefreshLoop, HasCredentials and TunnelToken are all untouched.
The backend worker simply calls RegisterFullWithRetry directly now.
WorkerPermissions is documented as serving agent nodes, and its non-agent
branch narrowed to _INBOX.> on both sides. It is NOT deleted: NATS reads
an empty allow list as no restriction, so returning nil would upgrade
every JWT the frontend still mints for a backend node from its own inbox
to the whole account.
Agent workers keep the bus everywhere: their CLI flags, their
subscriptions, the agent branch of WorkerPermissions, and the compose
service with its LOCALAI_NATS_URL and depends_on: nats.
Also corrected two flags the Nodes page advertised that do not exist
(--distributed-nats, --distributed-db), and a log line plus several
comments that still named a bus the code no longer touches.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
|
||
|
Ettore Di Giacinto
|
4c3e0deb19 |
test(distributed): pin the unreadable-request rule at all three file verbs
The rule "a body this worker could not parse is a non-2xx, never the worker's answer" is written at three exits in control_files.go and only ensure was pinned. Turning stage's or listdir's decode exit into a 200-with-error left worker and nodes entirely green, and what that converts is a frontend's malformed request into the worker's own verdict about a file, which passes cluster.IsWorkerAnswer and reaches a reap guard. The production code was already right; nothing held it there. The e2e NATS JWT spec was asserting the opposite of the code and passing. It published nodes.<id>.files.in and called it an allowed subject after that grant was deleted, and it could not tell: a permission violation does not close the connection, so FlushTimeout and IsConnected both stay happy. It now reads LastError, the way its sibling always has, and asserts the denial plus the one publish right a backend worker has left. Also pinned, each mutation-verified alone: the CreateTemp branch (an existing staging-tmp at 0500 reaches it without a seam), the walk's context check (a caller that gave up must fail the listing, never be answered with a short one), and the cache and data directory layout. The data directory was derived twice, once in worker.go and once for the listdir verb; worker.go now reads the same helper, so a move cannot leave a verb listing files the file server does not serve. The per-verb RPC ceiling moves from an argument at five call sites into fileRPCBudget, so no site can name the wrong one, and the two values are asserted. The body-cap table now holds both directions locally and with two different claims: a body exactly at the cap proves the bound is a ceiling and not an off-by-one, and an absolute megabyte proves the cap stays above real gallery traffic. Only the second notices a cap shrunk to 64 KiB. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
7fc617c8ed |
feat(distributed): serve file staging over the worker tunnel
The four nodes.<id>.files.* subjects were the last commands a serve-backend worker took off the bus. They are now HTTP routes under workerctl.Prefix, on the same loopback server and behind the same bearer check as the ten lifecycle verbs, so the frontend reaches them through the worker's tunnel. files.listdir is the verb this matters most for. Its reply had to fit a payload the bus would carry, which put a wide model directory close to the limit; a response body has no such ceiling, so nothing truncates the listing at either end. A short listing reads to the frontend as files the worker does not have. S3NATSFileStager becomes S3FileStager and calls ControlClient, which means every failure now lands in the bucket phase 3 exists to keep straight: a route this frontend could not use is unroutable and nothing may act on it, while the worker's own answer, including "that file is not there", is evidence a caller may act on. Each RPC's deadline is DERIVED FROM the caller's context rather than started fresh, at every one of the five call sites, so a caller that gave up stops the RPC too. A worker started without an object store mounts no file verb at all and answers 404, which is the same answer a build too old to know them gives. The subjects and the backend worker's files.> publish grant go with them; a backend worker now publishes nowhere but its own inbox. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
44f12b2adb |
feat(distributed): call the worker's control routes instead of the bus
The ten backend and model lifecycle verbs stop being NATS requests and become HTTP calls on the worker's own control routes, reached through that worker's tunnel on the `http` stream tag that already carries file staging. Nine subject builders and the per-op install-progress subject are deleted with their entries in the worker's NATS permissions; the request and reply DTOs are untouched, so a body on the wire is byte for byte what the subject carried. This closes the merge gate Task 3 left open, which was worse than lost commands. Once the worker stopped subscribing, PingNode was still asking nodes.<id>.backend.list and nodes.<id>.models.running, so EVERY healthy worker answered no-responders, nodeAnswersOnBus read it as absence and pickReachableNode demoted it on the scheduling path. PingNode is a control RPC now, and no control RPC can produce ErrNoResponders, which is the only error that exclusion acts on. Two specs drive pickReachableNode against a real adapter and a worker answering over its control plane, which is the only arrangement that can see the difference: the router's own double never touches a transport and stayed green for the whole window the defect was live. How a control RPC FAILS is the whole of this change, so it is decided in ONE function reading ONE table. A worker's answer passes through unwrapped, so cluster.IsWorkerAnswer still sees it and a reap guard may act on it; everything else is wrapped in ErrWorkerUnroutable so nothing can. There is no third branch, because a third branch is how the eight collapses on this branch happened: each was a site that decided for itself which errors were evidence. A 404 under the prefix is its own sentinel, because it is the worker stating a deployment fact about ITSELF rather than a verdict about a backend, and only the legacy upgrade fallback may act on it. The caller's budget is checked FIRST. A timeout is not a verdict: a refusal arriving in the instant a deadline expires would otherwise be reported as the worker's non-transient answer, which reaps a row, and nothing orders the two timers. A 5xx and an undecodable body are transport failures, not answers. An empty ModelsRunningReply means "this worker is running nothing", which the reconciler acts on, so it must never be manufactured from a body that would not parse. A stream that ends before its reply line is the same rule one layer up: a tunnel dying mid-install is not the worker saying the install failed. backend.stop is split by node type rather than moved. Agent workers hold no tunnel, so they have no control plane to serve, and they still subscribe to nodes.<id>.backend.stop to drop cached MCP sessions; that subject and its agent permission both survive. It is the honest intermediate state until agent workers hold tunnels too. A failed control RPC no longer demotes a node anywhere. ErrNoResponders meant "not on the bus"; a control failure means "this frontend could not route to it", which is equally what a healthy worker re-homing its tunnel between replicas produces. Absence is a fact read from the database, and the scheduler starts reading it in a later task. The rolling-update fallback re-fires a DESTRUCTIVE force-reinstall, so it runs only on the worker's own 404. Its negative direction was pinned at the admin call site and unpinned at the reconciler's, where widening the condition to any error left all 676 specs green: a background drain nobody is watching would then force-reinstall every queued backend the moment a replica lost its tunnels. Three specs cover it, arranged so the force install IS reachable in the negative case and a fallback that fired would show as a call and a drained row. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
localai-org-maint-botandEttore Di Giacinto
|
9afe10ba21 |
fix(distributed): survive a slow control-plane database (#11837)
* fix(distributed): evict only when a node is known to be full scheduleNewModel asked the registry for a free replica slot and treated every error as "this node is full", so a control-plane database slow enough to time out the lookup evicted a healthy loaded model. The evicted process died, a peer frontend still holding its address dialled the dead port and retried, and the model thrashed between nodes. The comment on the branch already said it meant a full node; the code never tested for it. Evict only on ErrNoFreeSlot. Any other error now returns and names the lookup that failed, so a slow database degrades into a diagnosable load failure instead of into lost work. An audit of the rest of the router found one branch of the same shape: node selection discarded the error from its last-resort finder, so a database timeout there also produced a nil node and evicted for it. That path now returns unless the finder said gorm.ErrRecordNotFound, which is the only answer that means the cluster had no node to give. No other destructive branch in router.go fires on a generic error. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * fix(distributed): checkpoint heartbeat writes instead of writing every beat Every heartbeat UPDATEd backend_nodes. Six nodes at a ten second beat is roughly 52,000 writes a day against a six-row table, and that churn is what turned a blocked autovacuum into a 460 MB table whose six-row scan cost 867 ms and timed out the queries that place models. A beat carrying only a fresher timestamp now waits for the checkpoint interval. Each reported field is compared against the value last persisted rather than tested for presence, because a worker sends its disk figures on every beat and presence alone would suppress nothing. A node's first beat, a changed total VRAM, total disk or GPU vendor, and a free VRAM, RAM or disk reading that has moved more than 256 MiB from the persisted value all still write at once. A node that is not active is never suppressed, because it recovers only when the health monitor sees a fresh timestamp. The persisted column is up to one interval stale by design, so the stale-node threshold moves from 60s to 5m to cover it. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * fix(distributed): fail worker readiness when a held backend is unreachable The readiness gate tracked only the NATS link, so a worker whose backend processes had died still answered /readyz with 200 and kept receiving loads. One node did exactly that during an incident: it reported healthy while its backend port refused connections, and every load routed to it failed. Readiness is now the NATS link and, for each backend process the worker believes it is running, a short dial of its recorded address. A worker holding no backends stays ready, because idle is a healthy state. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * fix(distributed): keep a starting backend out of the readiness dial set A backend process is inserted into the supervisor map with its gRPC address already recorded, but the address refuses connections until the gRPC server binds, which the startup poll allows up to 30 seconds for and which takes 10 to 15 seconds on a slow node. The new data-path readiness probe dialled that address straight away, so a worker answered /readyz with 503 for the whole of every cold backend start. The container HEALTHCHECK absorbs that, but a Kubernetes readinessProbe at 10s does not, and the worker would leave rotation each time it loaded a model. The skip for a stopping process had no counterpart at the other end of the lifecycle. Backend processes now carry a serving flag, set where the startup health-check gate succeeds, and the probe dials only processes that are serving and not yet stopping. backendStartStillValid becomes markBackendServing: the check and the mark must share one lock hold, so the flag can only ever land on the entry the key currently owns. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * feat(distributed): export control-plane database health gauges Four transactions wedged on a corrupt index held the vacuum horizon open for 42 days. Nothing measured it, so the first symptom anyone saw was models failing to load six weeks later, by which time a six-row table had grown to 460 MB. Export the oldest xmin age, the longest open transaction, and the dead tuple ratio on the registry tables. The first is the number that would have caught it: it sits near zero in health and was 21,002,291. Sampling is scrape-driven behind a cache, and a failed sample reports the last good values rather than failing the scrape, because these gauges matter most when the database is already struggling. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * fix(distributed): rate-limit failed control-plane database samples The cache advanced its clock only on a successful sample, so once the database started failing every scrape retried the query immediately. That turned the cache off in the one regime it exists for: a retry storm at scrape cadence aimed at a database already in trouble. A catalog read that consistently exceeds the 5 second timeout also paid that cost on every scrape, with all scrapes serialised behind the sampler mutex. Time every attempt rather than every success, so failures and timeouts cost the same interval as good samples. Whether a good sample exists moves to its own field, keeping the gauges absent until the first success and holding the last good values through later failures. Also note in the runbook that pg_stat_activity cannot see prepared transactions or replication slot xmins, so a healthy-looking xmin age does not by itself rule out a blocked horizon. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * test(distributed): pin that a failing database evicts nothing Exercises the real distributed stack against a control-plane database that refuses the router's slot lookup, and asserts the scheduler reports the lookup it could not answer instead of falling through to eviction. The failure is injected with privileges rather than a statement timeout. A timeout set with ALTER DATABASE also breaks AutoMigrate, and it leaks into every later spec in the suite unless it is reset, so the spec would end up testing the migration rather than the scheduler. Instead the spec creates a dedicated login role, points a second gorm handle at it, and revokes that role's SELECT on node_models.replica_index. This has to be a separate role: the test container's owner is a PostgreSQL superuser, and superusers bypass every privilege check, so revoking from CURRENT_USER is recorded and then ignored. The revoke is scoped to one column on purpose. Revoking the whole table would also blind node selection, which runs first and has a guard of its own, so the scheduler would never reach the slot lookup this spec is about. Leaving every other column readable lets selection succeed and lands the refusal exactly on NextFreeReplicaIndex, which plucks replica_index. The grant is restored from BeforeEach via DeferCleanup, so a failing assertion or a panic cannot hand the next spec a role that cannot read. Reverting the eviction guard fails this spec, which is the point of it: the router then reports "no replica slot on keeper and eviction failed" for an error that was never evidence the node was full. The surviving-row assertions are secondary under this injection, because the eviction path reads whole node_models rows and the same revoke blinds it too; a comment in the spec says so, so nobody mistakes them for the load-bearing ones. Also documents why the vector store and the control plane must not share a database: the removable-tuple cutoff is per database, not per table, so one transaction left open anywhere stops autovacuum reclaiming the node registry, and a six-row table bloats into hundreds of megabytes. The note names LOCALAI_AUTH_DATABASE_URL and LOCALAI_AGENT_POOL_DATABASE_URL as the two knobs that must differ, and the localai_control_plane_oldest_xmin_age gauge as the way to see it coming. grep for StaleNodeThreshold and HealthCheckInterval in core/config/runtime_settings_registry.go returns no matches: the distributed duration knobs are not exposed as runtime settings, so the new heartbeat checkpoint interval follows them and needs no registry entry. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * fix(distributed): close the review gaps in the heartbeat and health path The stale-node threshold moved from 60 seconds to 5 minutes in this branch because checkpointing makes last_heartbeat up to one checkpoint interval behind by design. Two things were left inconsistent with that. NewHealthMonitor still fell back to a hardcoded 60 seconds when handed a zero threshold, so any future caller that stopped passing the configured value would mark every healthy, beating node offline on every cycle. And the threshold itself had a flag-name constant but no AppOption, no CLI field and no env binding, so an operator who widened --node-heartbeat-checkpoint had no way to widen the threshold to match. The fallback now tracks config.DefaultStaleNodeThreshold, and --stale-node-threshold / LOCALAI_STALE_NODE_THRESHOLD is wired the same way its sibling is. Heartbeat suppression compared the RAW reported free VRAM against the snapshot, but the column persists capAvailable(raw, ceiling). On any node with a VRAM budget set, whose actual free VRAM oscillates above that ceiling, every beat looked material while the persisted value never moved: suppression was defeated on exactly the nodes an operator had configured, and the write amplification this branch exists to remove came straight back there. The comparison and the snapshot now both hold the capped figure, so they measure the same quantity as the column. Fixing that needs the ceiling, and reading it cost a SELECT on every beat, including suppressed ones. The skip decision therefore moved ahead of the updates map and now reuses the ceiling cached on the last durable write, while the write path still re-reads it before capping anything. A ceiling that changed inside the checkpoint window can cost one extra or one late write; it cannot persist a wrong figure. A suppressed beat now costs no query at all. Also: the operations section now says to grant pg_read_all_stats to the LocalAI role, because PostgreSQL blanks backend_xmin and xact_start for sessions owned by other roles, and the transaction that wedged the horizon in the incident was a co-located vector store connecting as a different role, so without the grant the new gauge sees only our own sessions. The compose healthcheck comment now describes readiness covering the backend data path, and the control-plane gauge registration records the otel.SetMeterProvider ordering it depends on. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> * fix(distributed): resolve the gauge's table names through gorm The dead-tuple gauge queried pg_stat_user_tables against a hardcoded list of three table names. Those three do not agree on where their name comes from: BackendNode and NodeModel take gorm's default pluralisation, while GalleryOperationRecord overrides TableName, and gallery_operations already had a constant of its own that the list duplicated. A literal list keeps compiling after any of that moves, and the query then matches nothing. The failure is silent and it points the wrong way: a dead-tuple ratio that matched no rows reports the same numbers as a cluster with no bloat, so the gauge would look healthiest exactly when it had stopped working. Ask gorm what each model is stored as instead, which follows a TableName override and the default pluralisation alike. A spec pins that the override really is consulted: naive pluralisation of the type would give gallery_operation_records, so the resolution cannot quietly stop asking the model. 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> |
||
|
Ettore Di Giacinto
|
0dc6ebd525 |
fix(cluster): stop blaming a peer for the caller's own expired deadline
Review round 1 on the end-to-end proof. Zero blocking items, eleven non-blocking, and three of them turned out to be production defects rather than notes on the report. The one that matters is a misclassification the phase is built to prevent. A dial carries the caller's deadline down to the socket, so when the budget runs out the socket's timer fires and the error travels back up through the WebSocket handshake and the multiplexer. The context's cancellation is a separate timer whose func the scheduler has to run before ctx.Err() stops returning nil, and nothing orders the two. Under contention the socket's error is back in PeerPool.Open first, ctx.Err() reads nil, and a peer that is listening and healthy is reported as ErrPeerUnreachable to a caller that simply ran out of time. An unreachable peer is a fact a caller may act on and an expired deadline is not, and core/services/nodes routes around a replica it is told is unreachable. callerRanOut answers that question in one place: ctx.Err() when it is set, and otherwise the wall clock against the caller's own deadline. That is sound because it is the same instant the socket compared itself against, so if the socket's timer fired this comparison is past it too. The ambiguous instant resolves towards the caller, which is the direction that never blames a peer. The spec that caught it, peerlink_test.go's "blames the caller's deadline", was red in three of seven -race runs and had been since Task 5, which is often enough to read as noise and is why single-run verification never saw it. Rather than leave the proof to a coin flip, a second spec makes the window deterministic: Open is handed a context whose deadline has passed and whose cancellation has not been delivered, against an address nothing is listening on, so the dial fails for real. It reddens without the fix. The peer link's yamux windows were applied to one end only. A receive window is advertised by the side that RECEIVES, so configuring the dialler alone tunes exactly one direction, and the direction left on the 256 KiB default is the one that carries a relayed model artifact INTO the replica that owns the worker's tunnel. That is the largest thing the link ever moves and it is the direction the load measurement exercises: the review read it as flowing toward the dialler and it does not. PeerLinkConfig is now exported and used on both ends. Measured, same box, 128 MiB staged through the relay against the same transfer without one: the relayed path cost 1.6x to 2.0x the direct path's transfer window before, and 1.06x to 1.25x after. The SSRF reachability spec could be fooled into reporting an SSRF that did not happen. It bound the victim on 127.0.0.2 at an ephemeral port and required 127.0.0.1 at the same port to refuse, so any other spec in the run holding that number made the dial succeed; red one run in seven, green five of five in isolation. It now picks from below the kernel's ephemeral range, the same fix the harness got for the adjacent-port collision. The rest are the specs and the report saying what they mean. Scenario 1's advertisement assertion could not tell "the worker advertises nothing" from "the JSON key moved", which matters because removing the advertisement is the change it covers. It was green against a renamed key. The roster now keeps the raw key set beside the decoded fields and the spec requires both keys present before reading them as empty. Scenario 4's refusal-body check was a four-way disjunction admitting bare "tunnel", "not connected" and "unroutable". Those alternatives were inert and each would be satisfied by refusals that say nothing about routing, in the one assertion the whole negative control rests on. It is "no route" alone. The head-of-line gate bounded the worst probe by the whole transfer window, which admits about eightfold degradation and loosens as the box slows. It is now half the window, plus a scale-free ratio against the worst probe under the SAME cold load with nothing to transfer, which is the control that isolates the transfer from the load. Not tighter than that, and the reason is measured rather than cautious: under a concurrent -race suite the worst relayed probe reached a fifth of its window, so a quarter-window gate would have had 1.2x of margin, and a spec that fails one run in three is worse than no spec. The report entry printed p90 and p99 off samples of twenty, where both land on the same element and p99 often lands on the max, so one number appeared three times under three names. A quantile is now printed only when the sample can separate it. Two claims in the report were wrong and are withdrawn rather than softened. Scenario 2's race is closed by the trailing re-read of the owner, not by the pre-assertion the report credited: a move to the non-owner mid-request would serve directly and still return 200, and only the trailing read reddens on it. And "the median request is unchanged" holds on this box and not on the reviewer's, where the relayed median rises up to 82% and p99 up to 3.5x. What survives on both is structural: the worst probe is a small fraction of the window in which bytes are moving, so the session interleaves rather than serialising. Sharing a session with a bulk transfer costs latency; it does not cost service. The disk footprint note undercounted, and the reviewer lost a run to a full disk on this box, so it is worth having right: two bulk models seeded into two frontends and staged to the worker is about 768 MiB, not 512 MiB. Left alone deliberately: the worker's backend port allocator still hands out ports without checking they are free, and its default range still overlaps the kernel's ephemeral range. It is confirmed, it is out of scope here, and it is being tracked as a named follow-up rather than fixed under an e2e task. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
0683fb1579 |
test(distributed): prove the worker tunnel end to end, under real inference
Everything this phase built was proven by unit and integration specs. This is the first run of it against the real binaries: a frontend replica per process, a worker that binds nothing routable, real inference over the result. Four scenarios, each with the question "what would make this pass if the tunnel were doing nothing" answered rather than left open. A worker with no advertised address is reached through its tunnel. The roster is asserted to report it advertising nothing, so there is no address a frontend could have dialled instead, and node_connections is asserted to name the replica that serves the request. A request landing on the replica that does NOT own the worker is relayed to the one that does. With N replicas behind round robin that is (N-1)/N of production traffic, so it gets the FIRST request for its model: the backend install, the file staging on the http tag, and the gRPC load and predict all cross the relay. Which replica owns the tunnel is read from the ownership table through the production Owner query and mapped to a frontend index through the address the harness pins per replica; the non-owner is derived from that reading and asserted to be a non-owner immediately before the request, rather than assumed from the harness default. Sending the same request to the owner reddens it. Killing the owning replica re-homes the worker onto the survivor. The worker dials a balancer rather than a replica, because LOCALAI_REGISTER_TO is resolved once at boot and is the tunnel endpoint as well as the registration one: aimed at a single replica, a worker has nowhere to reconnect to when that replica dies, and the re-home cannot happen at all. Removing the kill reddens it. And the negative control for the whole suite, which is why the other three mean anything. Frontend and worker share a host here, so every backend port the frontend names in a stream target is one it could have dialled directly; if it did, the first three would pass with the tunnel inert. LOCALAI_WORKER_TUNNEL is no longer usable for this, because it is a fatal startup error and a worker that never started says nothing about a worker reachable some other way. The balancer answers the tunnel connect path itself instead, leaving a worker that registers, heartbeats, reports healthy and holds no tunnel. It is asserted to have dialled and been refused, asserted to be held by nobody, and then asserted unreachable with the refusal naming the missing route. Then the block is lifted, nothing else changes, and the same request succeeds: that is what attributes the refusal to the tunnel rather than to any of the ordinary reasons an e2e inference fails. The fifth spec measures the head-of-line blocking this phase deferred three times. 128 MiB crosses the session while a warm model is probed back to back, direct and relayed. Median latency is unchanged, the worst probe is about 3x the baseline median and about a seventeenth of the transfer window, and the transfer runs at 415-490 MB/s direct and 222-268 MB/s relayed. A session that head-of-line blocked would park a probe for the length of the window. Leave the yamux windows untuned; and note this is loopback, so it says the multiplexing does not serialise and says nothing about a link with a bandwidth-delay product. The load spec is measured against a control that the first version did not have. It passed with the bulk artifact cut to 4 KiB, because the window it read probes against was mostly cold-load overhead: it would have reported a clean bill on a session carrying no large message. The same cold load now runs twice, once empty and once bulk, and the difference between the windows is asserted to be real before any latency is read from it. Two defects on the base commit came out of this. cluster_peerlink_test.go has been red since the relay landed, deterministically, in isolation and in the suite. It asserted that an accepted peer stream is refused at once, on the premise that phase 1 installs no relay. The relay correctly waits fifteen seconds for a frame naming the worker, and the spec's budget was five. It now writes a relay request for a node no replica holds and asserts the refusal is ErrNotOwner and specifically not ErrNoConnection, which is a stronger spec than the one it replaces and the only thing in the e2e suite that exercises the relay's refusal path. The harness handed a worker's own HTTP port to a backend process. It took two ports from freeport and used one as the gRPC base and the other for the file transfer server; freeport returns adjacent ports often, and the backend allocator hands out base, base+1, base+2, so the second backend started on a worker was regularly given the HTTP server's port and died with EADDRINUSE. No spec had started two backends on one worker before, so it had never fired; the load spec starts five and it failed about one run in three. Each worker now reserves a contiguous bind-probed block laid out the way production lays it out, below the kernel's ephemeral range, with LOCALAI_GRPC_MAX_PORT bounding the allocator to it. The underlying production defect is not fixed here and is recorded in the report: allocatePort never checks that a port is free, and its default range overlaps the ephemeral range on every Linux box. Constraint 6, whether distributed mode should now refuse to start without an advertised address, is DEFERRED, and the comment and the docs that described the cost were understating it. A replica with no advertised address writes no instances row, and Owner joins a connection against a live instance, so a worker whose tunnel lands there is unroutable from every OTHER replica while being registered and healthy. Refusing to start would still be wrong, because the deployments it would break are single-host ones with no peers to be unreachable by, and telling those apart at startup is a design with its own specs. Both places now say what actually happens. Suite wall clock 592s for 15 specs, up from 502s for 10 of which 2 were red. The CI budget of 20 minutes does not move. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
1cf847f29e |
feat(distributed): stop workers listening, and stop them advertising
A worker now opens no listener on a routable interface and states no endpoint at registration. Backend processes and the file-transfer server bind loopback, and the frontend reaches both through the tunnel the worker dials. The bind address is built from loopbackHost, the same constant the tunnel's grpc tag dials, so "the worker binds where its tunnel dials" is one fact in one place rather than two literals that can drift. All three advertisement sites are closed, not one: the registration body, RegisterNodeRequest, and the per-backend address in the install reply. That third one was hiding a live bug. stopModelExact refuses a stop whose ExpectedAddress does not match what the worker recorded for the process. The worker recorded 127.0.0.1:port; handleBackendInstall reported advertiseHost:port; the router stored the reported one and sent it straight back. On any worker whose advertise host was not 127.0.0.1, every acknowledged model stop failed with an address mismatch. Nothing caught it because the e2e harness set LOCALAI_ADVERTISE_ADDR=127.0.0.1, which made the rewrite a no-op. Removing the rewrite makes the two strings the same by construction. The brief was wrong about two of the four functions it called dead. effectiveBasePort is the base of the backend port allocator and resolveHTTPAddr is the file server's bind address; deleting them would have deleted the port allocator and the file server. Only the two advertise* helpers were dead, and addr_test.go is rewritten rather than deleted, because the port arithmetic it pinned still needs pinning. NodeModel.Address survives with a narrowed meaning and is renamed WorkerLocalAddress, along with the install reply field that feeds it. The frontend still has to say WHICH backend process on a worker it means, and the port in this string is how it says it: it travels as a stream target and the worker dials its own loopback. The gorm column and the json key stay "address", so neither a migration nor an API break rides along. Every fall-back to the node's address is gone. installBackendOnNode now errors when a worker reports success without naming one, because substituting the now-always-empty node address would name an empty target, and the worker refuses that as an invalid stream, which is classified as the worker answering about its backend. That is the "a present worker reads as something it is not" class this phase forbids. DistributedModelStore.Range had the same shape and was already wrong: it built each remote model's client from the node's base gRPC port, never the port a backend process listens on, so Free and Status went to the wrong place. It uses the replica's address now. BackendNode.Address and HTTPAddress are kept but made provably inert: no writer, no reader that acts on them, and Register force-clears both on re-registration so an upgraded worker's stale advertisement does not outlive its own upgrade in the API and the Nodes page. Dropping the columns is a ~90-site edit across the specs, the e2e suite, the MCP dto and the UI; it is recorded as a follow-up rather than folded in here. A persistent tunnel 401 still does not trigger re-registration, and now for a reason rather than a deferral. Register CLEARS the node's replica rows, so re-registering on a 401 would delete a live worker's rows on every retry, and under the name collision that causes the 401 the two workers would take turns doing it forever: a credential failure causing model reclamation. It also cannot fix the named cause, since a collision is indistinguishable from a restart. The 401 log now names both causes and says nothing can reach this worker, which is true only now that it has no listener. The container healthcheck did not break the way the brief expected, since the listener still exists on loopback and the probe runs inside the container. It did have a real #10987 defect that this change makes the common case: it read LOCALAI_SERVE_ADDR only, while effectiveBasePort reads LOCALAI_ADDR first, so a worker on a non-default base port was probed on 50050 and reported unhealthy while working. It follows the same precedence now. Docs, the compose file and the e2e harness are updated in step: no inbound rule or published port is needed for a worker, the two advertise variables are gone, the remaining address variables are read for their port only, the firewall-the-file-transfer-port warning is narrowed to the LOCALAI_HTTP_ADDR opt-out, and the upgrade-order note no longer claims the worker still listens. The Nodes page showed node.address, which is now always blank, so it shows the node id instead. Eight mutations, all red on a named spec, including reverting the loopback bind, re-adding the address to the registration body, restoring both node-address fall-backs, dropping the force-clear, storing the endpoint's address again, and un-fixing the healthcheck. One of them caught a defect in a spec I had just written: it asserted 200 where the endpoint returns 201, which went unnoticed because core/http/endpoints/localai is not on the task's verify list. It is run here. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
75953d9f63 |
feat(cluster): reach every worker through its tunnel, never its address
The tunnel, the fence, the registry and the relay were all built and none of them carried a byte: every dial from the frontend still went to the address a worker registered. This is where that stops. One WorkerDialer resolves where a worker's tunnel is held, opens a stream on it locally or relays through the owning replica, and hands back a conn past both handshakes; gRPC, the file stager's HTTP client and the log-streaming WebSocket are all pointed at it. A worker's address stops being somewhere to connect to and becomes the name of which backend process a stream is for. It still appears in URLs, logs and errors, because that is what identifies the process; what it no longer decides is where the bytes go. Nothing falls back to dialling it. BackendClientFactory now has exactly one method, NewClientForNode, and returns an error where there is no way to reach the worker. The direct-dial constructor was removed rather than kept beside it, because leaving one on the interface keeps the bypass one word away from every call site that holds an address, which is all of them. The second construction path is closed too. DistributedModelStore built remote models with a nil client, and pkg/model.Model.GRPC then dialled the raw address lazily on first use - reached in production by ShutdownModel's Free and by the backend monitor's Status. Those models now carry the tunnel-backed client, and a model that cannot be given one is logged and not listed. Four conditions stay unmixable, and one path produces absence: the dialer answers ErrNoConnection only where Owner's liveness join did. A peer that will not answer, a stale ownership row, a worker's own refusal and a missing relay path are each reported as themselves. This matters because nodes ACTS on absence, and the collapse would have it reclaim the models of a worker that is connected and busy. That is not hypothetical. Writing the mutation for it exposed the bug in this change's own first draft: probeHealth returned bare false when it could not build a client, and tryWarmPath deletes the replica row on a false probe. A frontend whose dialer broke would have emptied node_models for the whole deployment while every model kept running. probeHealth now returns alive and probed separately, the reconciler gets a ProbeUnknown outcome that neither advances nor clears a failure streak, and the health monitor skips rather than counting a miss. Task 5 left the relay's open timeout at a fixed 15s and said so: no operator has the information to set it, because the number that matters is the original client's remaining budget, which is invisible on the relay side. The dialer has that budget, so it now states it in the relay request frame and the owner takes the smaller of the two. It can only shorten - a patient client must not be able to park a relay goroutine and a stream slot on a worker that stopped accepting. Zero is written as no budget at all, since on the far side the number zero is a caller with nothing left and would refuse healthy traffic. Seven mutations, each reddening a named spec: peer-unreachable as absence; the local-failure guard dropped; max instead of min on the budget; the nil-client model restored; ProbeUnknown falling through to the reaper; OwnerRow instead of Owner; probed collapsed into alive. The first budget spec passed for the wrong reason - a handshake deadline, not the relay - and was replaced by three that each assert one link, including one where the spec plays the owning replica and reads the budget out of the frame instead of inferring it from a clock. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
e26d556594 |
fix(cluster): hold the guarantees phase 1's comments were claiming
Review found the recurring class: assertions that a wrong implementation also satisfies. The "refuse promptly, never park the peer" guarantee was stated in three places and tested in none. Removing the Close from the no-relay branch left the whole cluster suite green, because the specs asserted only that some error arrived and yamux reports a read deadline as ErrTimeout: a parked stream satisfied that as well as a refused one. Both specs now require an ENDING, EOF or a reset, inside a deadline short enough that parking is unmistakable, and both go red when the Close is removed. Deregistration existed only in a comment. Membership.Stop ended the loop and left the row behind, so every clean rolling restart had peers dialling a corpse for the full liveness window; the shutdown comment described the opposite. Registry.Deregister deletes the row and the connections that replica owned, in one transaction, for the reason the sweeper does both, and an e2e spec pins departure inside a budget shorter than the liveness window so it cannot pass on the sweeper doing the work. Before: the spec times out with both replicas still live. After: 3.6s. The configured advertised address bypassed every check discovery makes, so the one value most likely to be copied between hosts, 127.0.0.1, was taken verbatim and would make every peer dial itself. Both paths now share one rejection rule: unparseable is refused, "this host" is warned about once and honoured, because a single-host deployment uses it correctly. Two comments claimed more than the code does. The sweeper said a stalled replica recovers via re-register; only its instance row does, while the connections another replica reaped stay gone and the sockets stay held here - phase 2 must re-claim, on re-register, every connection a replica still holds locally. And Owner became OwnerRow, documenting that the owner it names may be dead for up to InstanceLiveness plus a heartbeat and that any caller acting on it must join instances itself, so the deferred constraint lives at the call site rather than in a report; the plain name is left free for the joining version. Minors: warn once when the peer link mounts with no registration token, so an operator sees the cause rather than 401s; Stop no longer blocks forever when Start was never called; corrected the NewRegistry migration doc and an e2e comment that described a 6s window as "throughout". Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
aca383d263 |
feat(cluster): give phase 1 a call site, and prove it against real replicas
Tasks 1 to 5 built an instances table, a splice, both halves of a peer link and an epoch fence, and nothing in the tree called any of it: no replica registered, no route was mounted, no sweeper ran. Proving phase 1 end to end therefore had to start by wiring it. A frontend in distributed mode now publishes the address its peers dial, heartbeats it, and sweeps replicas that stopped answering along with the connection rows they owned, in one pass so the two can never disagree about who is alive. It serves the peer link and owns the sessions peers dial in, refusing streams on them until phase 2 installs a relay: a session nobody accepts on does not fail a peer's Open, it hangs it. The address is the one peers use, not the one the process binds, and it is derived from the route to PostgreSQL. That derivation only holds while the database is remote, so LOCALAI_DISTRIBUTED_ADVERTISE_ADDR sets it explicitly and a replica that can determine neither warns and keeps serving rather than failing to start. Three e2e scenarios run against real local-ai processes, real PostgreSQL and real dials: replicas publish addresses that can actually be connected to; a sibling opens a stream over the peer link and is refused without the cluster token; and a killed replica is reported unreachable, never absent, loses the claim it held, and takes no worker with it. Each was verified by mutation: eight injected defects, each failing the scenario that claims to catch it. Also moves RegisterClusterRoutes to core/http/routes beside every other registrar, folds AutoMigrate and the epoch sequence into one cluster.Migrate, and turns the peer route's auth-coverage spec into a real assertion: it drives the request through the actual auth middleware instead of comparing two string constants, which the old spec would have passed even with the exemption deleted. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
b13ebeaa1b |
docs(e2e): correct three claims in the distributed e2e comments
The closure note in cluster/failure.go quoted a Gomega error that Gomega does not emit. Describe the argument-count failure and the Eventually().WithArguments() hint instead, so nobody greps for a string that never appears. The advisory-lock note in cluster_failover_test.go called the wedge window unbounded. A SIGKILLed local child closes its socket at once, the Postgres backend reads EOF and is reaped in milliseconds, so the mechanism bounds the window tightly. Say bounded, and keep the low probability but real framing, which was right. The workflow comment attributed HealthCheckInterval to core/services/nodes/health.go. It is declared in core/config/distributed_config.go:64; health.go only carries the ticker on the unexported checkInterval. Point a debugger at the right file. Comments only, no behaviour change. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
c5796d407f |
test(distributed): correct the claims the e2e comments make
Review of the whole branch found five comments that would send a reader to the wrong place, plus three smaller inaccuracies. Nothing here changes behaviour. The KNOWN RACE note on both backend-log WebSocket handlers said the fix needs an atomic snapshot-plus-subscribe "under the store lock". It does not: BackendLogStore.mu guards only the buffers map, and AppendLine enqueues and fans out under the per-buffer buf.mu. Whoever took the store lock would ship and the race would survive, so both notes now name buf.mu and say what s.mu does and does not exclude. Two comments in the cluster harness quoted Eventually(c.FrontendAlive) .Should(BeFalse()). FrontendAlive takes an index, so Gomega rejects that with "requested 1 arguments but received 0". Both now quote the closure form the specs actually use, and say why the closure is needed. proveHealthCheckingIsAlive claimed to prove the health monitor ran for the whole preceding window. It proves the monitor was alive at the end of it, and inferring backwards needs any wedge to be sticky. In the peer-replica-death spec that inverts: health checks are single-flighted by a session-scoped pg_try_advisory_lock, the spec SIGKILLs the replica that may hold it, and until Postgres reaps the session the survivor acquires nothing and checks nothing silently. Consistently(healthy) can then pass because nothing was checking, with the positive control still succeeding once the lock frees. The doc now states what is proven, names that gap, and says the assertion is a floor rather than a proof. The Makefile still called DISTRIBUTED_TEST_FLAKES a retry count, which is what seeded that error into the two docs just corrected against it, and the workflow called the 15s window a reconcile tick when the mechanism is HealthCheckInterval in the node health monitor. Also: the cluster suite measured 509.1s / 509.8s / 512.3s, so about 8m30s and not the 8m39s/8m40s three files claimed; the dead-worker spec title implied two independent detectors when both probes read one advisory-lock-serialised verdict out of the same row; and the sanitizeDBName length assertion used <= 50, which an empty string also satisfies, where the invariant for an over-long input is exactly 50. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
58232a3f04 |
test(distributed): prove health checking was alive during the failover windows
The two specs that assert a healthy worker stays healthy were pure negatives: they say nothing happened. A cluster whose health checking had wedged, by leaking the advisory lock the monitor takes at health.go:110, would freeze the roster and satisfy both while observing a corpse. Kill the worker once the window closes and require the roster to settle it to offline, so the preceding Consistently is a statement about behaviour rather than about a stopped clock. Applied to the cold-restart spec as well as the peer-death one: a restart is exactly the event that could leave a replacement unable to check anything. Document the hazard that can make an offline assertion hang. The staleness branch skips a node already marked unhealthy (health.go:153-155), a skip meant for nodes an operator took down, which also swallows the flap: an unhealthy mark landing after the heartbeat goes stale means MarkOffline is never called and the node stays unhealthy forever. Name the file and line at the assertion, and have the failure message say so when the roster shows a node stuck there, so a timeout sends the reader to LocalAI rather than to the harness. Stop calling the two-replica registration spec a race. Start spawns workers sequentially and the registrations land about a second apart; it is a shared-roster identity test, and saying otherwise invites someone to trust it for something it does not check. WorkerRegistrar now bound-checks its index like every other index-taking method here. It answered 0 for an out-of-range worker, and 0 is a real frontend index, so the failure mode was a spec killing the wrong replica. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
875ff339ba |
test(distributed): cover replica death, rolling restart and racing replicas
Four scenarios with no prior equivalent: killing a replica must not disturb a worker that never depended on it, a cold-restarted replica must rehydrate the roster from shared state and keep accepting the worker's heartbeats, a dead worker must settle to offline on every replica, and two replicas registering a worker each must converge on one roster. The timings are measured, not assumed. Node liveness is heartbeat freshness, so the only eviction path is StaleNodeThreshold (60s) plus one HealthCheckInterval tick (15s), and neither is reachable from the CLI. A worker whose registrar was killed was observed going offline at 74.2s. Every window here is sized to outlast that, because an assertion that expires before the system could have reacted proves nothing. Two assertions are deliberately unlike the obvious form. Statuses are compared for equality against a probe that returns a sentinel on error, rather than asserting a name is absent from the healthy list: the list probe returns nil on any error, and "does not contain" is satisfied by nil, so a 401 at the second replica would have passed while observing nothing. And a killed worker is required to settle to exactly offline, because it first flaps to unhealthy at ~8s and back to healthy at ~14s, which any not-healthy matcher would accept. SpreadWorkerRegistrations is new, off by default, and exists so the racing spec is a race: the harness otherwise points every worker at frontend 0, which would have left that scenario asserting on two sequential writes through one process. The default is unchanged because the baseline specs depend on it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
737eb6c34c |
test(distributed): require the cluster binaries by default under CI
The previous round made a missing binary fail instead of skip, but only when a workflow remembered to set LOCALAI_E2E_REQUIRE_BINARIES. That leaves the silent pass one forgotten line away: the Cluster label partition is two specs, Ginkgo exits 0 on skips, and a job that skips both reports "0 Passed | 2 Skipped" and goes green having never started a cluster. So the polarity is inverted. Binaries are required whenever CI is set, which GitHub Actions always does, and the flag now exists to force the requirement OFF rather than to be remembered ON. A local developer sees no change, since CI is unset in an ordinary shell and a missing binary still skips with a message naming the path and how to build it. off, no, n and disabled are honoured as off; ParseBool rejects them, and reading a word that unambiguous as its opposite would be a worse trap than the one this removes. Also correct a claim the previous commit message got wrong. Comparing the worker's registration id across the two replicas does not pin the topology: NodeRegistry.Register looks a node up by name and preserves the existing id, and both replicas read one Postgres, so registering the worker with every frontend would yield identical ids too. The assertion is still worth keeping for what it does catch, a replica answering from its own registry or database instead of the shared one, and the comment now says that and nothing more. The topology fact moves to where someone would break it: a note on LOCALAI_REGISTER_TO recording that workers register with frontend 0 only, that the cross-replica specs depend on it, and that nothing in those specs can detect a change to it. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
2e4c731ea1 |
test(distributed): fail rather than skip the cluster specs in CI
The Cluster label partition is these two specs and nothing else, so a missing binary skipped the entire job. Ginkgo exits 0 on skips, so a build step that broke or moved its output would have left the job reporting "0 Passed | 2 Skipped" and going green without ever starting a cluster: the silent pass this suite exists to make impossible. Skipping stays the local default, which is the right courtesy for someone who has not run `make build`, but LOCALAI_E2E_REQUIRE_BINARIES turns it into a failure that names the missing path and the target that builds it. A value that is set but unparseable counts as on, since reading it as off would restore the very skip it disables. Failures also name themselves now. The roster poll kept returning a bare nil on error, so a 401 at the second replica, a decode failure and "the worker never registered" all presented identically as an empty list. It now retains the last error and the last roster and reports whichever happened, through a lazily evaluated Gomega description that costs nothing until something fails. Finally, the two-frontend spec no longer depends on the harness to mean what it says. It asserts an unauthenticated GET /api/nodes at frontend 1 is refused, which observes the admin gate instead of assuming it, and it compares the worker's registration id across the two replicas rather than its name. A future harness that registered every worker with every frontend would have kept a name-only assertion green while it quietly stopped proving anything about shared state. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
1f241fb310 |
test(distributed): prove the cluster harness with a two-replica baseline
Tasks 4 to 6 built a harness that runs local-ai as real child processes, but none of it had ever started a process: every spec so far returned inside argument validation. These two specs are the first to run it against a real binary, a real Postgres and a real NATS. Two frontends against one database both see a worker that registered through only one of them. Every failover spec assumes this, so it is asserted first. One admin session is minted at frontend 0 and reused for both replicas rather than registering per frontend. The auth routes share a five-per-minute-per-IP limiter and all e2e traffic is 127.0.0.1, so a session per frontend would exhaust the budget as soon as a spec needs a third one. Reuse is sound because sessions live in the shared Postgres and the harness pins one HMAC secret across replicas; frontend 1 answering /api/nodes with 200 on a cookie minted at frontend 0 is what proves it. The binaries are resolved before SetupInfra so a missing build skips without first provisioning a database the skip would then have to tear down. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
fc0fce8b7d |
test(distributed): correct the failure-primitive comments and guard the wipe
Review round 1. Comments only, plus one guard. The note on Process.alive claimed the exited check closed the zombie window. It does not. The reaper closes exited only after Cmd.Wait returns, and Wait marks the os.Process done before returning, so exited being closed implies signal 0 already errors and the branch cannot fire earlier than the one it precedes. The window between the child exiting and waitid collecting it stays open in both versions, and the only real mitigation is for callers to poll with Eventually rather than sample once. Keep the check as hygiene, say what it actually does, and say it again on the exited field, so nobody reads the old claim and drops the Eventually. Record what the cold wipe destroys. The harness sets no LOCALAI_STORAGE_URL, so the object store is a directory under DataPath, and quantization and fine-tune outputs live there too. Postgres keeps the job row; the artifact it points at does not survive the restart. A spec that asserts otherwise will fail for a storage reason wearing a failover costume. Tell callers to let a graceful stop finish before restarting: RestartFrontend terminates with SIGKILL, so pairing it straight after StopFrontendGracefully cuts the drain short and silently converts the rolling-update case into the crash case. Refuse to wipe when the cluster has no work dir. frontendDataDir is relative when baseDir is empty, so a Cluster built by some future test helper without one would have RemoveAll walking frontend-N/data inside the source tree. The guard sits before terminate, so a refusal leaves the cluster as it was. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
ce3f360219 |
test(distributed): add kill and restart primitives to the cluster harness
The point of running LocalAI as real child processes is to be able to take
one away. Add KillFrontend (SIGKILL, the lost replica), StopFrontendGracefully
(SIGTERM, the rolling update), KillWorker, RestartFrontend and FrontendAlive.
RestartFrontend pins the dead replica's original port. Workers read
LOCALAI_REGISTER_TO once at boot and never re-resolve it, so a replica that
returns on a fresh port is unreachable by exactly the workers that registered
with it and the failover under test never happens.
It also wipes the replica's data directory, so the process comes back with
empty local state and has to rehydrate node, session and job state from the
shared Postgres and NATS. Reusing the directory would model a pod with a
persistent volume and hide the class of bug these tests exist to find. That
is only safe because the harness pins LOCALAI_AUTH_HMAC_SECRET; otherwise the
wipe would take {DataPath}/.hmac_secret with it and every session minted
before the restart would 401 afterwards.
FrontendAlive consults the reaper's exited channel before signal 0: a child
that has died but has not yet been waited on is a zombie, and signal 0 to a
zombie succeeds, which would report a dead replica as alive.
The new specs cover argument validation only. Killing, stopping and
restarting a live process needs a built binary plus Postgres and NATS, so
those paths stay unexecuted until the failover suites land.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
|
||
|
Ettore Di Giacinto
|
23a2bd5f1b |
test(distributed): give each frontend its own data dir and one pinned secret
Session rows are keyed by an HMAC of the token under a secret generated
per instance into {DataPath}/.hmac_secret. The replicas shared that
secret only because they shared a working directory, and that directory
was the source tree. Give each frontend LOCALAI_DATA_PATH under its own
baseDir and pin LOCALAI_AUTH_HMAC_SECRET, so a session minted at one
replica resolves at every other one by construction.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
|
||
|
Ettore Di Giacinto
|
53cd640a89 |
test(distributed): add admin session helper to the cluster harness
The register handler answers 201 both for "user created, here is your session" and for "this email already exists", so the status code cannot tell a fresh registration from a repeat one. Key on the session cookie instead and fall through to login when it is absent. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
a7847b8a37 |
test(distributed): make the cluster harness survive a restart
Restarting a frontend replica must not move it: workers read LOCALAI_REGISTER_TO once at boot and never re-resolve it, so a replica that returns on a fresh port is unreachable by the workers that registered with it. startFrontend now takes the port, with <= 0 meaning "allocate". Process logs are opened for append rather than truncated, so a restarted process cannot erase the log of the instance that died, which is the log a failover post-mortem needs. The post-SIGKILL wait is bounded, so one stuck child no longer becomes a suite-wide timeout that names nothing. Stop is nil-safe because Start returns a nil cluster after stopping itself. Start's doc comment no longer claims to wait for worker registration; that needs an authenticated admin session, so it now says callers must poll /api/nodes themselves. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
c0af66a7eb |
test(distributed): add a process-level cluster harness
Runs local-ai as real child processes, one per frontend replica and one per worker, against containerised infrastructure. The in-process suites cannot express frontend-replica failure: there is no process to kill and no real HTTP boundary between a worker and the frontend it registered with. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
53639c4df3 |
test(distributed): scope the log-subscriber wait and mark the race it works around
Three corrections from review of the previous commit. The lock-order comment on SubscriberCount claimed no path takes s.mu and a buffer lock together. Subscribe does exactly that, holding s.mu.RLock across replica registrations that take buf.mu. State the rule that is actually true — s.mu precedes any buffer lock, so counting after releasing it preserves the order — and say what follows from it: the total is a sample, not a snapshot. waitForLogSubscriber read as general-purpose but unblocks on the first registered subscription. Subscribe attaches the exact-key buffer and each replica buffer one at a time, so for a replicated model the count goes positive while later replicas are still unattached and the race survives. Rename it waitForSingleLogSubscriber, document that it holds only where Subscribe resolves to one buffer, and assert on exactly 1: misuse then fails loudly on the count rather than going quietly back to being flaky. Taking the expected count as a parameter was the alternative, but that makes callers predict a store-internal number and an under-count fails the same silent way as the original bug. The snapshot-then-subscribe race had no artifact outside a report, and review found a second site carrying it. Mark both handlers identically, including the point that swapping the two calls duplicates rather than drops and so is not the fix. The race itself is left alone; this branch stays test infrastructure. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
f0fa4a7b1f |
test(distributed): wait for the log subscriber instead of racing it
The WebSocket log handler writes its "initial" batch before it calls Subscribe, so a line appended the instant that batch arrives lands in the circular buffer with no subscriber to receive it. Three backend-logs specs append exactly there and then wait out a 5s read deadline; once a gorilla read hits its deadline the connection is unusable, so the spec cannot retry. `--focus='Worker WebSocket log streaming' --repeat=25` failed on attempt 17 with nothing else running, which is far too often to wire into CI. Add BackendLogStore.SubscriberCount, resolving a model ID by the same exact-key and replica-prefix rules Subscribe uses, and have the specs poll it until the handler has attached. Nothing in production calls it and no assertion is weakened; the handler's own snapshot/subscribe window is left as it is, being a production streaming question rather than a test one. Verified with 60 repeats of the WebSocket specs and three consecutive --randomize-all runs of the whole distributed suite, all at --flake-attempts 1: 239 of 240 specs pass in about 80 seconds. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |
||
|
Ettore Di Giacinto
|
1974bc1ea0 |
test(distributed): stop leaking admin pools when database setup fails
A failed CREATE DATABASE panics out of the assertion before closeDB runs, leaking a pgx pool per attempt. With --flake-attempts 5 that exhausts postgres:16-alpine's 100 connection slots, at which point the cleanup path's own Expect fails the spec and one hiccup cascades across the suite. Scope the admin handle so the panic unwinds through defer closeDB, and let cleanup use a fallible tryAdminDB that reports rather than asserts. Register DeferCleanup immediately after CREATE so a later failure cannot leave the database behind, and warn on TestInfra that the container handles are now suite-wide. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> |