Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5a8b808
fix(pipeline): PipelinePoolSize alone should create the pipeline pool
ndyakov Aug 10, 2026
42e36ae
feat(pipeline): give autopipelined clients a dedicated pipeline pool …
ndyakov Aug 10, 2026
0b4e32d
feat(pipeline): 64KiB default buffers and no pre-dial for the pipelin…
ndyakov Aug 10, 2026
ab82c0e
test: make pipeline-conn specs aware of the dedicated pipeline pool
ndyakov Aug 10, 2026
577f3ca
test: warm the pipeline-pool conn in AP tests that instrument connect…
ndyakov Aug 10, 2026
b20bc4a
docs: TODO(v10) — ClientTracking* and ClientMaintNotifications are st…
ndyakov Aug 10, 2026
b28fa58
docs: drop the TODO(v10) notes on the per-connection commands
ndyakov Aug 10, 2026
6ed3372
feat(pipeline): always create the pipeline pool; spill on exhaustion
ndyakov Aug 10, 2026
d4242a4
test: keep HImport mock choreography on the main pool; doc the pool d…
ndyakov Aug 10, 2026
86ed0a0
test: skip conn-init handshakes in the cluster shard pipeline hooks
ndyakov Aug 10, 2026
5683f5f
test: adapt plain-pipeline tests to the always-on pipeline pool
ndyakov Aug 10, 2026
1cb41b4
Merge branch 'master' into ndyakov/fix-pipeline-pool-size-gate
ndyakov Aug 10, 2026
cb8f9a2
fix(pipeline): address #3959 review (spill, caps, UniversalOptions)
ndyakov Aug 13, 2026
dad8cbf
fix(pipeline): review follow-ups (docs, de-flake, init-spill, stats)
ndyakov Aug 13, 2026
19cdc58
fix(pipeline): review follow-ups (docs, de-flake, init-spill, stats)
ndyakov Aug 14, 2026
65a2ef6
fix(pipeline): accept pipeline pool settings in URL options
ndyakov Aug 14, 2026
a1605d3
ci(govulncheck): use stable Go to pick up security patches
ndyakov Aug 14, 2026
0f73f75
fix(pool): size maintnotif defaults for the pipeline pool too
ndyakov Aug 15, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions autopipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1874,8 +1874,24 @@ func TestAutoPipelineRetriesOnNetworkError(t *testing.T) {
}
defer ap.Close()

// Arm after the handshake: the pooled conn is healthy, so it passes the
// pool health check, and the batch's first write dies on the wire.
// Warm the DEDICATED PIPELINE POOL connection first: creating the
// autopipeliner created the pipeline pool (lazily), and BATCHES run there,
// not on the main-pool conn the Ping dialed. Without this warm-up the
// batch's first dispatch dials a fresh pipeline conn and the dial count
// reads one high for a reason unrelated to the retry. The warm-up must be
// a real multi-command batch: a lone command takes the solo fast path
// (Process on the main pool) and would not touch the pipeline pool.
w1 := ap.Set(ctx, "apr:warm", 1, 0)
w2 := ap.Incr(ctx, "apr:warm2")
if err := w1.Err(); err != nil {
t.Fatal(err)
}
if err := w2.Err(); err != nil {
t.Fatal(err)
}

// Arm after the handshake: the pipeline-pool conn is healthy, so it passes
// the pool health check, and the batch's first write dies on the wire.
dialsBefore := dials.Load()
failNextWrite.Store(true)

