feat(pool): Dial limiter

[ndyakov]

Adds opt-in DialRateLimit/DialRateBurst. When idle conns are exhausted and a
dial would be throttled, the request parks and waits for a returned idle conn
(preferring reuse) instead of dialing; only after PoolTimeout does it create a
new connection. Exposes PoolStats.RateLimitedDials.

Closes #3890.

---

Note

Medium Risk
Substantial new concurrency in ConnPool.Get() and min-idle refill when the feature is on, though default-off limits blast radius; mis-tuning can add tail latency or soft-limit bypass after PoolTimeout.

Overview
Adds opt-in DialRateLimit / DialRateBurst so the pool can pace new dials during bursts instead of dialing up to PoolSize immediately (addresses #3890). When enabled, a miss path that would dial first tries a token-bucket limiter; if throttled, Get() parks on a FIFO wait queue and prefers reusing an idle connection returned via Put, with PoolTimeout (from Get() entry) as a soft escape that still allows an unpaced dial. Idle hits stay unchanged (nil limiter check only).

MinIdleConns refill is integrated: workers take dial tokens before pool turns, use refillPending so token-waiting workers do not inflate MaxActiveConns / poolSize, and Close() wakes parked waiters. New PoolStats.RateLimitedDials (and cluster aggregation) exposes throttle volume. Options propagate through standalone, cluster, sentinel, universal clients and URL dial_rate_limit / dial_rate_burst.

CI pins Go 1.26.5 in govulncheck so runner-cached older patches do not fail the job on stdlib vulns unrelated to the PR.

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

[ndyakov]

DialRateLimit — measured latency / throughput / connection tradeoff

Smoke benchmark against a real Redis (local, Apple M4 Max, RESP3): 200 goroutines × 100 PINGs arriving as one burst, PoolSize=50, PoolTimeout=3s. Every request in flight simultaneously, i.e. sustained saturation — the worst case for the limiter, since there are never enough idle connections for everyone.

config	conns created	throttled Gets	timeouts	errors	ops/s	avg	p50	p99	max
disabled	50	0	0	0	51,267	3.9ms	3.8ms	5.6ms	13ms
rate=50/s	50	0	0	0	45,570	4.4ms	4.2ms	8.6ms	19ms
rate=20/s	33	8,147	0	0	30,616	6.4ms	6.2ms	12.8ms	24ms
rate=5/s	13	5,923	0	0	12,386	15.8ms	13.9ms	53.6ms	260ms

Takeaways:

• Disabled = unchanged baseline: the burst instantly dials the pool to 50 connections (the "dial storm" from Feature Request: Add built-in connection creation rate limiter with idle-retry capability for burst traffic #3890). Hot-path benchmarks vs master show no significant delta when the feature is off (interleaved benchstat, p≥0.16, allocs byte-identical).
• rate ≥ burst demand (50/s): behaves like disabled — the token bucket (default burst = rate) absorbs the storm, park machinery never engages.
• rate=20/s: connection count drops 34% (33 vs 50); cost is ~2× p99 under this saturated workload.
• rate=5/s: 74% fewer connections (13 vs 50). Under sustained saturation that necessarily costs throughput (fewer pipes) and tail latency (max 260ms = park waits); zero pool timeouts and zero errors — every request completed via reuse or the PoolTimeout escape hatch.

Note on interpretation: sustained saturation makes the tradeoff look maximal. The feature's target scenario is a short burst on an otherwise steady workload — there the pool stops inflating to PoolSize for a spike that reuse could absorb, and steady-state latency is untouched because the limiter is only consulted on the dial slow path (idle hits never see it).

Rule of thumb: set DialRateLimit at or above your expected steady-state connection-creation demand; it should throttle storms, not normal growth. DialRateBurst (default = rate) tunes how large a spike passes before pacing engages.

[ofekshenawa]

Here the refill worker takes a pool slot and then waits for its rate-limit token, holding the slot the whole time it waits. So if MinIdleConns and DialRateLimit are both on, the workers can end up sitting on all the slots while they wait, and real Get() calls have nothing left and time out. Could we grab the token first and then the slot?

[ofekshenawa]

Left a small comment there. Other than that, all good!

[cursor]

Escape path creates tight CPU spin without sleep or context check

Medium Severity

When a throttled Get() escapes the deadline and hasAcquirableIdleConn() returns true, the outer loop continues back to the inner idle-check loop. If the inner loop exhausts getAttempts without obtaining a usable connection (e.g., a hook rejects and puts it back via putConnWithoutTurn), the code re-enters the rate limiter path where waitForDialSlot returns immediately (deadline passed), hasAcquirableIdleConn() still returns true, and continue fires again — forming a tight CPU loop with no sleep, no context cancellation check, and no iteration bound, spinning until Allow() eventually succeeds (up to 1/rate seconds).

Additional Locations (1)

• internal/pool/pool.go#L1368-L1373

 

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

[ofekshenawa]

A hook rejected idle connection can be retried in a tight loop after the timeout, so maybe limit the retry and check for context cancellation.

[ofekshenawa]

While min idle jobs wait for a rate limit token, normal requests can fill the pool, so maybe recheck capacity before those jobs create another connection?

[cursor]

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

There are 2 total unresolved issues (including 1 from previous review).

^{❌ 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 40a9a9f. Configure here.}

[cursor]

Refill worker skips capacity recheck after token wait

Low Severity

After waitForDialToken() returns (potentially seconds later at low rates), the refill worker acquires a semaphore turn and unconditionally bumps poolSize/idleConnsLen without rechecking whether idleConnsLen < MinIdleConns still holds. Other refill workers or normal Get() paths may have already satisfied the min-idle requirement during the wait, causing unnecessary connection creation beyond MinIdleConns.

 

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