Skip to content

Commit f317da7

Browse files
localai-botmudler
andauthored
fix(galleryop): make admitted operations queryable and survive a failed op (#11044)
Two lifecycle defects observed on a 2-replica distributed cluster. The install endpoints mint a job UUID, hand the operation to an unbuffered channel, and answer HTTP 200 immediately. The gallery worker is a single goroutine that processes operations serially, and the first status write happens inside modelHandler/backendHandler — i.e. only once the worker actually starts the work. An operation queued behind a running install therefore had no status at all: GET /models/jobs/<uuid> answered "could not find any status for ID" and GET /models/jobs did not list it, so the endpoint reported success for work nothing could observe. On the paths that sent directly rather than from a goroutine, the same unbuffered channel blocked the HTTP handler for the whole duration of the in-flight install, which is how a replica came to accept no /models/apply at all while /readyz stayed green. Admission now goes through EnqueueModelOp/EnqueueBackendOp, which publish a "queued" status before handing the operation over, so a job ID is queryable from the instant it is handed out. Delivery selects on the operation's context, so cancelling a still-queued operation releases the delivery goroutine instead of stranding it on a send that will never be received, and an operation the worker never accepts becomes a terminal failure rather than a silent leak. The worker also had no panic containment. A panic in any handler propagated out of the single consumer goroutine and killed the process, taking every queued operation with it; it is now contained to the operation that caused it. The two ignored galleryStore.Create errors are logged, and the model and backend delete endpoints now run under the same ID they hand back — they previously ran under an empty ID and returned a status URL for a job that could never have a status. Second, an operation orphaned by a controller replaced mid-download kept reporting phase=downloading, processed=false, error=none while nothing was downloading. The PostgreSQL side does recover on its own (FindDuplicate ignores rows untouched for 30 minutes and CleanStale marks them failed), but the reaper only ever corrected the database. The in-memory statuses map that GET /models/jobs/<id> and /api/operations actually read was never corrected, so every replica kept serving the frozen tick indefinitely. ReapStaleOperations now reconciles the in-memory copy with the reap. Note that operation ownership is still not tracked: gallery_operations has a FrontendID column that nothing writes, so a live operation and one whose owner died are distinguished only by a 30-minute staleness timeout. Narrowing that window needs a lease/heartbeat mechanism and is out of scope here. Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash] Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Co-authored-by: Ettore Di Giacinto <mudler@localai.io>
1 parent 6cee8de commit f317da7

10 files changed

Lines changed: 440 additions & 54 deletions

File tree

core/http/endpoints/localai/backend.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,12 @@ func (mgs *BackendEndpointService) ApplyBackendEndpoint(systemState *system.Syst
140140
if err != nil {
141141
return err
142142
}
143-
mgs.backendApplier.BackendGalleryChannel <- galleryop.ManagementOp[gallery.GalleryBackend, any]{
143+
mgs.backendApplier.EnqueueBackendOp(galleryop.ManagementOp[gallery.GalleryBackend, any]{
144144
ID: uuid.String(),
145145
GalleryElementName: input.ID,
146146
Galleries: mgs.galleries,
147147
Force: input.Force,
148-
}
148+
})
149149

150150
return c.JSON(200, schema.BackendResponse{ID: uuid.String(), StatusURL: fmt.Sprintf("%sbackends/jobs/%s", middleware.BaseURL(c), uuid.String())})
151151
}
@@ -221,17 +221,21 @@ func (mgs *BackendEndpointService) DeleteBackendEndpoint() echo.HandlerFunc {
221221
return func(c echo.Context) error {
222222
backendName := c.Param("name")
223223

224-
mgs.backendApplier.BackendGalleryChannel <- galleryop.ManagementOp[gallery.GalleryBackend, any]{
225-
Delete: true,
226-
GalleryElementName: backendName,
227-
Galleries: mgs.galleries,
228-
}
229-
230224
uuid, err := uuid.NewUUID()
231225
if err != nil {
232226
return err
233227
}
234228

229+
// The op carries the same ID the caller is handed back: without it the
230+
// deletion ran under an empty ID and StatusURL pointed at a job that
231+
// could never have a status.
232+
mgs.backendApplier.EnqueueBackendOp(galleryop.ManagementOp[gallery.GalleryBackend, any]{
233+
ID: uuid.String(),
234+
Delete: true,
235+
GalleryElementName: backendName,
236+
Galleries: mgs.galleries,
237+
})
238+
235239
return c.JSON(200, schema.BackendResponse{ID: uuid.String(), StatusURL: fmt.Sprintf("%sbackends/jobs/%s", middleware.BaseURL(c), uuid.String())})
236240
}
237241
}
@@ -313,12 +317,12 @@ func (mgs *BackendEndpointService) UpgradeBackendEndpoint() echo.HandlerFunc {
313317
return err
314318
}
315319

316-
mgs.backendApplier.BackendGalleryChannel <- galleryop.ManagementOp[gallery.GalleryBackend, any]{
320+
mgs.backendApplier.EnqueueBackendOp(galleryop.ManagementOp[gallery.GalleryBackend, any]{
317321
ID: uuid.String(),
318322
GalleryElementName: backendName,
319323
Galleries: mgs.galleries,
320324
Upgrade: true,
321-
}
325+
})
322326

323327
return c.JSON(200, schema.BackendResponse{ID: uuid.String(), StatusURL: fmt.Sprintf("%sbackends/jobs/%s", middleware.BaseURL(c), uuid.String())})
324328
}

