Background pool warmup and replenishment for ChannelDbConnectionPool

[mdaigle]

Summary

Implements background pool warmup and replenishment for ChannelDbConnectionPool, per the new specs/003-pool-warmup/spec.md. Stacks on the pool rate-limiting work (base: dev/mdaigle/pool-channel-rate-limiting).

On Startup(), the pool asynchronously pre-creates connections up to Min Pool Size, serially (one at a time) through the shared rate limiter used by user requests. Whenever the pool drops below Min Pool Size for any reason (destroy-on-return, idle-timeout eviction, pruning), it automatically replenishes through the same path.

Implementation

• RequestWarmup() — the single, coalesced entry point for every trigger. No-op when MinPoolSize == 0, not running, shutting down, or already at/above the minimum. A _warmupLoopRunning CAS guard ensures only one warmup loop runs at a time; a request that arrives while a loop is running is simply dropped, because that loop re-reads Count each iteration and drives the pool to the minimum regardless. It launches the loop via a fire-and-forget Task.Run and releases the guard if scheduling throws.
• RunWarmupLoopAsync() — the background loop. Creates connections serially up to MinPoolSize; a null return means the shared rate limiter is saturated, so it ends the current pass rather than bypassing or spinning on the limiter (convergence to the minimum is handled by the concurrent user creations that saturated it and by re-triggering on the next below-minimum event); publishes fresh connections to the idle channel; and releases the single-loop guard in a finally on every exit path. All exceptions are absorbed so nothing escapes onto the thread pool.
• Error-state parity with the WaitHandle pool. Warmup creates through the same OpenNewInternalConnection path as user requests, with no isWarmup exemption: a genuine open failure enters the blocking-period error state and a successful open clears it, exactly like an on-demand creation. The warmup-specific behavior is narrow: the loop absorbs the rethrown failure (so it never surfaces as an unhandled exception) and stands down while the pool is blocking (its loop condition checks ErrorOccurred), rather than piling more doomed opens onto a struggling server. Consequently, a user request during the blocking window fast-fails with the cached exception, matching the WaitHandle pool.
• Triggers wired: Startup() → warmup; RemoveConnection() → replenish (the single choke point for all below-minimum events); Shutdown() cancels + disposes the warmup CTS. The loop reads the CTS token once under a guard and is resilient to disposal (Cancel always precedes Dispose, so post-capture reads observe cancellation, not disposal).
• The loop never blocks the caller and uses no sync-over-async. The physical connection open itself is currently synchronous (executed on the background task); the connection factory does not yet expose an async open.

Testing

• Tests added — ChannelDbConnectionPoolWarmupTest.cs, 14 tests covering Stories 1–5: Min Pool Size 0/1/N, serial creation, shared rate limiter, warmup-failure resilience (absorbed, enters the blocking-period error state, user fast-fails during the window), standing down while the pool is blocking, shutdown cancellation, and all replenishment triggers with no overshoot at the minimum.
• Public API changes: none
• No breaking changes

Full ConnectionPool unit suite: 263/263 pass. Warmup tests stable across repeated runs.

Implements specs/003-pool-warmup/spec.md.

[Copilot]

Pull request overview

Implements background warmup and automatic replenishment for ChannelDbConnectionPool so pools proactively create connections up to MinPoolSize and restore back to the minimum after below-minimum events, without blocking callers.

Changes:

• Add a coalesced, serial warmup/replenishment loop driven by Startup() and RemoveConnection() triggers, using the shared connection-creation rate limiter.
• Extend OpenNewInternalConnection with an isWarmup mode that avoids blocking-period error-state interaction and absorbs warmup failures.
• Add a new unit test suite (ChannelDbConnectionPoolWarmupTest) and a feature spec document (specs/003-pool-warmup/spec.md).

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	Adds warmup/replenishment loop plumbing, shutdown cancellation, and warmup-specific connection creation behavior.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolWarmupTest.cs	New unit tests covering warmup, shared rate limiter behavior, failure resilience, shutdown cancellation, and replenishment triggers.
specs/003-pool-warmup/spec.md	New spec documenting scenarios and acceptance criteria for the warmup feature.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1071

• RequestWarmup can start a warmup loop even when the pool is already at/above MinPoolSize (e.g., any RemoveConnection call). The loop will immediately no-op, but it still incurs Task.Run scheduling overhead and makes the behavior differ from the spec’s “only when below minimum” trigger. Add a fast-path Count >= MinPoolSize check here to keep warmup truly demand-driven.

            // No-op when there is nothing to pre-create (MinPoolSize == 0), the pool is not running,
            // or shutdown has cancelled background activity.
            if (MinPoolSize == 0 || State != Running || _warmupCts.IsCancellationRequested)
            {
                return;

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:135

• The _warmupCts summary claims the token is passed down into physical connection creation so a blocked open can be unwound. However, OpenNewInternalConnection only checks the token before calling ConnectionFactory.CreatePooledConnection(...), and the factory path itself does not accept a CancellationToken (see the nearby TODO). This comment is misleading about the cancellation behavior.

        /// <summary>
        /// Cancels in-flight background warmup/replenishment when the pool shuts down so no new
        /// connections are created after shutdown begins (Story 4). The token is observed at the
        /// warmup loop's await points and before each creation attempt, so it stops the loop and
        /// prevents further attempts promptly. It cannot abort an already in-progress physical open:

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:377

• Shutdown comment says cancelling _warmupCts “unwinds a create that is currently blocked”, but the connection factory does not take a cancellation token (only a timeout budget). Cancellation here prevents additional warmup attempts and lets the loop exit promptly once the in-flight create completes; it does not abort a synchronous create already inside CreatePooledConnection.

            // routes returning connections to RemoveConnection.
            State = ShuttingDown;

            // Cancel any in-flight background warmup/replenishment so no new connections are

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1205

• WarmupPassAsync calls OpenNewInternalConnection(owningConnection: null, ...). That can break certain connection-string paths that assume a non-null SqlConnection during physical open; for example, SqlConnectionFactory.CreateConnection dereferences sqlOwningConnection in the UserInstance initialization path when the pool is empty. Consider using a dummy SqlConnection as the owning connection when UserInstance is enabled, while keeping null for the normal warmup case.

                DbConnectionInternal? connection;
                try
                {
                    // owningConnection is null: warmup connections are created unattached and enter

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs:1229

• The warmup loop’s outer catch (Exception) currently swallows non-catchable exceptions (e.g., OutOfMemoryException / NullReferenceException), which can hide serious corruption bugs. It’s safer to only absorb exceptions that ADP.IsCatchableExceptionType deems catchable and let the rest propagate (the finally still releases the warmup guard).

            catch (Exception ex)
            {
                // Defense in depth: the loop must never throw onto the thread pool.
                SqlClientEventSource.Log.TryPoolerTraceEvent(
                    "<prov.DbConnectionPool.RunWarmupLoopAsync|RES|CPOOL> {0}, Warmup loop failed, absorbing: {1}", Id, ex);

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs:1931

• This test no longer synchronizes on B actually being denied by the rate limiter while A holds the only permit. In some interleavings (especially the async path), B may start after A releases the lease, so the assertions can pass without exercising the intended “rate-limited waiter / lease-release poke” behavior.

            // Caller B is denied a permit (A holds it) and falls back to the idle-channel wait.
            Task<DbConnectionInternal?> requestB = Open(new SqlConnection());