Skip to content

Commit e54ce53

Browse files
authored
fix: release gate reservation on retry to prevent queue wedge (#311) (#334)
* fix: release gate reservation on retry to prevent queue wedge (#311) The queue-gate reservation stored in RedisSortedSetFlow.activeReleases was released only by resultWorker (terminal results). The retry path (retryWorker) re-enqueued the request without releasing, so every retry leaked a reservation: inFlight ratcheted up until Budget() reached 0 and the queue stopped dispatching entirely (LocalConcurrencyGate, no TTL), or the redis-quota counter over-admitted when its key TTL reset while requests were still in flight. - retryWorker now releases each retried request's reservation before re-enqueue (a re-dispatched request re-reserves via gate.Apply), ordered before flushRetryBatch's ZAdd so the reservation cannot be overwritten/orphaned. - Dispatch-time store uses sync.Map.Swap and releases any prior closure defensively, so a lingering reservation is never orphaned. - Add a regression test for the retry->release path (previously untested, which is why the leak stayed latent). Pub/Sub is unaffected: it holds the reservation in a function-scoped defer that runs on every return path including Nack/retry. Closes #311 Signed-off-by: Shimi Bandiel <shimib@google.com> * docs(release-notes): add fragment for #334 (gate reservation leak fix) Signed-off-by: Shimi Bandiel <shimib@google.com> --------- Signed-off-by: Shimi Bandiel <shimib@google.com>
1 parent 68b46e8 commit e54ce53

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

pkg/redis/sortedset_impl.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,13 @@ func (r *RedisSortedSetFlow) processMessages(ctx context.Context, msgChannel cha
524524
}
525525

526526
if len(releases) > 0 {
527-
r.activeReleases.Store(rview.ReqID(), releases)
527+
// Defensive: never orphan a lingering reservation for this id — release
528+
// any prior closure instead of silently overwriting it (see #311).
529+
if prev, loaded := r.activeReleases.Swap(rview.ReqID(), releases); loaded {
530+
if rels, ok := prev.([]pipeline.GateReleaseFunc); ok {
531+
pipeline.ReleaseGateReleases(rels)
532+
}
533+
}
528534
}
529535

530536
// Stamp ingestion time as the message enters the in-process buffer so the
@@ -572,6 +578,23 @@ func (r *RedisSortedSetFlow) parseMessage(z redis.Z, logger logr.Logger) (*api.I
572578
func (r *RedisSortedSetFlow) retryWorker(ctx context.Context) {
573579
processMsg := func(processCtx context.Context, msg pipeline.RetryMessage) {
574580
batch := drainBatch(msg, r.retryChannel, maxBatchSize)
581+
// #311: a retried request returns to the queue and is re-gated (and thus
582+
// re-reserved) on its next dispatch, so its current gate reservation must
583+
// be released here — otherwise inFlight ratchets up on every retry until
584+
// Budget() reaches 0 and the queue stops dispatching entirely. Release
585+
// BEFORE re-enqueue: a re-dispatch can only occur after flushRetryBatch's
586+
// ZAdd, so releasing first prevents the reservation from being overwritten
587+
// and orphaned.
588+
for _, m := range batch {
589+
if m.InternalRequest == nil || m.PublicRequest == nil {
590+
continue
591+
}
592+
if val, ok := r.activeReleases.LoadAndDelete(m.PublicRequest.ReqID()); ok {
593+
if rels, ok := val.([]pipeline.GateReleaseFunc); ok {
594+
pipeline.ReleaseGateReleases(rels)
595+
}
596+
}
597+
}
575598
r.flushRetryBatch(processCtx, batch)
576599
}
577600

pkg/redis/sortedset_impl_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,59 @@ func TestSortedSetFlow_RetryBackoff(t *testing.T) {
495495
}
496496
}
497497

498+
// TestSortedSetFlow_RetryReleasesReservation is a regression test for #311: a
499+
// retried request returns to the queue and is re-gated (re-reserved) on its next
500+
// dispatch, so the retry path must release its current queue-gate reservation.
501+
// Before the fix, only resultWorker released reservations, so every retry leaked
502+
// one — inFlight ratcheted up until Budget() hit 0 and the queue wedged.
503+
func TestSortedSetFlow_RetryReleasesReservation(t *testing.T) {
504+
s, rdb, ctx, cancel := setupTest(t)
505+
defer s.Close()
506+
defer rdb.Close() // nolint:errcheck
507+
defer cancel()
508+
509+
const reqID = "retry-release-1"
510+
queue := "retry-release-queue"
511+
512+
flow := &RedisSortedSetFlow{
513+
rdb: rdb,
514+
retryChannel: make(chan pipeline.RetryMessage, 1),
515+
pollInterval: 50 * time.Millisecond,
516+
batchSize: 10,
517+
gate: noopGate(),
518+
}
519+
520+
// Simulate a dispatched request holding a gate reservation (as the dequeue
521+
// loop stores after gate.Apply).
522+
releasedCh := make(chan struct{}, 1)
523+
flow.activeReleases.Store(reqID, []pipeline.GateReleaseFunc{
524+
func() { releasedCh <- struct{}{} },
525+
})
526+
527+
go flow.retryWorker(ctx)
528+
529+
flow.retryChannel <- pipeline.RetryMessage{
530+
EmbelishedRequestMessage: pipeline.EmbelishedRequestMessage{
531+
InternalRequest: api.NewInternalRequest(
532+
api.InternalRouting{RetryCount: 1, RequestQueueName: queue},
533+
&api.RequestMessage{ID: reqID, Created: time.Now().Unix(), Deadline: 9999999999},
534+
),
535+
},
536+
BackoffDurationSeconds: 0,
537+
}
538+
539+
select {
540+
case <-releasedCh:
541+
// Reservation released on the retry path — correct.
542+
case <-time.After(2 * time.Second):
543+
t.Fatal("retry did not release the queue-gate reservation (#311 leak)")
544+
}
545+
546+
if _, held := flow.activeReleases.Load(reqID); held {
547+
t.Error("reservation still present in activeReleases after retry")
548+
}
549+
}
550+
498551
func TestSortedSetFlow_ResultFIFO(t *testing.T) {
499552
s, rdb, ctx, cancel := setupTest(t)
500553
defer s.Close()

release-notes.d/unreleased/334.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
pr: 334
3+
url: https://github.com/llm-d/llm-d-async/pull/334
4+
author: shimib
5+
date: 2026-07-21
6+
---
7+
Fixed a gate capacity reservation leak in the Redis sorted-set flow: retried requests never released their per-queue gate reservation, so `inFlight` ratcheted up on every retry until the queue stopped dispatching entirely (`local-max-concurrency`) or the `redis-quota` counter over-admitted on TTL reset. Retries now release the reservation before re-enqueue (and re-reserve on re-dispatch).

0 commit comments

Comments
 (0)