Repository navigation
Conversation
KartikYZ
force-pushed
the
unblock-on-blocking-cancel
branch
8 times, most recently
from
October 6, 2026 20:08
c7a9f01 to
007646e
Compare
When a blocking command (ex. BLPOP) on a pooled connection is aborted by its context being canceled or exceeding its deadline, the connection is closed today, so workers polling with short timeouts reconnect on every timeout. With the new opt-in ClientOption.UnblockOnCancel, send CLIENT UNBLOCK <id> for the aborted connection over the shared pipeline to the same node instead, and return the connection to the pool if the server replies 1. Any other reply closes the connection as before. - mux.blocking: unblock only for blocking cmds aborted by a context error on a still-healthy wire whose client id is known from HELLO. - pipe.Do: with UnblockOnCancel, route blocking cmds with a deadline through the queue, so the deadline aborts the call without closing the conn. - pipe.Do: release blcksig when the abandoned blocking reply arrives, so a recycled pipe resumes keepalive pings and isn't closed by CleanSubscriptions. This is intentionally not gated on the option: without it, an aborted blocking cmd leaked blcksig forever, which only went unnoticed because the pooled conn was closed. The leak is still observable on a DedicatedClient, whose conn kept skipping pings and was closed on release even after the late reply was consumed. It only applies to blocking commands sent with Do, or with ToPipe() when DisableAutoPipelining is set. It does not cover DoMulti or DedicatedClient, requires Redis 6+ (for the client id from HELLO) and the permission to run CLIENT UNBLOCK, and must not be enabled behind proxies or load balancers that may route connections to different servers. These are documented on the option. Refs redis#897 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Run BLPOPs with random deadlines from concurrent workers on a smaller BlockingPool, mixed with SET/GET of unique values, so that unblocked conns are reused across workers. Any reply other than a context or network timeout, or the exact pushed value, means a late reply of an aborted cmd reached another caller. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
KartikYZ
force-pushed
the
unblock-on-blocking-cancel
branch
from
October 6, 2026 20:24
007646e to
5fa013a
Compare
KartikYZ
marked this pull request as ready for review
October 6, 2026 21:45
rueian
reviewed
Oct 10, 2026
| // It is not compatible with DedicatedClient or proxies that may route connections to different servers as | ||
| // CLIENT UNBLOCK may reach another server and unblock an unrelated client. | ||
| // Ref: https://github.com/redis/rueidis/issues/897 | ||
| UnblockOnCancel bool |
Collaborator
There was a problem hiding this comment.
Hi @KartikYZ, thanks for the PR! Most of the implementations look good to me. One thing I am not sure is whether should we have this global UnblockOnCancel option. Could we try to enable this behavior per command?
Author
There was a problem hiding this comment.
Hi @rueian, thank you for your review! Is the suggestion that we have client options scoped to enabling each command in the @\blocking ACL? Will revise to do so.
Collaborator
There was a problem hiding this comment.
No. I am thinking something similar to the ToPipe() modifier that changes the behavior at per command level.
Author
There was a problem hiding this comment.
Ah understood! That makes sense. Revising to do so.
Author
There was a problem hiding this comment.
Switched from a ClientOption to a per-command modifier in the latest commit, thanks.
Author
There was a problem hiding this comment.
Also added a safeguard to clear the flag for dedicated clients, following the precedent with ClearStaticTTL.
…ientOption Replace ClientOption.UnblockOnCancel with Completed.UnblockOnCancel(), so unblocking a canceled blocking command is opted into per invocation, like ToPipe(). Commands without the modifier keep closing the connection when canceled. Co-authored-by: Cursor <cursoragent@cursor.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 9ee20fa. Configure here.
A DedicatedClient never sends CLIENT UNBLOCK, so a flagged blocking cmd aborted by its deadline left the dedicated connection open and blocked, making later commands on it wait until the blocking cmd returned. Clear the flag in the dedicated clients' Do, like ClearStaticTTL, so the connection is closed on cancel as before. Co-authored-by: Cursor <cursoragent@cursor.com>
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
2 participants
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.
Implements the suggestion in #897 to reuse connections for cancelled blocking calls using toPipe() when auto-pipelining is disabled.
Commands can opt into this behavior with
.UnblockOnCancel(), e.g.client.Do(ctx, client.B().Blpop().Key("q").Timeout(0).Build().ToPipe().UnblockOnCancel())Note: the
blcksigrelease inpipe.Do's abort path is independent ofUnblockOnCancel. At present, an aborted blocking command doesn't releaseblcksig. For pooled connections this seems fine because the connection is closed, but on aDedicatedClientthe connection keeps skipping keepalive pings and is closed on release even after the late reply was consumed. Releasing it when the reply later arrives is safe regardless of the modifier, since the server has finished the command by then. Added a test for this,TestBlockingCancelReleasesBlcksig_Do.Note
Medium Risk
Changes blocking-connection lifecycle, async unblock, and pipe blocking-state handling; misconfiguration or proxy routing could leave connections stuck or unblock the wrong client.
Overview
Adds an opt-in
.UnblockOnCancel()command modifier so canceled or deadline-expired blockingDocalls (e.g.BLPOP) canCLIENT UNBLOCKthe connection and return it to the pool instead of closing it.mux.blockingruns unblock asynchronously when the error iscontext.Canceled/DeadlineExceeded, the command is blocking and flagged, and the wire still has a Redis client id fromHELLO; it closes the wire if unblock does not return success.pipe.DoroutesUnblockOnCancelblocking commands through the queued path even when the context has a deadline, and the abort goroutine decrementsblcksigwhen the late blocking reply is consumed.Dedicated clients (standalone and cluster) call
ClearUnblockOnCancelbeforeDo, so they keep the prior behavior of closing the connection on cancel. Documented constraints: Redis 6+,CLIENT UNBLOCKpermission, mainlyDowith auto-pipelining disabled andToPipe(); not forDoMultior proxies that may routeCLIENT UNBLOCKto another server.Reviewed by Cursor Bugbot for commit 2724ca4. Bugbot is set up for automated code reviews on this repo. Configure here.