Expand Down Expand Up @@ -3927,7 +3943,6 @@ func TestAutoPipelineHookPostNextErrorPartialBatch(t *testing.T) {
if err := c.Set(ctx, "pnp:present", "v", 0).Err(); err != nil {
t.Fatal(err)
}
c.AddHook(postNextErrorHook{err: errInjected})

// Wide flush window so all three commands deterministically land in ONE
// pipeline batch (the rule is per-batch: hooks fire per batch).
Expand All @@ -3940,6 +3955,23 @@ func TestAutoPipelineHookPostNextErrorPartialBatch(t *testing.T) {
}
defer ap.Close()

// Initialize the dedicated pipeline-pool connection BEFORE installing the
// error-injecting hook: connection init runs its handshake pipeline through
// the client's hook chain (newConn shares hooksMixin), so a hook that
// unconditionally injects an error would fail the pipeline conn's init and
// this test would measure init poisoning instead of the post-next rule.
// Must be a real multi-command batch — a lone command takes the solo fast
// path (Process on the main pool) and would not init the pipeline conn.
w1 := ap.Set(ctx, "pnp:warm", 1, 0)
w2 := ap.Set(ctx, "pnp:warm2", 1, 0)
if err := w1.Err(); err != nil {
t.Fatal(err)
}
if err := w2.Err(); err != nil {
t.Fatal(err)
}
c.AddHook(postNextErrorHook{err: errInjected})

runWithWatchdog(t, 30*time.Second, func() {
// One batch: a hit, a miss (redis.Nil), and a write.
hit := ap.Get(ctx, "pnp:present")
Expand Down
38 changes: 31 additions & 7 deletions commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,27 @@ var _ = Describe("Commands", func() {
Expect(cmds[0].Err().Error()).To(authErr)

stats := rawClient.PoolStats()
Expect(stats.Hits).To(Equal(uint32(2)))
Expect(stats.Misses).To(Equal(uint32(1)))
Expect(stats.Timeouts).To(Equal(uint32(0)))
Expect(stats.TotalConns).To(Equal(uint32(1)))
Expect(stats.IdleConns).To(Equal(uint32(1)))
if stats.PipelineStats != nil {
// The autopipeline subject faces create the dedicated pipeline
// pool lazily, so the two Pipelined calls above ran there: the
// first dialed (miss), the second reused (hit). The main pool
// served only the BeforeEach FlushDB.
Expect(stats.PipelineStats.Hits).To(Equal(uint32(1)))
Expect(stats.PipelineStats.Misses).To(Equal(uint32(1)))
Expect(stats.PipelineStats.Timeouts).To(Equal(uint32(0)))
Expect(stats.PipelineStats.TotalConns).To(Equal(uint32(1)))
Expect(stats.PipelineStats.IdleConns).To(Equal(uint32(1)))
Expect(stats.Hits).To(Equal(uint32(0)))
Expect(stats.Misses).To(Equal(uint32(1)))
} else {
// No dedicated pipeline pool: FlushDB dialed the one connection
// (miss) and both Pipelined calls reused it (hits).
Expect(stats.Hits).To(Equal(uint32(2)))
Expect(stats.Misses).To(Equal(uint32(1)))
Expect(stats.Timeouts).To(Equal(uint32(0)))
Expect(stats.TotalConns).To(Equal(uint32(1)))
Expect(stats.IdleConns).To(Equal(uint32(1)))
}
})

It("should hello", func() {
Expand Down Expand Up @@ -393,8 +409,16 @@ var _ = Describe("Commands", func() {
}()
pipe.ClientSetInfo(ctx, libInfo)
}).To(Panic())
// Test setting the default options for libName, libName suffix and libVer
clientInfo := rawClient.ClientInfo(ctx).Val()
// Test setting the default options for libName, libName suffix and libVer.
// CLIENT SETINFO is per-connection state, so read CLIENT INFO through the
// same pipeline path that issued the SETINFOs above: when the client has a
// dedicated pipeline pool (the autopipeline subject faces create one
// lazily), those ran on a pipeline-pool connection and rawClient.ClientInfo
// would inspect a different, main-pool connection.
infoCmd := pipe.ClientInfo(ctx)
_, err = pipe.Exec(ctx)
Expect(err).NotTo(HaveOccurred())
clientInfo := infoCmd.Val()
Expect(clientInfo.LibName).To(ContainSubstring("go-redis(go-redis,"))
// Test setting the libName suffix in options
opt := redisOptions()
Expand Down
36 changes: 33 additions & 3 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,27 @@ type Options struct {
PipelineWriteBufferSize int

// PipelinePoolSize is the pool size for the separate pipeline connection pool.
// Only used if PipelineReadBufferSize or PipelineWriteBufferSize is set.
// Setting this alone is enough to create the dedicated pipeline pool; the
// pipeline buffer sizes then default to ReadBufferSize/WriteBufferSize.
Comment thread
ndyakov marked this conversation as resolved.
Outdated
//
// Pipelining typically needs fewer connections than regular operations because
// batching reduces connection contention. A smaller pool saves memory while
// maintaining high throughput.
//
// If not set (0), defaults to 10 connections.
// The dedicated pipeline pool is created unconditionally at NewClient —
// like the pubsub pool — so pipelines never compete with regular commands
// for main-pool connections. It never pre-dials (MinIdleConns is forced
// to 0 on it), so the size is a cap on burst capacity, not a standing
// footprint: an unused pipeline pool holds zero connections. A burst of
// concurrent pipelines wider than the cap spills back to the main pool
// instead of queueing. Its connections use DefaultPipelineBufferSize
Comment thread
ndyakov marked this conversation as resolved.
Outdated
// buffers unless the pipeline buffer sizes are set explicitly.
//
// default: 10
// Set to a negative value to opt out of the dedicated pool entirely:
// pipelines then run on the main pool, as they did before the pool
// existed.
//
// default: 0 (= DefaultPipelinePoolSize connections)
Comment thread
ndyakov marked this conversation as resolved.
Outdated
PipelinePoolSize int
Comment thread
ndyakov marked this conversation as resolved.

// AutoPipelineOptions is the default config for BOTH autopipeliner faces:
Expand Down Expand Up @@ -452,6 +464,23 @@ const (
CSCStrategySharedTracking CSCStrategy = iota
)

// DefaultPipelinePoolSize is the pipeline pool size used when
// PipelinePoolSize is not set. Pipelining batches many commands per round
// trip, so it needs far fewer connections than regular traffic. The pool is
// pure burst capacity: it never pre-dials idle connections (MinIdleConns is
// forced to 0 on it), so an unused pipeline pool holds no connections at all
// and the size is only a cap — bursts wider than it spill to the main pool.
const DefaultPipelinePoolSize = 10

// DefaultPipelineBufferSize is the per-connection read/write buffer size for
// the dedicated pipeline pool when no explicit pipeline buffer size is set
// (the larger of this and the regular buffer size is used). Pipeline
// connections move whole batches per round trip, so they earn bigger buffers
// than regular per-command traffic: measured on the autopipeline engine,
// throughput plateaus around 64 KiB and gains nothing past ~128 KiB, while
// very large buffers (>=512 KiB) can regress it.
const DefaultPipelineBufferSize = 64 * 1024

func (opt *Options) init() {
if opt.Addr == "" {
opt.Addr = "localhost:6379"
Expand Down Expand Up @@ -497,6 +526,7 @@ func (opt *Options) init() {
if opt.PoolSize == 0 {
opt.PoolSize = 10 * runtime.GOMAXPROCS(0)
}

if opt.MaxConcurrentDials <= 0 {
opt.MaxConcurrentDials = opt.PoolSize
} else if opt.MaxConcurrentDials > opt.PoolSize {
Expand Down
Loading
Loading