core/http/endpoints/localai/gallery.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,14 @@ func (mgs *ModelGalleryEndpointService) ApplyModelGalleryEndpoint() echo.Handler
8787
if err != nil {
8888
return err
8989
}
90-
mgs.galleryApplier.ModelGalleryChannel <- galleryop.ManagementOp[gallery.GalleryModel, gallery.ModelConfig]{
90+
mgs.galleryApplier.EnqueueModelOp(galleryop.ManagementOp[gallery.GalleryModel, gallery.ModelConfig]{
9191
Req: input.GalleryModel,
9292
ID: uuid.String(),
9393
GalleryElementName: input.ID,
9494
Variant: input.Variant,
9595
Galleries: mgs.galleries,
9696
BackendGalleries: mgs.backendGalleries,
97-
}
97+
})
9898

9999
return c.JSON(200, schema.GalleryResponse{ID: uuid.String(), StatusURL: fmt.Sprintf("%smodels/jobs/%s", middleware.BaseURL(c), uuid.String())})
100100
}
@@ -110,18 +110,22 @@ func (mgs *ModelGalleryEndpointService) DeleteModelGalleryEndpoint() echo.Handle
110110
return func(c echo.Context) error {
111111
modelName := c.Param("name")
112112

113-
mgs.galleryApplier.ModelGalleryChannel <- galleryop.ManagementOp[gallery.GalleryModel, gallery.ModelConfig]{
114-
Delete: true,
115-
GalleryElementName: modelName,
116-
}
117-
118-
mgs.configLoader.RemoveModelConfig(modelName)
119-
120113
uuid, err := uuid.NewUUID()
121114
if err != nil {
122115
return err
123116
}
124117

118+
// The op carries the same ID the caller is handed back: without it the
119+
// deletion ran under an empty ID and StatusURL pointed at a job that
120+
// could never have a status.
121+
mgs.galleryApplier.EnqueueModelOp(galleryop.ManagementOp[gallery.GalleryModel, gallery.ModelConfig]{
122+
ID: uuid.String(),
123+
Delete: true,
124+
GalleryElementName: modelName,
125+
})
126+
127+
mgs.configLoader.RemoveModelConfig(modelName)
128+
125129
return c.JSON(200, schema.GalleryResponse{ID: uuid.String(), StatusURL: fmt.Sprintf("%smodels/jobs/%s", middleware.BaseURL(c), uuid.String())})
126130
}
127131
}

