Repository navigation
DOC-6996 Document the go-redis pipeline connection pool - #3854
Open
andy-stark-redis wants to merge 3 commits into
Open
andy-stark-redis wants to merge 3 commits into
andy-stark-redis wants to merge 3 commits into
Conversation
Contributor
Contributor
Contributor
🧠 Redis MemoryFound 8 related items from repository history (2 new this commit):
Memory updated at 9e30b06 |
7 of 10 tasks
Add a Connection pooling section to the go-redis production usage page covering the dedicated pipeline pool that go-redis PR #3959 makes default-on, and point the two pipeline pages at it. The ticket framed this as expanding the autopipeline paragraph at lines 157-160. That turned out to be the wrong home. Pipeline() and TxPipeline() both route through withPipelineConn (redis.go:1753-1765, 1807), so the new pool serves hand-built pipelines and MULTI/EXEC too, not just automatic pipelining -- and autopipeline.md carries an "experimental feature" banner that would have wrongly colored default-on pooling behavior. produsage.md already owns Timeouts and Retries, so sizing advice belongs there; the Go guide had no pooling prose at all before this. The ticket's "Not parked: the change is already merged upstream" is a merged-vs-released conflation. v9.22.0 shipped 2026-08-03, #3959 merged 2026-08-24, master is 15 commits ahead of the tag and no release contains it. Ran the negative check to be sure rather than reasoning from dates: on v9.22.0, PoolStats().PipelineStats is nil by default AND stays nil with PipelinePoolSize:10 alone, because the released version only builds the pool when a buffer field is set. So the whole section describes behavior no reader can observe yet, and the version line is a deliberate vX.Y.Z placeholder with an HTML TODO. Compiling the PipelineStats snippet was necessary but not sufficient. Running it against master + Redis 8.8 is what confirmed the claims: pipeline pool non-nil and holding zero connections when idle, a plain command touching only the main pool, both Pipeline() and TxPipeline() using the pipeline pool, PipelinePoolSize:-1 making PipelineStats nil, and 60 concurrent pipelines capping the pool at 10. The fallback claim did NOT reproduce on that burst (Timeouts stayed 0, main pool untouched) -- fast batches never hold a connection past the 100ms wait. It took PipelinePoolSize:1 plus BLPOP batches to force it: Timeouts=5 and five batches on the main pool. DEBUG SLEEP is unavailable on the local server, so BLPOP on a never-populated key is the way to hold a pipeline connection open. Also fixed a pre-existing broken anchor in the same checklist: #seamless-client-experience never matched the "Smart client handoffs" heading. Deliberately not done: no connect.md change for client-side caching. Pipeline connections now skip CLIENT TRACKING (redis.go:942-949), but pipelined commands never consulted or populated the cache in v9.22.0 either, so nothing a reader can observe changed. URL query params deferred by decision -- documenting them drags in ParseURL coverage the Go guide has never had. Upstream doc bug worth reporting: osscluster.go:148 still says the pool is created "only when PipelineReadBufferSize or PipelineWriteBufferSize is set", stale after #3959 and contradicting ring.go:154. Cluster nodes are built via clOpt.NewClient() (osscluster.go:518) with PipelinePoolSize passed through, so they do get pipeline pools -- which is also why the ceiling arithmetic multiplies per node. Learned: merged != released, and a saturation claim needs saturation forced -- a 60-pipeline burst never triggered the main-pool fallback that PipelinePoolSize:1 plus BLPOP did. Constraint: the PipelineStats snippet must keep `if ps := stats.PipelineStats; ps != nil` -- the field is *internal/pool.Stats, so type inference is the only form that compiles outside the module, not a style preference. Constraint: produsage.md carries no page-level bannerText; the pipeline pool's version requirement is stated in-section so the released Health checks, Retries and Timeouts sections are not mislabeled as unreleased. Rejected: expanding autopipeline.md lines 157-160 as the ticket suggested | files default-on pooling under that page's experimental-feature banner and leaves hand-built Pipeline()/TxPipeline() readers with nothing Gaps: the 64 KiB buffer defaults and the RESP3 minimum clamp are read from pipelinePoolOptions, not observed at runtime; the Limiter-charged-once behavior is likewise source-only. Recheck: replace the vX.Y.Z placeholder and delete the DOC-6996 HTML comment in produsage.md when a non-prerelease go-redis tag contains bd0cea4. Directive: verify DefaultPipelinePoolSize (10), DefaultPipelineBufferSize (64 KiB) and DefaultPipelinePoolTimeout (100ms) against the shipping tag before merging -- all three are quoted as bare numbers in the prose and table. Ticket: DOC-6996 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Unpark of #3854. The trigger fired when v9.23.0 shipped on 2026-10-05: bd0cea4...v9.23.0 and d907b98 (#4002)...v9.23.0 both show behind_by 0. The park snapshot rated the semantics HIGH because #3959 had been run on master. That was true at the time, but #4002 (full duplex) merged after park and changed two of the claims. Saturation no longer waits up to 100 ms for a pipeline connection: withPipelineConn now uses a non-blocking TryGet and spills to the main pool immediately, and a spill doesn't count as a Timeout. The page's "wait for PoolTimeout, then fall back" paragraph and its "watch PipelineStats.Timeouts" advice were therefore both wrong. DefaultPipelineBufferSize also went from 64 KiB to 128 KiB, as a full-duplex backpressure guardrail. The identifiers, the other two defaults (10 connections, 100 ms), MinIdleConns forced to 0, the MaxActiveConns + PipelinePoolSize ceiling and Limiter-once all held. The page now says spills are immediate and open main-pool connections. It points readers at TotalConns on both pools, because no field counts spills. The new snippet prints misses, not timeouts. Re-ran the runtime probe against v9.23.0 itself, with Redis 7.2.7 standalone plus a 3-node cluster. A plain 60-pipeline burst now spills 50 batches to the main pool with Timeouts=0, where at park time it never spilled. Six concurrent 1 s BLPOP batches on a 1-connection pool all finished in 1.007 s. The cluster check showed 30 pipeline connections across 3 nodes, which settles the per-node claim that was source-only at park. Same commit: the autopipeline MaxBatchBytes default (0 -> 128 KiB in v9.23.0), a new MaxQueuedCommands row (run: 48 of 50 rejected with ErrAutoPipelineQueueFull at limit 2), the version placeholder filled in as v9.23.0 prose instead of a note, and the branch's relref links converted to repo-path links after rebasing onto main. Learned: a merged-and-run HIGH-confidence park snapshot still goes stale when a later PR touches the same function; #4002 reversed the saturation behavior between park and release Rejected: documenting AutoPipelineOptions.FullDuplex here | opt-in feature with hook/Limiter semantics big enough for its own section and ticket; this PR is about the pipeline pool Constraint: no Stats field counts pipeline-pool spills in v9.23.0; don't reintroduce Timeouts-based spill monitoring unless go-redis adds a counter Gaps: buffer-size resolution and the RESP3 clamp are still source-only (pipelinePoolOptions, redis.go:733); Ring per-node pools are source-only (cluster was run) Recheck: if go-redis adds a spill counter to pool.Stats, replace the TotalConns heuristic in produsage.md Ticket: DOC-6996 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
andy-stark-redis
force-pushed
the
DOC-6996-go-redis-pipeline-pool
branch
from
October 6, 2026 08:46
48490fe to
9e30b06
Compare
kaitlynmichael
approved these changes
Oct 9, 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.
Documents the dedicated pipeline connection pool that
go-redis#3959 makes default-on.
Adds a
### Connection poolingsection to the go-redis Production usage page — the Goguide had no pooling prose anywhere before this — and points the two pipeline pages at it:
content/develop/clients/go/produsage.md— new section, plus a checklist entry. Also fixes apre-existing broken anchor in that checklist (
#seamless-client-experiencenever matched the"Smart client handoffs" heading).
content/develop/clients/go/autopipeline.md— the paragraph that listed the three pipelinefields inline now links to the new section.
content/develop/clients/go/transpipe.md— one sentence noting that hand-built pipelines andtransactions use the pool too.
The ticket proposed expanding the autopipeline paragraph instead. That was the wrong home:
Pipeline()andTxPipeline()both route throughwithPipelineConn, so the pool serveshand-built pipelines and MULTI/EXEC as well as automatic pipelining, and
autopipeline.mdcarries an "experimental feature" banner that would have wrongly colored default-on behavior.
Unparked 2026-10-06. go-redis v9.23.0 (released 2026-10-05) contains #3959 and #4002. Reconciled against that tag and re-run against it:
TryGet), and a spill isn't counted inTimeouts. The fallback paragraph and the monitoring advice were rewritten to match.autopipeline.md: theMaxBatchBytesdefault changed from 0 to 128 KiB in v9.23.0, and there's a newMaxQueuedCommandsrow.main; the branch'srelreflinks became repo-path links.AutoPipelineOptions.FullDuplex(new and opt-in in v9.23.0) is deliberately out of scope here. The upstreamosscluster.gocomment saying the pool is created only when a buffer field is set is still stale at v9.23.0 (now line 162).Note
Low Risk
Documentation-only changes with no runtime or security impact; the only caveat is the parked placeholder version until go-redis releases the pipeline pool behavior.
Overview
Adds go-redis production guidance for connection pooling, including the default-on pipeline connection pool from go-redis PR #3959, and wires the pipeline docs to that single section instead of repeating option names inline.
On Production usage (
produsage.md), a new Connection pooling checklist item and section explain the main pool (PoolSize,MinIdleConns,MaxActiveConns,PoolTimeout) and the separate pipeline pool (PipelinePoolSize, buffer sizes), fallback to the main pool under saturation, per-node behavior for cluster/ring, and monitoring viaPoolStats().PipelineStats. A version note uses avX.Y.Zplaceholder until the feature ships in a release. The checklist link for smart client handoffs is corrected from a broken#seamless-client-experienceanchor to#smart-client-handoffs.Automatic pipelining and Pipelines/transactions now briefly state that batches use the pipeline pool and link to the new section rather than listing the three
Pipeline*fields in autopipeline alone.Reviewed by Cursor Bugbot for commit 48490fe. Bugbot is set up for automated code reviews on this repo. Configure here.