Skip to content

Commit 225da13

Browse files
committed
fix(openresponses): make responses visible and cancellable across replicas
In distributed mode the Open Responses store is process-local: a sync.OnceValue over a map behind an RWMutex. With several frontend replicas behind a round-robin load balancer, every request that lands on a replica other than the creator misses. Measured on a live 2-replica cluster (#10993): the same response id returns 200 on the creating replica and 404 on its peer, and a cancel on the peer returns 404 without ever invoking CancelFunc, so generation runs to completion on the other replica while the caller is told the response does not exist. previous_response_id chaining fails through the same lookup. Split the state by what can actually cross a process boundary: - Replicated: response metadata (request, response resource, owner, expiry, stream/background flags) via syncstate.SyncedMap, the same component finetune, quantization and agent tasks already use. A local miss in Get/FindItem now falls back to it and returns a read-only remote view, so polling and chaining resolve on any replica. - Delegated: cancellation. context.CancelFunc is a function pointer and exists only in the creating process, so a cancel that lands elsewhere is broadcast on responses.<id>.cancel and applied by whichever replica holds the function. The broadcast is fire-and-forget rather than request/reply: if the owner crashed or was scaled down nobody answers, and the handler must not block on a reply that will never come. The replicated status moves to cancelled either way, which is truthful, since a dead owner's generation died with its process. - Refused: streaming resume. The resume buffer is a byte log plus a live notification channel and cannot be replicated without shipping every token over the bus. A resume that reaches the wrong replica now returns HTTP 409 naming the owning replica via the new ErrResponseNotLocal, instead of an empty event list that looks like a finished stream. It is deliberately distinct from ErrOffsetLost, which means the owner's buffer evicted the requested events. Standalone deployments never call EnableDistributed and keep exactly the previous process-local behaviour. Fixes #10993 Signed-off-by: Ettore Di Giacinto <mudler@localai.io> Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash]
1 parent 2f33ad6 commit 225da13

7 files changed

Lines changed: 770 additions & 30 deletions

File tree

core/http/endpoints/openresponses/responses.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3047,11 +3047,26 @@ func GetResponseEndpoint() func(c echo.Context) error {
30473047

30483048
// handleStreamResume handles resuming a streaming response from a specific sequence number
30493049
func handleStreamResume(c echo.Context, store *ResponseStore, responseID string, stored *StoredResponse, startingAfter int) error {
3050+
// The resume buffer is process-local by design (see ErrResponseNotLocal), so
3051+
// a resume that the load balancer routed to a replica which is not
3052+
// generating this response cannot be served here. Say that plainly: a
3053+
// truthful error lets the caller retry against the right replica, whereas
3054+
// replaying an empty buffer would look like a stream that simply ended.
3055+
if stored.Remote {
3056+
return sendOpenResponsesError(c, 409, "invalid_request_error",
3057+
fmt.Sprintf("response %s is being generated on another replica (%s) and its stream cannot be resumed here; poll GET /v1/responses/%s instead, or route the resume with session affinity", responseID, stored.OwnerReplica, responseID),
3058+
"stream")
3059+
}
3060+
30503061
// Fetch buffered events before committing to an SSE response so an
30513062
// offset-lost gap can be reported as a clean HTTP status rather than a
30523063
// silently truncated event stream.
30533064
events, err := store.GetEventsAfter(responseID, startingAfter)
30543065
if err != nil {
3066+
if errors.Is(err, ErrResponseNotLocal) {
3067+
return sendOpenResponsesError(c, 409, "invalid_request_error",
3068+
fmt.Sprintf("response %s is being generated on another replica and its stream cannot be resumed here", responseID), "stream")
3069+
}
30553070
if errors.Is(err, ErrOffsetLost) {
30563071
return sendOpenResponsesError(c, 409, "invalid_request_error", fmt.Sprintf("starting_after=%d is older than the oldest retained event; the resume buffer evicted those events and the stream cannot be resumed from that point", startingAfter), "starting_after")
30573072
}

core/http/endpoints/openresponses/store.go

Lines changed: 163 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"time"
1010

1111
"github.com/mudler/LocalAI/core/schema"
12+
"github.com/mudler/LocalAI/core/services/messaging"
13+
"github.com/mudler/LocalAI/core/services/syncstate"
1214
"github.com/mudler/xlog"
1315
)
1416

@@ -49,6 +51,16 @@ type ResponseStore struct {
4951
// them. A value <= 0 disables that particular cap.
5052
maxStreamEvents int
5153
maxStreamBytes int
54+
55+
// Cross-replica replication. All zero unless EnableDistributed was called
56+
// (see sync.go), which is how a standalone deployment keeps exactly the
57+
// previous process-local behaviour. Guarded by mu.
58+
synced *syncstate.SyncedMap[string, *syncedResponse]
59+
nats messaging.MessagingClient
60+
cancelSub messaging.Subscription
61+
replicaID string
62+
lifeCtx context.Context
63+
lifeCancel context.CancelFunc
5264
}
5365

5466
// StreamedEvent represents a buffered SSE event for streaming resume
@@ -80,6 +92,15 @@ type StoredResponse struct {
8092
EventsChan chan struct{} // Signals new events for live subscribers
8193
mu sync.RWMutex // Protect concurrent access to this response
8294

95+
// Remote marks a read-only view materialised from another replica's
96+
// replicated metadata rather than from this process's own map. Such a view
97+
// has no CancelFunc, no EventsChan and no resume buffer, so every path that
98+
// needs one of those must check it and either delegate over the bus
99+
// (cancel) or refuse with ErrResponseNotLocal (stream resume).
100+
// OwnerReplica names the replica that does hold them.
101+
Remote bool
102+
OwnerReplica string
103+
83104
// streamBytes tracks the total serialized size of the events currently
84105
// retained in StreamEvents, used to enforce the byte cap. droppedThrough
85106
// is the highest sequence number evicted from the front of the buffer
@@ -144,7 +165,6 @@ func NewResponseStore(ttl time.Duration) *ResponseStore {
144165
// Store stores a response with its request and items
145166
func (s *ResponseStore) Store(responseID string, request *schema.OpenResponsesRequest, response *schema.ORResponseResource) {
146167
s.mu.Lock()
147-
defer s.mu.Unlock()
148168

149169
// Build item index for quick lookup
150170
items := make(map[string]*schema.ORItemField)
@@ -171,26 +191,40 @@ func (s *ResponseStore) Store(responseID string, request *schema.OpenResponsesRe
171191
}
172192

173193
s.responses[responseID] = stored
194+
s.mu.Unlock()
195+
196+
// Replicate outside the lock: the broadcast can be delivered synchronously
197+
// and a subscriber re-enters the store.
198+
s.mirror(responseID, stored)
174199
xlog.Debug("Stored Open Responses response", "response_id", responseID, "items_count", len(items))
175200
}
176201

177-
// Get retrieves a stored response by ID
202+
// Get retrieves a stored response by ID.
203+
//
204+
// In distributed mode a miss in this process's map is not proof the response
205+
// does not exist: a round-robin load balancer routes polls and
206+
// previous_response_id lookups to any replica, not the one that created it. So
207+
// a local miss falls back to the replicated metadata and returns a read-only
208+
// remote view (issue #10993). Only a miss in both is a real "not found".
178209
func (s *ResponseStore) Get(responseID string) (*StoredResponse, error) {
179210
s.mu.RLock()
180-
defer s.mu.RUnlock()
181-
182211
stored, exists := s.responses[responseID]
183-
if !exists {
184-
return nil, fmt.Errorf("response not found: %s", responseID)
212+
s.mu.RUnlock()
213+
214+
if exists {
215+
// Check expiration
216+
if stored.ExpiresAt != nil && time.Now().After(*stored.ExpiresAt) {
217+
// Expired, but we'll return it anyway and let caller handle cleanup
218+
return nil, fmt.Errorf("response expired: %s", responseID)
219+
}
220+
return stored, nil
185221
}
186222

187-
// Check expiration
188-
if stored.ExpiresAt != nil && time.Now().After(*stored.ExpiresAt) {
189-
// Expired, but we'll return it anyway and let caller handle cleanup
190-
return nil, fmt.Errorf("response expired: %s", responseID)
223+
if remote, ok := s.remoteGet(responseID); ok {
224+
return remote, nil
191225
}
192226

193-
return stored, nil
227+
return nil, fmt.Errorf("response not found: %s", responseID)
194228
}
195229

196230
// GetItem retrieves a specific item from a stored response
@@ -211,29 +245,61 @@ func (s *ResponseStore) GetItem(responseID, itemID string) (*schema.ORItemField,
211245
// FindItem searches for an item across all stored responses
212246
// Returns the item and the response ID it was found in
213247
func (s *ResponseStore) FindItem(itemID string) (*schema.ORItemField, string, error) {
248+
now := time.Now()
249+
250+
if item, responseID, found := s.findItemLocal(itemID, now); found {
251+
return item, responseID, nil
252+
}
253+
254+
// An item referenced by ID can belong to a response created on any replica,
255+
// so the local sweep alone reproduces the same coin-flip lookup #10993
256+
// describes for GET /v1/responses/{id}.
257+
if m := s.syncMap(); m != nil {
258+
for responseID, v := range m.Snapshot() {
259+
if v == nil || v.Response == nil {
260+
continue
261+
}
262+
if v.ExpiresAt != nil && now.After(*v.ExpiresAt) {
263+
continue
264+
}
265+
for i := range v.Response.Output {
266+
if v.Response.Output[i].ID == itemID {
267+
return &v.Response.Output[i], responseID, nil
268+
}
269+
}
270+
}
271+
}
272+
273+
return nil, "", fmt.Errorf("item not found in any stored response: %s", itemID)
274+
}
275+
276+
// findItemLocal sweeps only this process's own responses. Split out so the
277+
// replicated sweep in FindItem runs with s.mu released - the SyncedMap is a
278+
// separate component with its own lock and must not be entered under ours.
279+
func (s *ResponseStore) findItemLocal(itemID string, now time.Time) (*schema.ORItemField, string, bool) {
214280
s.mu.RLock()
215281
defer s.mu.RUnlock()
216282

217-
now := time.Now()
218283
for responseID, stored := range s.responses {
219284
// Skip expired responses
220285
if stored.ExpiresAt != nil && now.After(*stored.ExpiresAt) {
221286
continue
222287
}
223288

224289
if item, exists := stored.Items[itemID]; exists {
225-
return item, responseID, nil
290+
return item, responseID, true
226291
}
227292
}
228-
229-
return nil, "", fmt.Errorf("item not found in any stored response: %s", itemID)
293+
return nil, "", false
230294
}
231295

232296
// Delete removes a response from storage
233297
func (s *ResponseStore) Delete(responseID string) {
234298
s.mu.Lock()
235-
defer s.mu.Unlock()
236299
delete(s.responses, responseID)
300+
s.mu.Unlock()
301+
302+
s.unmirror(responseID)
237303
xlog.Debug("Deleted Open Responses response", "response_id", responseID)
238304
}
239305

@@ -244,22 +310,33 @@ func (s *ResponseStore) Cleanup() int {
244310
}
245311

246312
s.mu.Lock()
247-
defer s.mu.Unlock()
248-
249313
now := time.Now()
250-
count := 0
314+
expired := []string{}
251315
for id, stored := range s.responses {
252316
if stored.ExpiresAt != nil && now.After(*stored.ExpiresAt) {
253317
delete(s.responses, id)
254-
count++
318+
expired = append(expired, id)
319+
}
320+
}
321+
s.mu.Unlock()
322+
323+
// Reap replicated entries too, including those whose owner never got to
324+
// expire them because it was scaled down mid-flight. This covers the
325+
// locally-expired IDs as well, since they were mirrored with the same
326+
// ExpiresAt. Any replica may do this; the delete broadcast is idempotent.
327+
if m := s.syncMap(); m != nil {
328+
for id, v := range m.Snapshot() {
329+
if v != nil && v.ExpiresAt != nil && now.After(*v.ExpiresAt) {
330+
s.unmirror(id)
331+
}
255332
}
256333
}
257334

258-
if count > 0 {
259-
xlog.Debug("Cleaned up expired Open Responses", "count", count)
335+
if len(expired) > 0 {
336+
xlog.Debug("Cleaned up expired Open Responses", "count", len(expired))
260337
}
261338

262-
return count
339+
return len(expired)
263340
}
264341

265342
// cleanupLoop runs periodic cleanup of expired responses
@@ -292,7 +369,6 @@ func (s *ResponseStore) Count() int {
292369
// StoreBackground stores a background response with cancel function and optional streaming support
293370
func (s *ResponseStore) StoreBackground(responseID string, request *schema.OpenResponsesRequest, response *schema.ORResponseResource, cancelFunc context.CancelFunc, streamEnabled bool) {
294371
s.mu.Lock()
295-
defer s.mu.Unlock()
296372

297373
// Build item index for quick lookup
298374
items := make(map[string]*schema.ORItemField)
@@ -324,6 +400,11 @@ func (s *ResponseStore) StoreBackground(responseID string, request *schema.OpenR
324400
}
325401

326402
s.responses[responseID] = stored
403+
s.mu.Unlock()
404+
405+
// Only the metadata crosses the bus. CancelFunc and the resume buffer stay
406+
// here, which is what makes this replica the owner for cancel and resume.
407+
s.mirror(responseID, stored)
327408
xlog.Debug("Stored background Open Responses response", "response_id", responseID, "stream_enabled", streamEnabled)
328409
}
329410

@@ -338,10 +419,13 @@ func (s *ResponseStore) UpdateStatus(responseID string, status string, completed
338419
}
339420

340421
stored.mu.Lock()
341-
defer stored.mu.Unlock()
342-
343422
stored.Response.Status = status
344423
stored.Response.CompletedAt = completedAt
424+
stored.mu.Unlock()
425+
426+
// Peers poll this response too, so every status transition has to be
427+
// republished or their view stays stuck at "queued" forever.
428+
s.mirror(responseID, stored)
345429

346430
xlog.Debug("Updated response status", "response_id", responseID, "status", status)
347431
return nil
@@ -358,7 +442,6 @@ func (s *ResponseStore) UpdateResponse(responseID string, response *schema.ORRes
358442
}
359443

360444
stored.mu.Lock()
361-
defer stored.mu.Unlock()
362445

363446
// Rebuild item index
364447
items := make(map[string]*schema.ORItemField)
@@ -371,6 +454,10 @@ func (s *ResponseStore) UpdateResponse(responseID string, response *schema.ORRes
371454

372455
stored.Response = response
373456
stored.Items = items
457+
stored.mu.Unlock()
458+
459+
// The final output is what a peer's poll must return, so replicate it.
460+
s.mirror(responseID, stored)
374461

375462
xlog.Debug("Updated response", "response_id", responseID, "status", response.Status, "items_count", len(items))
376463
return nil
@@ -436,6 +523,13 @@ func (s *ResponseStore) GetEventsAfter(responseID string, startingAfter int) ([]
436523
s.mu.RUnlock()
437524

438525
if !exists {
526+
// The response may be alive on a peer, whose resume buffer is not
527+
// replicated. Say so explicitly instead of reporting "not found" (which
528+
// invites the client to give up on a live stream) or an empty slice
529+
// (which looks like a finished one).
530+
if _, remote := s.remoteGet(responseID); remote {
531+
return nil, ErrResponseNotLocal
532+
}
439533
return nil, fmt.Errorf("response not found: %s", responseID)
440534
}
441535

@@ -460,16 +554,41 @@ func (s *ResponseStore) GetEventsAfter(responseID string, startingAfter int) ([]
460554
return result, nil
461555
}
462556

463-
// Cancel cancels a background response if it's still in progress
557+
// Cancel cancels a background response if it's still in progress.
558+
//
559+
// The context.CancelFunc that actually stops generation is a function pointer
560+
// and only exists in the process that created the response. When the cancel
561+
// lands on any other replica - which a round-robin load balancer makes roughly
562+
// as likely as landing on the right one - it is delegated to the owner over the
563+
// bus instead of being reported as a 404 while generation keeps running
564+
// (issue #10993).
464565
func (s *ResponseStore) Cancel(responseID string) (*schema.ORResponseResource, error) {
465566
s.mu.RLock()
466567
stored, exists := s.responses[responseID]
467568
s.mu.RUnlock()
468569

469-
if !exists {
470-
return nil, fmt.Errorf("response not found: %s", responseID)
570+
if exists {
571+
response, err := s.cancelLocal(responseID, stored)
572+
if err != nil {
573+
return nil, err
574+
}
575+
s.mirror(responseID, stored)
576+
return response, nil
577+
}
578+
579+
if m := s.syncMap(); m != nil {
580+
if v, ok := m.Get(responseID); ok && v != nil {
581+
return s.delegateCancel(v)
582+
}
471583
}
472584

585+
return nil, fmt.Errorf("response not found: %s", responseID)
586+
}
587+
588+
// cancelLocal cancels a response this process owns. Shared by the direct HTTP
589+
// path and by the delegated path a peer triggers over the bus, so both go
590+
// through exactly the same terminal-state and CancelFunc handling.
591+
func (s *ResponseStore) cancelLocal(responseID string, stored *StoredResponse) (*schema.ORResponseResource, error) {
473592
stored.mu.Lock()
474593
defer stored.mu.Unlock()
475594

@@ -502,6 +621,11 @@ func (s *ResponseStore) GetEventsChan(responseID string) (chan struct{}, error)
502621
s.mu.RUnlock()
503622

504623
if !exists {
624+
// A live subscriber channel cannot be handed across processes; see
625+
// ErrResponseNotLocal.
626+
if _, remote := s.remoteGet(responseID); remote {
627+
return nil, ErrResponseNotLocal
628+
}
505629
return nil, fmt.Errorf("response not found: %s", responseID)
506630
}
507631

@@ -515,6 +639,11 @@ func (s *ResponseStore) IsStreamEnabled(responseID string) (bool, error) {
515639
s.mu.RUnlock()
516640

517641
if !exists {
642+
// Stream-enabled is replicated metadata, so a peer can answer this one
643+
// even though it cannot serve the buffer itself.
644+
if remote, ok := s.remoteGet(responseID); ok {
645+
return remote.StreamEnabled, nil
646+
}
518647
return false, fmt.Errorf("response not found: %s", responseID)
519648
}
520649

@@ -541,6 +670,10 @@ func (s *ResponseStore) SetOwner(responseID, owner string) {
541670
}
542671

543672
stored.Owner = owner
673+
674+
// Owner is part of the replicated metadata: without it a peer would treat
675+
// the response as ownerless and skip the access check in accessAllowed.
676+
s.mirror(responseID, stored)
544677
}
545678

546679
// accessAllowed reports whether a caller identified by callerID may read or

0 commit comments

Comments
 (0)