core/http/endpoints/localai/import_model.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,15 @@ func ImportModelURIEndpoint(cl *config.ModelConfigLoader, appConfig *config.Appl
108108
opcache.Set(galleryID, uuid.String())
109109
}
110110

111-
galleryService.ModelGalleryChannel <- galleryop.ManagementOp[gallery.GalleryModel, gallery.ModelConfig]{
111+
galleryService.EnqueueModelOp(galleryop.ManagementOp[gallery.GalleryModel, gallery.ModelConfig]{
112112
Req: gallery.GalleryModel{
113113
Overrides: map[string]any{},
114114
},
115115
ID: uuid.String(),
116116
GalleryElementName: galleryID,
117117
GalleryElement: &modelConfig,
118118
BackendGalleries: appConfig.BackendGalleries,
119-
}
119+
})
120120

121121
resp.ID = uuid.String()
122122
resp.StatusURL = fmt.Sprintf("%smodels/jobs/%s", httpUtils.BaseURL(c), uuid.String())

core/http/endpoints/localai/nodes.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -521,9 +521,7 @@ func InstallBackendOnNodeEndpoint(_ nodes.NodeCommandSender, galleryService *gal
521521
CancelFunc: cancelFunc,
522522
}
523523
galleryService.StoreCancellation(jobID, cancelFunc)
524-
go func() {
525-
galleryService.BackendGalleryChannel <- op
526-
}()
524+
galleryService.EnqueueBackendOp(op)
527525

