fix: release gate reservation on retry to prevent queue wedge (#311) - #334
Merged
Conversation
) 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 llm-d#311 Signed-off-by: Shimi Bandiel <shimib@google.com>
shimib
requested review from
RishabhSaini,
ahg-g,
evacchi and
jtechapps
as code owners
July 21, 2026 18:55
…k fix) Signed-off-by: Shimi Bandiel <shimib@google.com>
jtechapps
approved these changes
Jul 22, 2026
jtechapps
pushed a commit
that referenced
this pull request
Jul 23, 2026
) (#336) * fix: refresh redis-quota concurrency TTL to prevent over-admission (#335) Concurrency-mode redis-quota tracks in-flight requests as an INCR/DECR counter, but set the key's EXPIRE only on the first acquire (new_val==1) and never refreshed it. The TTL equals window (factory default 1m; the multitenant overlays don't set it). Under sustained per-tenant load the counter stays positive, so ~window seconds after it first went positive the key expires mid-flight, the counter resets to 0, and further requests are admitted beyond the limit -- over-admission up to 'limit' extra concurrent per tenant, recurring every window (and for any request in flight longer than window). This is the correctness residual after #311/#334: #334 made the counter accurate (retries no longer leak INCRs), but an accurate-but-positive counter was still wiped by the TTL. - Refresh EXPIRE on every acquire so the counter cannot expire while there is activity; the key now expires only after window of total inactivity (crash-orphan cleanup). - Refresh EXPIRE on release while reservations remain in flight. - Add a regression test (TTL refresh across interleaved acquires) that fails before this change and passes after. Rate-limit mode is unchanged (its sorted set expires per-member correctly). Closes #335 Signed-off-by: Shimi Bandiel <shimib@google.com> * docs(release-notes): add fragment for #336 (redis-quota TTL over-admission fix) Signed-off-by: Shimi Bandiel <shimib@google.com> --------- Signed-off-by: Shimi Bandiel <shimib@google.com>
This was referenced Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What / why
Fixes #311. The Redis sorted-set flow's per-queue gate reservation (a
LocalConcurrencyGateslot or aredis-quotaINCR) was released only byresultWorker(terminal results). The retry path (retryWorker) re-enqueued the request without releasing, so every retry leaked a reservation:LocalConcurrencyGate(no TTL),inFlightratcheted up untilBudget()hit 0 and the queue stopped dispatching permanently;redis-quota, it was masked by the key's TTL reset — which itself over-admits while requests are in flight.Compounded by
activeReleases.Storeoverwriting a prior closure on re-dispatch, orphaning it.Changes (
pkg/redis/sortedset_impl.go)retryWorkernow releases each retried request's reservation before re-enqueue (a re-dispatched request re-reserves viagate.Apply). Releasing beforeflushRetryBatch'sZAddmeans a re-dispatch cannot race with / overwrite the reservation.sync.Map.Swapand releases any prior closure defensively — never orphans a lingering reservation.TestSortedSetFlow_RetryReleasesReservationfor the retry→release path (previously untested — why the leak stayed latent). It fails without this fix and passes with it.Not affected
Pub/Sub holds the reservation in a function-scoped
deferthat runs on every return path includingNack/retry — no analogous leak, so the fix is scoped to the sorted-set flow.Testing
make testgreen across all modules.mainand pass with the fix.Closes #311