Skip to content

Commit 447c186

Browse files
localai-botmudler
andauthored
fix(distributed): make backend upgrade actually re-install on workers (#9708)
* fix(distributed): make backend upgrade actually re-install on workers UpgradeBackend dispatched a vanilla backend.install NATS event to every node hosting the backend. The worker's installBackend short-circuits on "already running for this (model, replica) slot" and returns the existing address — so the gallery install path was skipped, no artifact was re-downloaded, no metadata was written. The frontend's drift detection then re-flagged the same backends every cycle (installedDigest stays empty → mismatch → "Backend upgrade available (new build)") while "Backend upgraded successfully" landed in the logs at the same time. The user-visible symptom: clicking "Upgrade All" silently does nothing and the same N backends sit on the upgrade list forever. Two coupled fixes, one PR: 1. Force flag on backend.install. Add `Force bool` to BackendInstallRequest and thread it through NodeCommandSender -> RemoteUnloaderAdapter. UpgradeBackend (and the reconciler's pending-op drain when retrying an upgrade) sets force=true; routine load events and admin install endpoints keep force=false. On the worker, force=true stops every live process that uses this backend (resolveProcessKeys for peer replicas, plus the exact request processKey), skips the findBackend short-circuit, and passes force=true into gallery.InstallBackendFromGallery so the on-disk artifact is overwritten. After the gallery install completes, startBackend brings up a fresh process at the same processKey on a new port. 2. Liveness check on the fast path. installBackend's "already running" branch read getAddr without verifying the process was alive, so a gRPC backend that died without the supervisor noticing left a stale (key, addr) entry. The reconciler then dialed that address, got ECONNREFUSED, marked the replica failed, retried install — and the supervisor said "already running addr=…" again. Loop forever, exactly what we observed on a node whose llama-cpp process had died but whose supervisor record persisted. Verify s.isRunning(processKey) before trusting getAddr; if the entry is stale, stopBackendExact cleans up and we fall through to a real install. Backwards-compatible: the new Force field is omitempty, older workers ignore it (their default behavior matches force=false). The signature change on NodeCommandSender.InstallBackend is internal-only. Verified: unit tests in core/services/nodes pass (108s suite). The pre-existing core/backend build break (proto regen pending for word-level timestamps) blocks core/cli and core/http/endpoints/localai package tests but is unrelated to this change. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-7 [Claude Code] * test(e2e/distributed): pass force=false to adapter.InstallBackend NodeCommandSender.InstallBackend gained a final force bool in the upgrade-force commit; the e2e distributed lifecycle tests still called the old 8-arg signature and broke compilation. These tests exercise the routine install path (single replica, default behavior), so force=false preserves their existing semantics. Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude:claude-opus-4-7 [Claude Code] --------- Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
1 parent cec5c4f commit 447c186

9 files changed

Lines changed: 107 additions & 27 deletions

File tree

core/cli/worker.go

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -664,10 +664,19 @@ func buildProcessKey(modelID, backend string, replicaIndex int) string {
664664
}
665665

666666
// installBackend handles the backend.install flow:
667-
// 1. If already running for this (model, replica) slot, return existing address
668-
// 2. Install backend from gallery (if not already installed)
669-
// 3. Find backend binary
670-
// 4. Start gRPC process on a new port
667+
// 1. If already running for this (model, replica) slot AND req.Force is false,
668+
// return existing address (the fast path used by routine load events that
669+
// just want to know which port a backend already serves on).
670+
// 2. If req.Force is true, stop any process(es) currently using this backend
671+
// so the gallery install can replace the on-disk artifact and the freshly
672+
// started process picks up the new binary. This is the upgrade path —
673+
// without it, every backend.install we receive after the first hits the
674+
// fast path and silently no-ops, leaving the cluster on a stale build.
675+
// 3. Install backend from gallery (force=req.Force so existing artifacts get
676+
// overwritten on upgrade).
677+
// 4. Find backend binary
678+
// 5. Start gRPC process on a new port
679+
//
671680
// Returns the gRPC address of the backend process.
672681
//
673682
// ProcessKey includes the replica index so a worker with MaxReplicasPerModel>1
@@ -677,10 +686,40 @@ func buildProcessKey(modelID, backend string, replicaIndex int) string {
677686
func (s *backendSupervisor) installBackend(req messaging.BackendInstallRequest) (string, error) {
678687
processKey := buildProcessKey(req.ModelID, req.Backend, int(req.ReplicaIndex))
679688

680-
// If already running for this model+replica, return its address
681-
if addr := s.getAddr(processKey); addr != "" {
682-
xlog.Info("Backend already running for model replica", "backend", req.Backend, "model", req.ModelID, "replica", req.ReplicaIndex, "addr", addr)
683-
return addr, nil
689+
if !req.Force {
690+
// Fast path: already running for this model+replica → return existing
691+
// address. Verify liveness before trusting the cached entry: a process
692+
// that died without the supervisor noticing leaves a stale (key, addr)
693+
// pair, and getAddr would otherwise hand the controller an address
694+
// that immediately ECONNREFUSEDs. The reconciler then marks the
695+
// replica failed, retries the install, the supervisor says "already
696+
// running" again, and the cluster loops on a dead replica forever.
697+
if addr := s.getAddr(processKey); addr != "" {
698+
if s.isRunning(processKey) {
699+
xlog.Info("Backend already running for model replica", "backend", req.Backend, "model", req.ModelID, "replica", req.ReplicaIndex, "addr", addr)
700+
return addr, nil
701+
}
702+
xlog.Warn("Stale process entry for backend (dead process); cleaning up before reinstall",
703+
"backend", req.Backend, "model", req.ModelID, "replica", req.ReplicaIndex, "addr", addr)
704+
s.stopBackendExact(processKey)
705+
}
706+
} else {
707+
// Upgrade path: stop every live process that shares this backend so the
708+
// gallery install can overwrite the on-disk artifact and the restarted
709+
// process picks up the new binary. resolveProcessKeys catches peer
710+
// replicas of the same backend (whisper#0, whisper#1, ...) on workers
711+
// configured with MaxReplicasPerModel>1. We also stop the exact
712+
// processKey from the request tuple — keys created with an explicit
713+
// modelID don't share the bare-name prefix the resolver matches, but
714+
// they're still using the old binary and need to come down. Both calls
715+
// are no-ops on missing keys.
716+
toStop := s.resolveProcessKeys(req.Backend)
717+
toStop = append(toStop, processKey)
718+
for _, key := range toStop {
719+
xlog.Info("Force install: stopping running backend before reinstall",
720+
"backend", req.Backend, "processKey", key)
721+
s.stopBackendExact(key)
722+
}
684723
}
685724

686725
// Parse galleries from request (override local config if provided)
@@ -692,20 +731,26 @@ func (s *backendSupervisor) installBackend(req messaging.BackendInstallRequest)
692731
}
693732
}
694733

695-
// Try to find the backend binary
696-
backendPath := s.findBackend(req.Backend)
734+
// On upgrade, run the gallery install path even if the binary already
735+
// exists on disk: findBackend would otherwise short-circuit and we'd
736+
// restart the same stale binary. The force flag passed to
737+
// InstallBackendFromGallery makes it overwrite the existing artifact.
738+
backendPath := ""
739+
if !req.Force {
740+
backendPath = s.findBackend(req.Backend)
741+
}
697742
if backendPath == "" {
698743
if req.URI != "" {
699-
xlog.Info("Backend not found locally, attempting external install", "backend", req.Backend, "uri", req.URI)
744+
xlog.Info("Installing backend from external URI", "backend", req.Backend, "uri", req.URI, "force", req.Force)
700745
if err := galleryop.InstallExternalBackend(
701746
context.Background(), galleries, s.systemState, s.ml, nil, req.URI, req.Name, req.Alias,
702747
); err != nil {
703748
return "", fmt.Errorf("installing backend from gallery: %w", err)
704749
}
705750
} else {
706-
xlog.Info("Backend not found locally, attempting gallery install", "backend", req.Backend)
751+
xlog.Info("Installing backend from gallery", "backend", req.Backend, "force", req.Force)
707752
if err := gallery.InstallBackendFromGallery(
708-
context.Background(), galleries, s.systemState, s.ml, req.Backend, nil, false,
753+
context.Background(), galleries, s.systemState, s.ml, req.Backend, nil, req.Force,
709754
); err != nil {
710755
return "", fmt.Errorf("installing backend from gallery: %w", err)
711756
}

core/http/endpoints/localai/nodes.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,10 @@ func InstallBackendOnNodeEndpoint(unloader nodes.NodeCommandSender) echo.Handler
407407
}
408408
// Admin-driven backend install: not tied to a specific replica slot
409409
// (no model is being loaded). Pass replica 0 to match the worker's
410-
// admin process-key convention (`backend#0`).
411-
reply, err := unloader.InstallBackend(nodeID, req.Backend, "", req.BackendGalleries, req.URI, req.Name, req.Alias, 0)
410+
// admin process-key convention (`backend#0`). force=false so the
411+
// worker's fast path takes over if the backend is already running —
412+
// upgrades go through the dedicated /api/backends/upgrade path.
413+
reply, err := unloader.InstallBackend(nodeID, req.Backend, "", req.BackendGalleries, req.URI, req.Name, req.Alias, 0, false)
412414
if err != nil {
413415
xlog.Error("Failed to install backend on node", "node", nodeID, "backend", req.Backend, "uri", req.URI, "error", err)
414416
return c.JSON(http.StatusInternalServerError, nodeError(http.StatusInternalServerError, "failed to install backend on node"))

core/services/messaging/subjects.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,14 @@ type BackendInstallRequest struct {
137137
// (single-replica behavior — no collision because the controller never
138138
// asks for replica > 0 on a node whose MaxReplicasPerModel is 1).
139139
ReplicaIndex int32 `json:"replica_index,omitempty"`
140+
// Force skips the "already running" short-circuit and re-runs the gallery
141+
// install. UpgradeBackend sets this so the worker actually re-downloads the
142+
// artifact, stops the live process, and starts a fresh one — without it,
143+
// the install handler's early return makes upgrades a silent no-op while
144+
// the coordinator's drift detection keeps re-flagging the backend forever.
145+
// Older workers that don't know this field treat it as false (current
146+
// behavior preserved).
147+
Force bool `json:"force,omitempty"`
140148
}
141149

142150
// BackendInstallReply is the response from a backend.install NATS request.

core/services/nodes/managers_distributed.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ func (d *DistributedBackendManager) InstallBackend(ctx context.Context, op *gall
339339
// Admin-driven backend install: not tied to a specific replica slot.
340340
// Pass replica 0 — the worker's processKey is "backend#0" when no
341341
// modelID is supplied, matching pre-PR4 behavior.
342-
reply, err := d.adapter.InstallBackend(node.ID, backendName, "", string(galleriesJSON), op.ExternalURI, op.ExternalName, op.ExternalAlias, 0)
342+
reply, err := d.adapter.InstallBackend(node.ID, backendName, "", string(galleriesJSON), op.ExternalURI, op.ExternalName, op.ExternalAlias, 0, false)
343343
if err != nil {
344344
return err
345345
}
@@ -360,6 +360,12 @@ func (d *DistributedBackendManager) InstallBackend(ctx context.Context, op *gall
360360
// would ask workers to "upgrade" something they never had, which fails at
361361
// the gallery (e.g. a darwin/arm64 worker has no platform variant for a
362362
// linux-only backend) and leaves a forever-retrying pending_backend_ops row.
363+
//
364+
// force=true on the install call is what distinguishes upgrade from install:
365+
// the worker stops the live process for this backend, overwrites the on-disk
366+
// artifact, and restarts. Without it, the worker's "already running" fast
367+
// path turns every backend.install into a no-op and the gallery's drift
368+
// detection never converges.
363369
func (d *DistributedBackendManager) UpgradeBackend(ctx context.Context, name string, progressCb galleryop.ProgressCallback) error {
364370
galleriesJSON, _ := json.Marshal(d.backendGalleries)
365371

@@ -377,7 +383,7 @@ func (d *DistributedBackendManager) UpgradeBackend(ctx context.Context, name str
377383
}
378384

379385
result, err := d.enqueueAndDrainBackendOp(ctx, OpBackendUpgrade, name, galleriesJSON, targetNodeIDs, func(node BackendNode) error {
380-
reply, err := d.adapter.InstallBackend(node.ID, name, "", string(galleriesJSON), "", "", "", 0)
386+
reply, err := d.adapter.InstallBackend(node.ID, name, "", string(galleriesJSON), "", "", "", 0, true)
381387
if err != nil {
382388
return err
383389
}

core/services/nodes/reconciler.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,12 @@ func (rc *ReplicaReconciler) drainPendingBackendOps(ctx context.Context) {
189189
_, applyErr = rc.adapter.DeleteBackend(op.NodeID, op.Backend)
190190
case OpBackendInstall, OpBackendUpgrade:
191191
// Pending-op drain for admin install/upgrade — not a per-replica
192-
// load. Replica 0 is the conventional admin slot.
193-
reply, err := rc.adapter.InstallBackend(op.NodeID, op.Backend, "", string(op.Galleries), "", "", "", 0)
192+
// load. Replica 0 is the conventional admin slot. Upgrade ops set
193+
// force=true so the worker reinstalls the artifact and restarts
194+
// the live process; install ops keep the existing fast-path
195+
// semantics for the case where the backend is already running.
196+
force := op.Op == OpBackendUpgrade
197+
reply, err := rc.adapter.InstallBackend(op.NodeID, op.Backend, "", string(op.Galleries), "", "", "", 0, force)
194198
if err != nil {
195199
applyErr = err
196200
} else if !reply.Success {

core/services/nodes/router.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -673,7 +673,10 @@ func (r *SmartRouter) installBackendOnNode(ctx context.Context, node *BackendNod
673673
return "", fmt.Errorf("no NATS connection for backend installation")
674674
}
675675

676-
reply, err := r.unloader.InstallBackend(node.ID, backendType, modelID, r.galleriesJSON, "", "", "", replicaIndex)
676+
// force=false: routine load, the worker's fast-path "already running →
677+
// return current address" is correct here. Upgrades go through
678+
// DistributedBackendManager.UpgradeBackend which sets force=true.
679+
reply, err := r.unloader.InstallBackend(node.ID, backendType, modelID, r.galleriesJSON, "", "", "", replicaIndex, false)
677680
if err != nil {
678681
return "", err
679682
}

core/services/nodes/router_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,11 @@ type installCall struct {
307307
backend string
308308
modelID string
309309
replica int
310+
force bool
310311
}
311312

312-
func (f *fakeUnloader) InstallBackend(nodeID, backend, modelID, _, _, _, _ string, replica int) (*messaging.BackendInstallReply, error) {
313-
f.installCalls = append(f.installCalls, installCall{nodeID, backend, modelID, replica})
313+
func (f *fakeUnloader) InstallBackend(nodeID, backend, modelID, _, _, _, _ string, replica int, force bool) (*messaging.BackendInstallReply, error) {
314+
f.installCalls = append(f.installCalls, installCall{nodeID, backend, modelID, replica, force})
314315
return f.installReply, f.installErr
315316
}
316317

core/services/nodes/unloader.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,13 @@ type backendStopRequest struct {
1616

1717
// NodeCommandSender abstracts NATS-based commands to worker nodes.
1818
// Used by HTTP endpoint handlers to avoid coupling to the concrete RemoteUnloaderAdapter.
19+
//
20+
// The `force` parameter on InstallBackend is set by the upgrade path to make
21+
// the worker re-run the gallery install (overwriting the on-disk artifact) and
22+
// restart any live process for that backend. Routine installs and load events
23+
// pass force=false so an already-running process short-circuits as before.
1924
type NodeCommandSender interface {
20-
InstallBackend(nodeID, backendType, modelID, galleriesJSON, uri, name, alias string, replicaIndex int) (*messaging.BackendInstallReply, error)
25+
InstallBackend(nodeID, backendType, modelID, galleriesJSON, uri, name, alias string, replicaIndex int, force bool) (*messaging.BackendInstallReply, error)
2126
DeleteBackend(nodeID, backendName string) (*messaging.BackendDeleteReply, error)
2227
ListBackends(nodeID string) (*messaging.BackendListReply, error)
2328
StopBackend(nodeID, backend string) error
@@ -77,10 +82,15 @@ func (a *RemoteUnloaderAdapter) UnloadRemoteModel(modelName string) error {
7782
// process key — distinct slots run on distinct ports so multiple replicas of
7883
// the same model can coexist on a fat node. Pass 0 for single-replica.
7984
//
85+
// force=true is the upgrade path: the worker stops any live process for this
86+
// backend, overwrites the on-disk artifact via gallery install, and restarts.
87+
// Routine installs and load events pass force=false to keep the existing
88+
// "already running → return current address" fast path.
89+
//
8090
// Timeout: 5 minutes (gallery install can take a while).
81-
func (a *RemoteUnloaderAdapter) InstallBackend(nodeID, backendType, modelID, galleriesJSON, uri, name, alias string, replicaIndex int) (*messaging.BackendInstallReply, error) {
91+
func (a *RemoteUnloaderAdapter) InstallBackend(nodeID, backendType, modelID, galleriesJSON, uri, name, alias string, replicaIndex int, force bool) (*messaging.BackendInstallReply, error) {
8292
subject := messaging.SubjectNodeBackendInstall(nodeID)
83-
xlog.Info("Sending NATS backend.install", "nodeID", nodeID, "backend", backendType, "modelID", modelID, "replica", replicaIndex)
93+
xlog.Info("Sending NATS backend.install", "nodeID", nodeID, "backend", backendType, "modelID", modelID, "replica", replicaIndex, "force", force)
8494

8595
return messaging.RequestJSON[messaging.BackendInstallRequest, messaging.BackendInstallReply](a.nats, subject, messaging.BackendInstallRequest{
8696
Backend: backendType,
@@ -90,6 +100,7 @@ func (a *RemoteUnloaderAdapter) InstallBackend(nodeID, backendType, modelID, gal
90100
Name: name,
91101
Alias: alias,
92102
ReplicaIndex: int32(replicaIndex),
103+
Force: force,
93104
}, 5*time.Minute)
94105
}
95106

tests/e2e/distributed/node_lifecycle_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ var _ = Describe("Node Backend Lifecycle (NATS-driven)", Label("Distributed"), f
5757
FlushNATS(infra.NC)
5858

5959
adapter := nodes.NewRemoteUnloaderAdapter(registry, infra.NC)
60-
installReply, err := adapter.InstallBackend(node.ID, "llama-cpp", "", "", "", "", "", 0)
60+
installReply, err := adapter.InstallBackend(node.ID, "llama-cpp", "", "", "", "", "", 0, false)
6161
Expect(err).ToNot(HaveOccurred())
6262
Expect(installReply.Success).To(BeTrue())
6363
})
@@ -78,7 +78,7 @@ var _ = Describe("Node Backend Lifecycle (NATS-driven)", Label("Distributed"), f
7878
FlushNATS(infra.NC)
7979

8080
adapter := nodes.NewRemoteUnloaderAdapter(registry, infra.NC)
81-
installReply, err := adapter.InstallBackend(node.ID, "nonexistent", "", "", "", "", "", 0)
81+
installReply, err := adapter.InstallBackend(node.ID, "nonexistent", "", "", "", "", "", 0, false)
8282
Expect(err).ToNot(HaveOccurred())
8383
Expect(installReply.Success).To(BeFalse())
8484
Expect(installReply.Error).To(ContainSubstring("backend not found"))

0 commit comments

Comments
 (0)