Complete connection pool pruning (Story 2/3/4)#4463
Conversation
There was a problem hiding this comment.
Pull request overview
Completes the V2 ChannelDbConnectionPool idle-pruning feature by deriving the pruning sampling window from the existing Connection Idle Timeout setting (instead of LoadBalanceTimeout + a fallback constant), and adds resilience/robustness unit tests for pruning behavior and shutdown races.
Changes:
- Size the pruning window from
PoolGroupOptions.IdleTimeout(sampleSize = ceil(idleTimeout / 10s), clamped toMaxSampleSizewith a pooler trace when clamped). - Only construct the pruner when
MinPoolSize < MaxPoolSizeandIdleTimeout != 0. - Expand unit tests to cover spike resilience, prune→disable→regrow cycles, shutdown-race no-op behavior, and concurrent checkout/return invariants.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs | Gates pruner construction on IdleTimeout != 0 and documents the new rationale. |
| src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/PoolPruner.cs | Computes sample window from idle-timeout seconds, clamps with EventSource trace, and uses Timeout.InfiniteTimeSpan consistently. |
| src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolPruningTest.cs | Updates existing tests for idle-timeout-based sizing and adds resilience/robustness/concurrency regression coverage. |
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using System.Transactions; |
| var threads = new Thread[threadCount]; | ||
| for (int t = 0; t < threadCount; t++) | ||
| { | ||
| threads[t] = new Thread(() => | ||
| { | ||
| for (int i = 0; i < iterations; i++) | ||
| { | ||
| var busy = CheckOutConnections(pool, perIteration); | ||
| ReturnConnections(pool, busy); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| foreach (var thread in threads) | ||
| { | ||
| thread.Start(); | ||
| } | ||
| foreach (var thread in threads) | ||
| { | ||
| thread.Join(); | ||
| } |
05509bf to
a07449d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4463 +/- ##
==========================================
- Coverage 65.83% 63.43% -2.40%
==========================================
Files 287 283 -4
Lines 43763 66823 +23060
==========================================
+ Hits 28812 42392 +13580
- Misses 14951 24431 +9480
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Wrap up the V2 ChannelDbConnectionPool idle-pruning feature started in Story 1.
Story 2 - derive the pruning window from Connection Idle Timeout:
- Base the sampling window on PoolGroupOptions.IdleTimeout instead of
LoadBalanceTimeout (which defaulted to 0 and fell back to a hard-coded 300s
window).
- Derive BOTH the sampling interval and the sample count from IdleTimeout so the
window always spans the full idle timeout while the sample count stays bounded:
interval = clamp(ceil(IdleTimeout / MaxSampleSize), 10s, 1 day)
sampleSize = min(MaxSampleSize (300), ceil(IdleTimeout / interval))
Short/typical idle timeouts keep the 10s cadence and grow the sample count
(e.g. default 300s -> 10s, 30 samples: identical to Story 1). Large idle
timeouts pin the sample count at 300 and stretch the interval instead of
shrinking the window. The 1-day interval cap is a defensive Timer.Change
overflow guard that only engages above ~300-day idle timeouts.
- Only construct the pruner when MinPoolSize < MaxPoolSize AND IdleTimeout != 0;
when idle reclamation is disabled there is nothing to prune. Gating is on the
IdleTimeout value, not on UseLegacyIdleTimeoutBehavior (which defaults to true).
- Default Connection Idle Timeout (300s) yields a 10s interval and 30 samples,
identical to Story 1 defaults, so shipped behavior is unchanged.
- Emit an EventSource pooler trace when the interval is stretched off the default.
- Use Timeout.InfiniteTimeSpan consistently in the timer-disable path.
No new connection-string keyword or AppContext switch is introduced; the window
is derived internally from the existing Connection Idle Timeout.
Story 3 - resilience tests (tests only): median ignores a transient demand
spike within a window (does not under-prune) and the prune -> disable -> regrow
-> re-arm -> prune cycle. No prune-margin algorithm change.
Story 4 - a pruning timer callback that races with Shutdown() is an inert no-op.
Also adds a multi-threaded checkout/return regression test asserting the
invariant: pool has more than MinPoolSize connections <=> pruning timer enabled.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c28ccaf1-48e0-44cd-b2f9-8f1cc5946df7
a07449d to
33f2900
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolPruningTest.cs:10
using System.Threading.Tasks;is unused in this test file, which will trigger CS8019 and fail the build (TreatWarningsAsErrors=true).
using System.Threading;
using System.Threading.Tasks;
using System.Transactions;
| /// The configured Connection Idle Timeout, used as the pruning window. The sampling interval | ||
| /// and sample count are derived from it so the window always spans the full idle timeout while | ||
| /// the sample count stays bounded by <see cref="MaxSampleSize"/> (see the constructor body for | ||
| /// the exact formula). The pool only constructs a pruner when idle-timeout based reclamation is | ||
| /// enabled (idleTimeout > 0); a defensive floor keeps the sizing math valid. |
Summary
Wraps up the V2
ChannelDbConnectionPoolidle-pruning feature started in Story 1: derive the pruning window fromConnection Idle Timeout(Story 2) plus resilience/robustness tests (Stories 3 & 4). Part of parent #37338.What changed
Story 2 — window derived from
Connection Idle Timeout(wasLoadBalanceTimeout, which defaulted to0-> hard-coded 300s):interval = clamp(ceil(IdleTimeout / 300), 10s, 1 day)sampleSize = min(300, ceil(IdleTimeout / interval))Timer.Changeoverflow.MinPoolSize < MaxPoolSizeandIdleTimeout != 0.300s -> 10s, 30 samples= Story 1 defaults (no behavior change).Story 3 (tests): median ignores a transient demand spike; prune -> regrow -> re-arm cycle. No prune-margin change (deferred).
Story 4 (test): pruning callback racing with
Shutdown()is an inert no-op.Regression: multi-threaded checkout/return asserts
Count > MinPoolSize <=> timer enabled.Notes for reviewer (@Malcolm)
Connection Idle Timeoutrather than add a V2-specificConnection Pruning Intervalkeyword (ambiguous to users re: V1 vs V2 pool).Testing
ChannelDbConnectionPoolPruningTest— 39/39 onnet8.0andnet9.0; net462 driver builds.Checklist
Suggested release note
Fixes AB#45165