Skip to content

Complete connection pool pruning (Story 2/3/4)#4463

Open
apoorvdeshmukh wants to merge 1 commit into
dotnet:mainfrom
apoorvdeshmukh:apoorvdeshmukh-dev-automation-pool-pruning
Open

Complete connection pool pruning (Story 2/3/4)#4463
apoorvdeshmukh wants to merge 1 commit into
dotnet:mainfrom
apoorvdeshmukh:apoorvdeshmukh-dev-automation-pool-pruning

Conversation

@apoorvdeshmukh

@apoorvdeshmukh apoorvdeshmukh commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Wraps up the V2 ChannelDbConnectionPool idle-pruning feature started in Story 1: derive the pruning window from Connection 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 (was LoadBalanceTimeout, which defaulted to 0 -> hard-coded 300s):

  • Both the sampling interval and sample count are derived so the window spans the full idle timeout while the sample count stays bounded:
    • interval = clamp(ceil(IdleTimeout / 300), 10s, 1 day)
    • sampleSize = min(300, ceil(IdleTimeout / interval))
  • Short/typical timeouts keep the 10s cadence; large timeouts pin sampleSize at 300 and stretch the interval (EventSource trace on stretch). The 1-day cap guards Timer.Change overflow.
  • Pruner constructed only when MinPoolSize < MaxPoolSize and IdleTimeout != 0.
  • No new keyword / AppContext switch. Default 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)

  • Adaptive interval per our discussion: bound both interval and sample count (max 300); stretch the interval instead of shrinking the window for large idle timeouts.
  • Chose to derive the window from Connection Idle Timeout rather than add a V2-specific Connection Pruning Interval keyword (ambiguous to users re: V1 vs V2 pool).

Testing

ChannelDbConnectionPoolPruningTest — 38/38 on net8.0 and net9.0; net462 driver builds.

Checklist

  • Tests added/updated
  • No public API changes
  • No breaking changes (defaults unchanged)
  • Verified against customer repro (N/A — internal pool tuning)

Suggested release note

Connection pooling (V2 ChannelDbConnectionPool): the background idle-connection pruner now sizes its sampling window from Connection Idle Timeout and is disabled when idle timeout is 0. Default behavior is unchanged.

Fixes #45165

Copilot AI review requested due to automatic review settings July 21, 2026 15:07
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Jul 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to MaxSampleSize with a pooler trace when clamped).
  • Only construct the pruner when MinPoolSize < MaxPoolSize and IdleTimeout != 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.

@apoorvdeshmukh
apoorvdeshmukh force-pushed the apoorvdeshmukh-dev-automation-pool-pruning branch 2 times, most recently from 05509bf to a07449d Compare July 21, 2026 15:39
@apoorvdeshmukh apoorvdeshmukh added this to the 7.1.0-preview3 milestone Jul 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.37%. Comparing base (fdebcd2) to head (dde8d05).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4463      +/-   ##
==========================================
- Coverage   65.83%   62.37%   -3.47%     
==========================================
  Files         287      283       -4     
  Lines       43763    66823   +23060     
==========================================
+ Hits        28812    41681   +12869     
- Misses      14951    25142   +10191     
Flag Coverage Δ
CI-SqlClient ?
PR-SqlClient-Project 62.37% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI review requested due to automatic review settings July 23, 2026 14:24
@apoorvdeshmukh
apoorvdeshmukh force-pushed the apoorvdeshmukh-dev-automation-pool-pruning branch from a07449d to 33f2900 Compare July 23, 2026 14:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

@apoorvdeshmukh
apoorvdeshmukh marked this pull request as ready for review July 23, 2026 14:35
@apoorvdeshmukh
apoorvdeshmukh requested a review from a team as a code owner July 23, 2026 14:35
@github-project-automation github-project-automation Bot moved this from To triage to Waiting for customer in SqlClient Board Jul 23, 2026
Copilot AI review requested due to automatic review settings July 24, 2026 09:32
@apoorvdeshmukh
apoorvdeshmukh force-pushed the apoorvdeshmukh-dev-automation-pool-pruning branch from 33f2900 to d7ebbfb Compare July 24, 2026 09:32
@apoorvdeshmukh
apoorvdeshmukh requested a review from mdaigle July 24, 2026 09:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

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.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c28ccaf1-48e0-44cd-b2f9-8f1cc5946df7
Copilot AI review requested due to automatic review settings July 24, 2026 09:40
@apoorvdeshmukh
apoorvdeshmukh force-pushed the apoorvdeshmukh-dev-automation-pool-pruning branch from d7ebbfb to dde8d05 Compare July 24, 2026 09:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@apoorvdeshmukh apoorvdeshmukh moved this from Waiting for customer to In review in SqlClient Board Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

4 participants