Skip to content

redis: retire canceled pooled connections after black-holed replies - #1255

Draft
NVGreg wants to merge 1 commit into
envoyproxy:mainfrom
NVGreg:redis-close-on-cancel-20260928
Draft

NVGreg wants to merge 1 commit into
envoyproxy:mainfrom
NVGreg:redis-close-on-cancel-20260928

Conversation

@NVGreg

@NVGreg NVGreg commented Sep 28, 2026

Copy link
Copy Markdown

Outcome

Add opt-in REDIS_CLOSE_ON_CANCEL and REDIS_PERSECOND_CLOSE_ON_CANCEL settings (both default false). When a Redis command's context is canceled, the wrapper closes its pooled socket before Radix can remain blocked discarding a late reply. This covers application commands, pool maintenance PING, and Redis Cluster topology checks. A bounded startup attempt also covers single/cluster initialization and pooled AUTH/READONLY bootstrap.

Why

With a Redis peer that accepts writes but never replies, Radix v4.1.4 can clear its read deadline and wait indefinitely while discarding the canceled response. Later commands sharing that connection can remain stuck. REDIS_TIMEOUT is a TCP dial timeout and does not bound established-socket I/O.

This change needs a deadline or cancellation on the command context. #1241 adds a service request deadline and admission controls for deployments whose callers do not supply one. This PR is independent of #1241; both need joint Stage qualification for the intended recovery path.

Verification

  • go test ./... passed on this branch.
  • go test -race ./src/redis -run '^TestExperiment' -count=1 -timeout=40s passed.
  • Local net.Pipe and loopback tests cover withheld replies, blocked writes, AUTH/READONLY bootstrap, TLS and write buffering, pre-canceled contexts, healthy reuse, and collateral failure of a concurrent shared command.
  • A separate local integration branch with service: bound concurrent rate-limit requests #1241 reproduced the admission-slot hang without this option and passed request, maintenance PING, and Cluster Sync black-hole recovery tests with it. Those service tests are not in this standalone PR.
  • Local Apple M4 Pro stub microbenchmark (two runs): default 2127–2158 ns/op, 5349 B/op, 32 allocs/op; enabled 2442–2560 ns/op, 5893 B/op, 39 allocs/op. This isolates callback overhead; it is not a production throughput result.

Rollout limits

This is a Draft for review and Stage testing. Closing a shared socket can fail other commands in flight even when their own deadlines have not expired. Frequent cancellations may raise reconnect, CPU, and GC load; commands without a deadline remain unbounded. Sentinel control-connection bootstrap is outside this wrapper. Before enabling the flags in production, measure request errors, connection churn, throughput, and latency at representative Stage load, including with #1241 enabled. No deployment or runtime qualification is claimed here.

Signed-off-by: Gregory Giecold <ggiecold@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant