chore(pipeline): Create pipeline pool even without specific buffer passed by ndyakov · Pull Request #3959 · redis/go-redis

ndyakov · 2026-08-10T12:00:50Z

Dedicated pipeline pool by default

Pipelines no longer share the main connection pool: NewClient and
NewFailoverClient always create a separate pipeline pool (like the pub/sub
pool), so pipelined bursts stop competing with per-command traffic for the same
connections.

Pool resolution (`pipelinePoolOptions`)

Buffers: PipelineReadBufferSize/PipelineWriteBufferSize when set;
otherwise the larger of the regular size and DefaultPipelineBufferSize
(pipelines move whole batches per round trip, so their connections earn
bigger buffers). The RESP3 minimum clamp applies.
Size: PipelinePoolSize when set, DefaultPipelinePoolSize otherwise.
PipelinePoolSize: -1 opts out (pipelines use the main pool as before).
MinIdleConns forced to 0 — burst capacity dials on demand; nothing is
pre-warmed (no doubled idle footprint).
MaxActiveConns deliberately NOT inherited — inheriting would double the
socket ceiling; the effective ceiling is MaxActiveConns + PipelinePoolSize,
a small bounded addition, and the main pool still enforces the cap.
Short PoolTimeout (DefaultPipelinePoolTimeout, 100ms; a caller's
shorter setting is honored): when the pipeline pool is saturated, the
pipeline spills to the main pool after the short wait instead of queueing
— the Limiter is charged once per operation across the spill
(report-before-release ordering preserved).

Config surface

PipelinePoolSize, PipelineReadBufferSize, PipelineWriteBufferSize on
Options, mirrored in UniversalOptions, and accepted as URL query params
(pipeline_pool_size, pipeline_read_buffer_size, pipeline_write_buffer_size)
by ParseURL/ParseClusterURL/ParseFailoverURL.

Ring and Cluster PoolStats fold every shard's pipeline pool into the
aggregate (PipelineStats), so pipeline-only workloads stay visible.

Note

Medium Risk
Default behavior change: all pipelines/autopipelines now use a separate pool unless opted out, affecting connection counts, pool stats, and saturation/spill under load; mitigated by lazy dialing, bounded spill, and extensive tests.

Overview
Pipelines get their own connection pool by default (like pub/sub), so batched traffic no longer contends with single-command work on the main pool. PipelinePoolSize: -1 restores the old “pipelines on main pool” behavior.

Pool setup is centralized in pipelinePoolOptions / buildPipelinePool: default 64 KiB pipeline buffers, 10 connections, no MinIdleConns, no inherited MaxActiveConns, and a 100ms pipeline PoolTimeout. When the pipeline pool is saturated, withPipelineConn spills to the main pool without double-counting the Limiter. Pipeline-pool connections skip CLIENT TRACKING for client-side cache.

Config & observability: pipeline pool knobs are wired through UniversalOptions and URL parsers (pipeline_pool_size, etc.). Cluster and Ring PoolStats aggregate per-shard pipeline stats into PipelineStats. Maint-notification defaults account for the extra pipeline connections.

Tests and CI are updated for lazy pipeline-pool dials (warm-up batches, PipelinePoolSize: -1 where sequences are pinned to the main pool) and new regression tests cover spill, limiter accounting, and pool creation rules.

^{Reviewed by Cursor Bugbot for commit 0f73f75. Bugbot is set up for automated code reviews on this repo. Configure here.}

The dedicated pipeline connection pool was created only when PipelineReadBufferSize or PipelineWriteBufferSize was set. Setting just PipelinePoolSize therefore produced no pipeline pool at all, with no error and no log line, and every pipelined operation silently kept using the main connection pool — competing with ordinary commands for the same connections, which is the opposite of what the option asks for. The gating was inverted. The pool size is the primary intent ("give pipelines their own N connections"); the buffer sizes are tuning. And the buffer sizes already supplied a default for the size (10), so the dependency only ran one way. Any of the three options now creates the pool. Enabling it by size inherits the regular ReadBufferSize/WriteBufferSize, so a caller who only wants to bound pipeline connections no longer has to know that a buffer size is secretly required. Found while measuring client-side caching over a high-latency link: the CSC background batched reads go through withPipelineConn, which falls back to the regular pool when none exists, so they were contending with the very reader misses they exist to prevent. Tests cover all four option combinations and were verified to fail with the gate reverted.