528526
xlog.Info("Node-scoped backend install dispatched", "node", nodeID, "backend", req.Backend, "uri", req.URI, "jobID", jobID)
529527
return c.JSON(http.StatusAccepted, map[string]string{
@@ -586,9 +584,7 @@ func UpgradeBackendOnNodeEndpoint(galleryService *galleryop.GalleryService, opca
586584
CancelFunc: cancelFunc,
587585
}
588586
galleryService.StoreCancellation(jobID, cancelFunc)
589-
go func() {
590-
galleryService.BackendGalleryChannel <- op
591-
}()
587+
galleryService.EnqueueBackendOp(op)
592588

593589
xlog.Info("Node-scoped backend upgrade dispatched", "node", nodeID, "backend", req.Backend, "jobID", jobID)
594590
return c.JSON(http.StatusAccepted, map[string]string{

core/http/routes/ui_api.go

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -986,9 +986,7 @@ func RegisterUIAPIRoutes(app *echo.Echo, cl *config.ModelConfigLoader, ml *model
986986
}
987987
// Store cancellation function immediately so queued operations can be cancelled
988988
galleryService.StoreCancellation(uid, cancelFunc)
989-
go func() {
990-
galleryService.ModelGalleryChannel <- op
991-
}()
989+
galleryService.EnqueueModelOp(op)
992990

993991
return c.JSON(200, map[string]any{
994992
"jobID": uid,
@@ -1035,10 +1033,8 @@ func RegisterUIAPIRoutes(app *echo.Echo, cl *config.ModelConfigLoader, ml *model
10351033
}
10361034
// Store cancellation function immediately so queued operations can be cancelled
10371035
galleryService.StoreCancellation(uid, cancelFunc)
1038-
go func() {
1039-
galleryService.ModelGalleryChannel <- op
1040-
cl.RemoveModelConfig(galleryName)
1041-
}()
1036+
galleryService.EnqueueModelOp(op)
1037+
cl.RemoveModelConfig(galleryName)
10421038

10431039
return c.JSON(200, map[string]any{
10441040
"jobID": uid,
@@ -1444,9 +1440,7 @@ func RegisterUIAPIRoutes(app *echo.Echo, cl *config.ModelConfigLoader, ml *model
14441440
}
14451441
// Store cancellation function immediately so queued operations can be cancelled
14461442
galleryService.StoreCancellation(uid, cancelFunc)
1447-
go func() {
1448-
galleryService.BackendGalleryChannel <- op
1449-
}()
1443+
galleryService.EnqueueBackendOp(op)
14501444

14511445
return c.JSON(200, map[string]any{
14521446
"jobID": uid,
@@ -1508,9 +1502,7 @@ func RegisterUIAPIRoutes(app *echo.Echo, cl *config.ModelConfigLoader, ml *model
15081502
}
15091503
// Store cancellation function immediately so queued operations can be cancelled
15101504
galleryService.StoreCancellation(uid, cancelFunc)
1511-
go func() {
1512-
galleryService.BackendGalleryChannel <- op
1513-
}()
1505+
galleryService.EnqueueBackendOp(op)
15141506

15151507
return c.JSON(200, map[string]any{
15161508
"jobID": uid,
@@ -1556,9 +1548,7 @@ func RegisterUIAPIRoutes(app *echo.Echo, cl *config.ModelConfigLoader, ml *model
15561548
}
15571549
// Store cancellation function immediately so queued operations can be cancelled
15581550
galleryService.StoreCancellation(uid, cancelFunc)
1559-
go func() {
1560-
galleryService.BackendGalleryChannel <- op
1561-
}()
1551+
galleryService.EnqueueBackendOp(op)
15621552

15631553
return c.JSON(200, map[string]any{
15641554
"jobID": uid,
@@ -1677,11 +1667,7 @@ func RegisterUIAPIRoutes(app *echo.Echo, cl *config.ModelConfigLoader, ml *model
16771667
}
16781668
// Store cancellation function immediately so queued operations can be cancelled
16791669
galleryService.StoreCancellation(uid, cancelFunc)
1680-
// Non-blocking send — BackendGalleryChannel is unbuffered and a direct
1681-
// send would hang the HTTP handler whenever the worker is busy.
1682-
go func() {
1683-
galleryService.BackendGalleryChannel <- op
1684-
}()
1670+
galleryService.EnqueueBackendOp(op)
16851671

16861672
return c.JSON(200, map[string]any{
16871673
"jobID": uid,

core/services/distributed/gallery.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,20 @@ func (s *GalleryStore) Cancel(id string) error {
203203
return s.UpdateStatus(id, "cancelled", "")
204204
}
205205

206+
// ListStale returns the IDs of in-progress operations that CleanStale would
207+
// reap. Callers need the IDs, not just a count: the reaper corrects the
208+
// database, but each replica also holds an in-memory copy of the operation
209+
// status that the API actually serves, and that copy has to be corrected too
210+
// or the reaped op keeps reporting "downloading" forever.
211+
func (s *GalleryStore) ListStale(age time.Duration) ([]string, error) {
212+
cutoff := time.Now().Add(-age)
213+
var ids []string
214+
err := s.db.Model(&GalleryOperationRecord{}).
215+
Where("updated_at < ? AND status IN ?", cutoff, activeStatuses).
216+
Pluck("id", &ids).Error
217+
return ids, err
218+
}
219+
206220
// CleanStale marks abandoned in-progress operations as failed and returns the
207221
// number of rows reaped. Called on startup AND periodically to recover from
208222
// crashed/restarted instances that left records in pending/downloading/

core/services/galleryop/enqueue.go

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
package galleryop
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
"github.com/mudler/LocalAI/core/gallery"
8+
"github.com/mudler/xlog"
9+
)
10+
11+
// queuedMessage is the status message an operation carries between admission
12+
// (the HTTP handler minting the job ID) and the moment the worker starts it.
13+
const queuedMessage = "queued"
14+
15+
// EnqueueModelOp admits a model operation: it registers a queryable "queued"
16+
// status for op.ID and then hands the op to the gallery worker.
17+
//
18+
// Why the status is written here and not in modelHandler: the gallery channels
19+
// are unbuffered and the worker processes one operation at a time, so an op
20+
// submitted while another install is downloading sits in a blocked send for as
21+
// long as that install takes. The admission handlers return HTTP 200 with the
22+
// job ID immediately, so for that whole window the client held an ID that
23+
// GET /models/jobs/<id> answered with "could not find any status for ID" and
24+
// GET /models/jobs did not list at all — the endpoint reported success for work
25+
// nothing could observe. Writing the status before the send makes the job
26+
// queryable from the instant its ID is handed out.
27+
//
28+
// Delivery is asynchronous on purpose: a direct send would block the HTTP
29+
// handler for the entire duration of the in-flight install (observed as a
30+
// request that never gets a response while /readyz stays green).
31+
func (g *GalleryService) EnqueueModelOp(op ManagementOp[gallery.GalleryModel, gallery.ModelConfig]) {
32+
g.markQueued(op.ID, op.GalleryElementName, op.Delete)
33+
go func() {
34+
select {
35+
case g.ModelGalleryChannel <- op:
36+
case <-enqueueContext(op.Context).Done():
37+
g.abandonQueued(op.ID, op.GalleryElementName)
38+
}
39+
}()
40+
}
41+
42+
// EnqueueBackendOp is the BackendGalleryChannel sibling of EnqueueModelOp.
43+
// Same rationale — see that comment.
44+
func (g *GalleryService) EnqueueBackendOp(op ManagementOp[gallery.GalleryBackend, any]) {
45+
g.markQueued(op.ID, op.GalleryElementName, op.Delete)
46+
go func() {
47+
select {
48+
case g.BackendGalleryChannel <- op:
49+
case <-enqueueContext(op.Context).Done():
50+
g.abandonQueued(op.ID, op.GalleryElementName)
51+
}
52+
}()
53+
}
54+
55+
// enqueueContext gives the delivery goroutine something to select on. Ops
56+
// submitted without a context (the /models/apply path) simply wait for the
57+
// worker; ops that carry one (every UI path) are released the moment the
58+
// operation is cancelled, so cancelling a still-queued op no longer strands
59+
// the goroutine on a send that will never be received.
60+
func enqueueContext(ctx context.Context) context.Context {
61+
if ctx == nil {
62+
return context.Background()
63+
}
64+
return ctx
65+
}
66+
67+
// markQueued publishes the pre-worker status for an admitted operation.
68+
func (g *GalleryService) markQueued(id, elementName string, deletion bool) {
69+
if id == "" {
70+
return
71+
}
72+
g.UpdateStatus(id, &OpStatus{
73+
Message: queuedMessage,
74+
Phase: queuedMessage,
75+
GalleryElementName: elementName,
76+
Deletion: deletion,
77+
Cancellable: !deletion,
78+
})
79+
}
80+
81+
// abandonQueued turns a queued operation that the worker never accepted into a
82+
// terminal failure. Without it the op keeps claiming it is waiting to start
83+
// while the goroutine that was supposed to deliver it is gone, which is the
84+
// same phantom-operation shape as an op orphaned by a replica restart.
85+
//
86+
// A status that already reached a terminal state (a concurrent cancel is the
87+
// common case) wins: we must not overwrite "cancelled" with a generic failure.
88+
func (g *GalleryService) abandonQueued(id, elementName string) {
89+
if id == "" {
90+
return
91+
}
92+
g.Lock()
93+
if st, ok := g.statuses[id]; ok && st.Processed {
94+
g.Unlock()
95+
return
96+
}
97+
g.Unlock()
98+
99+
xlog.Warn("Gallery operation was never picked up by the worker", "op_id", id, "element", elementName)
100+
g.UpdateStatus(id, &OpStatus{
101+
Processed: true,
102+
Error: fmt.Errorf("operation was cancelled before the gallery worker could start it"),
103+
Message: "error: operation was cancelled before the gallery worker could start it",
104+
GalleryElementName: elementName,
105+
})
106+
}

0 commit comments

Comments
 (0)