…by default Autopipelining is pipeline-heavy by definition, yet the dedicated pipeline pool still had to be enabled separately; without it every batch competes with regular commands for main-pool connections. Two paths now default it in: Config time: Options.init defaults PipelinePoolSize to DefaultPipelinePoolSize (new exported const, 10) when AutoPipelineOptions is set. Cluster node clients do not inherit AutoPipelineOptions (the cluster autopipeliner lives at cluster level and dispatches into the node clients' pipeline hooks), so clientOptions applies the same default there. Sentinel and failover node clients inherit AutoPipelineOptions and are covered by Options.init; Ring does not support autopipelining. A negative PipelinePoolSize opts out explicitly. Runtime: the common pattern configures the autopipeliner after NewClient (AutoPipelineWithOptions on a plain client), where pool creation has already happened. ensurePipelinePool now builds the pool lazily when an autopipeliner is created. baseClient.pipelinePool becomes a pointer to an atomic slot (pool + pool-name ref) so the lazy publication is race-free against concurrent Pipelined callers on the hot path, and WithTimeout clones share the slot — two sharers cannot install two different pools (CAS; the loser closes its candidate). Streaming-credentials and maintnotifications hooks are wired before publication; otel registration happens for the winner. Conn, Tx and SentinelClient never allocate the slot and keep using the main pool. The extraction into buildPipelinePool also surfaced that NewFailoverClient kept a duplicated creation block with the pre-fix gate (buffer sizes only), so PipelinePoolSize alone silently created no pipeline pool on failover clients; the shared helper fixes that, with a regression test. Stale RingOptions/FailoverOptions doc lines still describing the old gate are corrected.

…e pool The options the dedicated pipeline pool is built with are now resolved by pipelinePoolOptions, a pure function with pinned tests: Buffers default to DefaultPipelineBufferSize (new exported const, 64 KiB) rather than inheriting the regular 32 KiB. Pipeline connections move whole batches per round trip and earn bigger buffers than per-command traffic: measured on the autopipeline engine, throughput plateaus around 64 KiB, gains nothing past ~128 KiB, and very large buffers can regress it. An explicitly larger regular buffer is kept rather than shrunk, and explicit pipeline buffer sizes always win. The RESP3 minimum clamp applies as on the main pool. MinIdleConns is forced to 0 on the pipeline pool. It previously inherited the main pool's value through the options clone, so a client with MinIdleConns set pre-dialed that many pipeline connections at pool creation — a silent doubling of the idle footprint. The pipeline pool is burst capacity: its connections dial on demand, and the pool size (PipelinePoolSize or DefaultPipelinePoolSize) is only a cap, so an unused pipeline pool holds zero connections.

The autopipeline-subjects job builds its suite subject by calling AutoPipelineWithOptions on the shared rawClient, which now (correctly) creates the dedicated pipeline pool lazily - so client.Pipelined and client.Pipeline run on pipeline-pool connections, not the main pool's. Two specs assumed the old sharing: should Auth asserted main-pool stats (Hits=2/Misses=1/TotalConns=1) after two Pipelined calls. It now branches on PoolStats.PipelineStats: when the dedicated pool exists, the two pipelines account there (first dials, second reuses) and the main pool holds only the BeforeEach FlushDB dial; in the default subject mode PipelineStats is nil and the original assertions run unchanged. should ClientSetInfo issued CLIENT SETINFO through a pipeline and then read CLIENT INFO through rawClient - with a pipeline pool those are different connections, and SETINFO is per-connection state. It now reads CLIENT INFO through the same pipeline that issued the SETINFOs, which is equivalent on a single-pool client and correct on a split-pool one. Verified against redis:8 in all three GOREDIS_TEST_SUBJECT modes (default, ap-blocking, ap-async): Ran 2 of 1543 Specs, 2 Passed.

…ions Two autopipeline unit tests assumed batches execute on the main pool's already-initialized connection; with the dedicated pipeline pool now created lazily at autopipeliner creation, a batch's first dispatch dials and initializes a fresh pipeline-pool connection instead. Both tests now warm that connection with a real two-command batch first (a lone command takes the solo fast path through Process on the main pool and would not touch the pipeline pool): TestAutoPipelineRetriesOnNetworkError counted the pipeline pool's first dial as if it were a retry redial (3 dials observed, 2 expected). The warm-up runs before the write failure is armed, so the count again measures exactly the one redial the retry performs. TestAutoPipelineHookPostNextErrorPartialBatch installed a hook that unconditionally injects a post-next error before the pipeline-pool connection existed. Connection init runs its handshake pipeline through the client's hook chain (newConn shares hooksMixin), so the hook failed the pipeline conn's init itself — retries exhausted, the injected error was stamped on every command, and the test measured init poisoning rather than the post-next outcome rule it pins. The warm-up initializes the connection before the hook is installed.

…ateful CLIENT TRACKING and CLIENT MAINT_NOTIFICATIONS mutate per-connection state, exactly like CLIENT SETNAME/SETINFO, but live on cmdable: on a pooled client they land on an arbitrary pool connection that is immediately returned to the pool, so the flag applies to a connection the caller cannot address again. They belong on statefulCmdable, next to ClientSetName — but the move removes the methods from Client/UniversalClient/Cmdable (and defining them on both cmdable and statefulCmdable makes every Pipeline selector ambiguous), so it is a breaking change deferred to v10. Documented now; the move happens in a follow-up PR. No behavior change.

The ClientTracking*/ClientMaintNotifications move to statefulCmdable ships as its own PR against master (feature/stateful-conn-commands) rather than waiting for v10, so the TODO notes here are obsolete. Docs only.

The dedicated pipeline pool is now created unconditionally at NewClient/NewFailoverClient — like the pubsub pool — instead of behind a matrix of triggers (pipeline options set, AutoPipelineOptions declared, or lazily when an autopipeliner was built). It is pure burst capacity: no pre-dialing (MinIdleConns forced to 0), a small cap (DefaultPipelinePoolSize) and 64 KiB buffers, so an unused pipeline pool holds zero connections and costs nothing, while pipelines stop competing with regular commands for main-pool connections. PipelinePoolSize < 0 opts out and restores the old pipelines-on-the-main-pool behavior. To keep this from capping heavy concurrent Pipelined callers — who previously shared the much larger main pool — withPipelineConn spills to the main pool when the pipeline pool is exhausted (ErrPoolTimeout): bursts wider than the cap keep their old capacity, spilled pipelines simply run with regular buffers, and total connections stay bounded by PoolSize + PipelinePoolSize. Pipeline-pool connections are excluded from CLIENT TRACKING during init. Pipelined commands never consult or populate the built-in client-side cache (only the single-command cached path on main-pool connections does), so tracking pipeline reads only grew the server's tracking table and produced invalidation pushes for keys the cache does not hold. With creation unconditional, the lazy machinery is deleted: the atomic slot and CAS publication, ensurePipelinePool, the autopipeliner-factory hooks, the Options.init AutoPipelineOptions defaulting and its cluster clientOptions mirror. pipelinePool is a plain field set before the client is visible to any goroutine; WithTimeout clones share it. Net diff of this commit is negative. Tests: the creation-rule table becomes always-on plus opt-out; new e2e coverage for the spill (a pipeline pool of one, held busy by a pipelined BLPOP, must not queue a second pipeline behind PoolTimeout) and for the tracking exclusion (CLIENT LIST shows the main-pool connection tracked and the pipeline-pool connection untracked).

…efault The HImport mock tests choreograph exact per-connection sequences — a pool of one, booms armed to fire on the Nth replayed PREPARE, session counts pinned per connection. The always-on pipeline pool adds a second connection whose init replays fieldsets (correct production behavior: HIMPORT pipelines work on pipeline-pool connections precisely because of that replay), which fired the armed booms during the pipeline conn's init instead of the reissue under test and shifted the session counts. Opt every client in the file out with PipelinePoolSize: -1: the tests pin HIMPORT reissue/session semantics, not pool routing. Also states the PipelinePoolSize default plainly in the option docs: DefaultPipelinePoolSize (10) connections (review feedback).

The shard-level pipeline hooks in the cluster hook-order specs asserted Expect(cmds).To(HaveLen(...)) unconditionally. With the always-on pipeline pool, a node's first pipeline dials a pipeline-pool connection whose init runs its handshake pipeline through the node client's hook chain, so the assertion fired on the handshake and the resulting Ginkgo panic aborted the whole suite. Guard both hooks with the idiom ring_test.go already uses for the same situation: defer GinkgoRecover() and pass connection-initialization pipelines (hello/client) through. The cluster-level hooks are untouched — node connection init never runs through the cluster client's chain.

TestNoPipelinePoolStats pinned the stats shape of a client without a pipeline pool by creating a client without pipeline options — a premise the always-on pool inverts. Having no pool now requires the explicit opt-out, so the test sets PipelinePoolSize: -1, which is exactly the configuration whose stats shape it verifies. TestPipelineRetriesOnNetworkError is the plain-pipeline twin of the AP dial-count test adapted earlier: pipelines now run on the dedicated pipeline pool, so the first Exec dialed a fresh pipeline-pool connection and the test counted it as a phantom retry redial. Warm the pipeline connection with one Pipelined round trip before arming the write failure, so the count again measures exactly the one redial the retry performs.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1cb41b4646

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…e is live The straggler-hold pool gate reaches the pipeline pool via an in-package interface assertion (getPipelinePool() pool.Pooler) in newAutoPipeliner. On master that accessor does not exist, so the assertion fails silently: ap.pipelinePool stays nil, pipelineHasFreeConn() always returns false, and the gate degrades to the conservative long hold — the fix compiles and passes CI while doing nothing. Add the accessor (returns the baseClient pipeline pool) so the assertion succeeds and the gate actually engages. Tests assert the AutoPipeliner captures a non-nil pool (gate live) and the safe nil fallback when no pipeline pool is configured. Note: PR #3959 introduces its own getPipelinePool over a pipelinePoolRef refactor; when both land keep one definition (the #3959 ref-based form) and drop this plain accessor.

ofekshenawa

Overall looks good!
Just UniversalOptions doesn't expose PipelinePoolSize or the pipeline buffer fields, so UniversalClient users get the new pool with no way to opt out or size it.

- UniversalOptions now exposes PipelineReadBufferSize / PipelineWriteBufferSize / PipelinePoolSize and maps them through Cluster()/Failover()/Simple(), so UniversalClient users can size or opt out (PipelinePoolSize < 0) of the now always-created pipeline pool (ofek's CHANGES_REQUESTED blocker). - withPipelineConn no longer double-counts the Limiter on spill: it acquires the main-pool conn directly (not via withConn) and records which pool owns the conn, so the single deferred ReportResult+release runs once, in order (cursor HIGH). - The pipeline pool no longer inherits MaxActiveConns (reset to 0, so the ceiling is MaxActiveConns + PipelinePoolSize, not ~2x) and uses a short DefaultPipelinePoolTimeout so a saturated pipeline pool spills promptly instead of queueing; spill also triggers on ErrPoolExhausted (defensive) and the doc is corrected. Stale "lazy ensurePipelinePool" comments reworded (eager, set-once). Adds tests: spill accounts the Limiter exactly once, the clone's cap/timeout overrides, and the UniversalOptions passthrough.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb8f9a2c81

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

ndyakov · 2026-08-13T18:59:03Z

@codex review

Copilot

Pull request overview

This PR changes go-redis client construction so pipelining (Pipeline/TxPipeline and autopipeline batches) uses a dedicated “pipeline pool” by default, instead of only when explicit pipeline buffer sizes were provided. It adds an explicit opt-out (PipelinePoolSize < 0), centralizes pipeline-pool option derivation/creation logic, and updates tests/docs to reflect new routing, spill behavior, and limiter accounting.

Changes:

Create and route pipeline/autopipeline work to a dedicated pipeline connection pool by default (opt-out via PipelinePoolSize < 0), with shared helper logic to avoid drift between constructors.
Implement “spill to main pool” when the pipeline pool is saturated, avoiding double Limiter.Allow/ReportResult accounting.
Propagate pipeline pool/buffer knobs through UniversalOptions and adjust tests/docs for new pool stats and initialization behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
universal.go	Adds pipeline pool/buffer fields to `UniversalOptions` and forwards them into Simple/Cluster/Failover options.
sentinel.go	Updates failover pipeline-pool creation to use shared builder and adjusts OTel pool registration / failover cleanup.
ring.go	Updates RingOptions documentation and forwards pipeline pool settings into per-shard client options.
redis.go	Introduces `pipelinePoolRef`, centralized pipeline pool option resolution and construction, spill logic, and tracking exclusion for pipeline conns.
options.go	Expands `PipelinePoolSize` docs and adds defaults/constants for pipeline pool sizing/buffers/timeouts.
pipeline_spill_test.go	Adds regression test ensuring spill path accounts `Limiter` exactly once.
pipeline_pool_options_test.go	Adds tests pinning pipeline pool option resolution and `UniversalOptions` passthrough.
pipeline_pool_gate_test.go	Adds tests for always-created/opt-out behavior, spill-to-main behavior, and CSC tracking exclusion.
pipeline_exec_test.go	Warms pipeline pool to keep retry/dial-count assertions stable under new routing.
pipeline_buffer_test.go	Updates “no pipeline pool” test to opt out explicitly under new defaults.
osscluster_test.go	Adjusts cluster hook tests to tolerate pipeline-connection init pipelines through hook chain.
himport_mock_test.go	Opts out of pipeline pool to keep deterministic per-connection sequencing in mocked tests.
commands_test.go	Updates pool stats expectations and ensures per-connection `CLIENT SETINFO` assertions read through same pipeline path.
autopipeline_test.go	Warms pipeline pool connections before injecting failures/hooks to avoid measuring init side-effects.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

chatgpt-codex-connector · 2026-08-13T19:05:47Z

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: cb8f9a2c81

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

- Options/FailoverOptions/RingOptions docs: pipeline pool is created by default (opt out PipelinePoolSize < 0); buffers default to max(regular, 64 KiB). - pipeline_pool_gate_test.go: poll PoolStats instead of a fixed Sleep (de-flake). - withPipelineConn now spills to the main pool on a pipeline-conn INIT failure too (e.g. a fresh dial refused with maxclients), not just Get saturation; non-saturation Get errors (ctx cancelled / pool closed) still do not spill. - ClusterClient.PoolStats / Ring.PoolStats aggregate each node/shard pipeline pool into PoolStats.PipelineStats.

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit dad8cbf. Configure here.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dad8cbfe95

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

- Options/FailoverOptions/RingOptions docs: pipeline pool is created by default (opt out PipelinePoolSize < 0); buffers default to max(regular, 64 KiB). - pipeline_pool_gate_test.go: poll PoolStats instead of a fixed Sleep (de-flake). - withPipelineConn now spills to the main pool on a pipeline-conn INIT failure too (e.g. a fresh dial refused with maxclients), not just Get saturation; non-saturation Get errors (ctx cancelled / pool closed) still do not spill. - ClusterClient.PoolStats / Ring.PoolStats aggregate each node/shard pipeline pool into PoolStats.PipelineStats. - Ring.PoolStats now folds WaitCount/WaitDurationNs/StaleConns into both the main-pool and pipeline-pool accumulators, matching the cluster fold (were dropped before). - pipelinePoolOptions caps the pipeline PoolTimeout at DefaultPipelinePoolTimeout but honors a caller's SHORTER PoolTimeout (min), so a client tuned to spill faster is not forced to wait the full default before falling back.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19cdc58717

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pipeline_spill_test.go:31

This test hard-codes ":6379" for the Redis address, but other plain-go tests in this PR use gateTestAddr() to respect REDIS_PORT. If CI (or local runs) override REDIS_PORT, this test can fail to connect and will be skipped or flaky for the wrong reason.

	ctx := context.Background()
	lim := &spillCountingLimiter{}
	c := NewClient(&Options{Addr: ":6379", PipelinePoolSize: 1, Limiter: lim})
	defer c.Close()

ParseURL/ParseClusterURL/ParseFailoverURL now consume pipeline_pool_size, pipeline_read_buffer_size and pipeline_write_buffer_size, so a URL such as redis://host:6379?pipeline_pool_size=-1 applies the documented opt-out instead of being rejected as an unexpected option. Adds TestParseURLPipelinePoolOptions.

setup-go's "1.26.x" resolved to go1.26.5, which govulncheck flags for two standard-library vulnerabilities fixed in go1.26.6: GO-2026-6090 (crypto/tls) and GO-2026-5972 (encoding/asn1). Track the latest stable toolchain so future security patches are picked up automatically instead of pinning a patch.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1605d31d9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Handoff worker and queue defaults were derived from PoolSize alone, but the manager hooks the dedicated pipeline pool as well, so its connections competed for capacity sized as if only the main pool existed. Options.init now derives the defaults from the combined ceiling (PoolSize + effective PipelinePoolSize) and widens the MaxActiveConns bound the same way, since the pipeline pool sits outside it.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f73f750f1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-15T10:29:39Z

+	}
+
+	if spill {
+		cn, retErr = c.connPool.Get(ctx)


Use one timeout budget when spilling

When both pools are saturated and the caller has no context deadline, this second Get starts a fresh PoolTimeout after the pipeline pool has already waited up to min(PoolTimeout, 100ms). Thus a client configured with PoolTimeout: 10ms can wait roughly 20ms before receiving ErrPoolTimeout, and larger values can overrun their documented connection-wait budget by 100ms. The cap added for shorter timeouts is fresh evidence beyond the earlier comment: it limits the first wait but still allows the two waits to accumulate; carry the remaining budget into the spill instead.

Useful? React with 👍 / 👎.

ndyakov added 8 commits August 10, 2026 13:31

docs: drop the TODO(v10) notes on the per-connection commands

b28fa58

The ClientTracking*/ClientMaintNotifications move to statefulCmdable ships as its own PR against master (feature/stateful-conn-commands) rather than waiting for v10, so the TODO notes here are obsolete. Docs only.

ndyakov commented Aug 10, 2026

View reviewed changes

Comment thread options.go Outdated

ndyakov and others added 4 commits August 10, 2026 18:24

Merge branch 'master' into ndyakov/fix-pipeline-pool-size-gate

1cb41b4

ndyakov marked this pull request as ready for review August 10, 2026 16:20

ndyakov requested a review from ofekshenawa August 10, 2026 16:24

cursor Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread redis.go Outdated

chatgpt-codex-connector Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread redis.go Outdated

Comment thread redis.go

Comment thread redis.go Outdated

ndyakov added the maintenance label Aug 10, 2026

ndyakov changed the title ~~fix(pipeline): Create pipeline pool even without specific buffer passed~~ chore(pipeline): Create pipeline pool even without specific buffer passed Aug 10, 2026

ofekshenawa reviewed Aug 12, 2026

View reviewed changes

Comment thread options.go Outdated

ofekshenawa reviewed Aug 12, 2026

View reviewed changes

Comment thread redis.go Outdated

ofekshenawa requested changes Aug 12, 2026

View reviewed changes

ndyakov mentioned this pull request Aug 13, 2026

feat(autopipeline): ordered full-duplex dispatch #3964

Open

chatgpt-codex-connector Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread redis.go Outdated

Comment thread redis.go

Comment thread redis.go

ndyakov requested a lite review from Copilot August 13, 2026 19:00

Copilot started reviewing on behalf of ndyakov August 13, 2026 19:00 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Comment thread options.go Outdated

Comment thread sentinel.go Outdated

Comment thread ring.go Outdated

Comment thread pipeline_pool_gate_test.go Outdated

cursor Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread ring.go

chatgpt-codex-connector Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread redis.go

chatgpt-codex-connector Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread options.go

Comment thread redis.go

ndyakov requested a lite review from Copilot August 14, 2026 08:36

Copilot started reviewing on behalf of ndyakov August 14, 2026 08:37 View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

ndyakov added 2 commits August 14, 2026 16:32

chatgpt-codex-connector Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread redis.go

Comment thread redis.go

chatgpt-codex-connector Bot reviewed Aug 15, 2026

View reviewed changes

Uh oh!

Conversation

ndyakov commented Aug 10, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dedicated pipeline pool by default

Pool resolution (pipelinePoolOptions)

Config surface

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ofekshenawa left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ndyakov commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Aug 13, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

ndyakov commented Aug 10, 2026 •

edited by cursor Bot

Loading

Pool resolution (`pipelinePoolOptions`)