Improve connection pool failure diagnostics by mdaigle · Pull Request #4590 · dotnet/SqlClient

mdaigle · 2026-08-24T22:20:23Z

Description

Connection acquisition failures can end as a generic pool timeout, hiding whether the request was blocked by pool capacity, connection creation throttling, abandoned connections, or failures consumed by connection-open retries.

This change:

Adds timeout-only diagnostics to both pool implementations, including the wait reason, capacity and idle counts, pending opens, waiting callers, checked-out and transaction-held connections, abandoned connections, longest checkout duration, and reclamation during the request. The values are included in the timeout message and Exception.Data.
Adds the public SqlException.ConnectionOpenRetryFailures property. It preserves transient failures in retry order while retaining the terminal exception's stack, Errors, and InnerException.
Keeps retry history scoped to one Open call. Successful opens discard it, and pool blocking-period replays do not expose another request's ledger.
Documents the new API and adds localized resource entries. Pool timeout messages now retain the existing text as a prefix and append the diagnostic snapshot.

Issues

Fixes #3545

Testing

Added sync and async coverage for full-pool and rate-limited acquisition timeouts across both pool implementations.
Added abandoned-owner and checkout-age diagnostics coverage.
Added simulated-server coverage for exhausted retries, transient failure followed by a final timeout, and successful retry cleanup.
Ran the connection-pool unit suite on net9.0, targeted diagnostics on net8.0 and net9.0, portable simulated connection tests, and SqlException functional coverage.
Built the implementation and reference assembly projects for net8.0 and net9.0.
The full unit-suite invocation did not complete within the available runner timeout. The affected suites completed successfully.

Guidelines

Please review the contribution guidelines before submitting a pull request:

Contributing
Code of Conduct
Best Practices
Coding Style
Review Process
Tests added or updated
Public API changes documented
Verified against customer repro (not applicable)
Ensure no breaking changes introduced

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Improves connection-acquisition failure diagnostics across both connection pool implementations and adds a new public API surface on SqlException to expose the per-Open retry ledger, making pool timeouts and connection-open retry behavior easier to troubleshoot.

Changes:

Append timeout-only pool diagnostic snapshots to pooled-open timeout messages and Exception.Data (wait reason, counts, reclamation, longest checkout, etc.) in both WaitHandleDbConnectionPool and ChannelDbConnectionPool.
Add public SqlException.ConnectionOpenRetryFailures and plumb retry-ledger capture/attachment during physical connection-open retries while keeping the terminal exception’s primary details intact.
Add/adjust unit and simulated-server tests to validate retry-ledger behavior and pool-timeout diagnostics (including rate limiting, full pool, and abandoned owners).

Reviewed changes

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

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/SqlExceptionRetryFailuresTest.cs	New unit tests validating `SqlException.ConnectionOpenRetryFailures` behavior and blocking-period replay behavior.
src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs	Simulated-server coverage for retry-ledger preservation, exhaustion, timeout-on-final attempt, and cleanup on success.
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/PoolAcquisitionDiagnosticsTest.cs	New unit tests validating pool-timeout diagnostics snapshots (full pool, rate limiting, abandoned owners).
src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs	Adjust existing assertions to allow diagnostic suffix appended to pooled-open timeout messages.
src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/TransientTdsErrorTdsServerArguments.cs	Add configurable delay after transient errors to force connect-timeout exhaustion scenarios.
src/Microsoft.Data.SqlClient/tests/tools/TDS/TDS.Servers/TransientTdsErrorTdsServer.cs	Implement delayed login response (cancellable on dispose) and make Dispose idempotent.
src/Microsoft.Data.SqlClient/src/Resources/Strings.resx	Add localized strings for pool timeout diagnostics and retry-ledger `ToString()` rendering.
src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs	Designer updates for the new localized resources.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlException.cs	Add `ConnectionOpenRetryFailures`, render retry ledger in `ToString()`, and support cloning with/without the ledger.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs	Capture and attach timeout diagnostics to pooled-open timeouts; track pending opens and reclamation deltas.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/PoolAcquisitionDiagnostics.cs	New diagnostics model + builder for timeout snapshots (message + `Exception.Data` keys).
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs	Capture and attach timeout diagnostics (including rate-limit wait reason and reclamation deltas); record checkout timestamps.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/BlockingPeriodErrorState.cs	Ensure cached exceptions do not replay another `Open` call’s retry ledger.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs	Record transient retry failures during physical open and attach them to the terminal `SqlException`.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs	Track checkout time and add pool-usage classification for timeout diagnostics; update `PostPop` signature.
src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs	Add pooled-open timeout overload that appends diagnostics and populates `Exception.Data`.
src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.cs	Update reference assembly surface for new public `SqlException.ConnectionOpenRetryFailures`.
doc/snippets/Microsoft.Data.SqlClient/SqlException.xml	Document the new `SqlException.ConnectionOpenRetryFailures` API.

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

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

+            InvalidOperationException timeout =
+                Assert.Throws<InvalidOperationException>(() =>
+                    pool.TryGetConnection(
+                        new SqlConnection(),
+                        taskCompletionSource: null,
+                        expiredTimeout,
+                        out _));


paulmedynski · 2026-08-25T13:46:03Z

        internal DateTime ReturnedTime { get; set; }

+        /// <summary>
+        /// UTC timestamp of the current checkout, or <see cref="DateTime.MinValue"/> while the


Why would we be examining CheckoutTime if the connection isn't checked-out? I wondering if this sentinel is necessary and if we could save an assignment on each return-to-pool.

paulmedynski · 2026-08-25T13:49:03Z

+                return PoolConnectionUsageState.CheckedOut;
+            }
+
+            if (_checkoutTime != DateTime.MinValue && IsEmancipated)


We already know the connection is checked out (from line 871). If there was no sentinel, we wouldn't have to check for it here.

paulmedynski · 2026-08-25T13:55:33Z

+        private void AttachConnectionOpenRetryFailures(
+            SqlException terminalException)
+        {
+            if (_connectionOpenRetryFailures is { Count: > 0 })


I think this would be more clear if inlined on line 466.

paulmedynski · 2026-08-25T14:05:39Z

 #endif
        private SqlException(SerializationInfo si, StreamingContext sc) : base(si, sc)
        {
+            _connectionOpenRetryFailures = s_emptyConnectionOpenRetryFailures;


Why is this necessary?

paulmedynski · 2026-08-25T14:07:28Z


+        /// <include file='../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlException.xml' path='docs/members[@name="SqlException"]/ConnectionOpenRetryFailures/*' />
+        public IReadOnlyList<SqlException> ConnectionOpenRetryFailures =>
+            _connectionOpenRetryFailures ?? s_emptyConnectionOpenRetryFailures;


I don't think _connectionOpenRetryFailures is ever null.

paulmedynski · 2026-08-25T14:27:31Z

                return GenerateErrorMessage(request);
            }

+            if (Arguments.DelayAfterTransientErrors > TimeSpan.Zero)


What is this doing, and what is _disposeCts for?

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs:1630

Monitor.TryEnter(connection, ref locked) can block indefinitely; on the timeout path this can add unbounded delay (or even hang) while trying to capture diagnostics. Use the zero-timeout overload so the diagnostics snapshot is truly best-effort and non-blocking.

                    try
                    {
                        Monitor.TryEnter(connection, ref locked);
                        if (locked)
                        {

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

Monitor.TryEnter(connection, ref locked) can block; during a pooled-open timeout this defeats the goal of capturing a cheap snapshot and can add unbounded delay. Use the zero-timeout overload so lock contention is handled immediately via the existing ObserveLockContention path.

                try
                {
                    Monitor.TryEnter(connection, ref locked);
                    if (locked)
                    {

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Files not reviewed (1)

src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs: Generated file

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Improve connection pool failure diagnostics

0d8bbfb

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings August 24, 2026 22:20

mdaigle requested a review from a team as a code owner August 24, 2026 22:20

github-project-automation Bot added this to SqlClient Board Aug 24, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Aug 24, 2026

Copilot started reviewing on behalf of mdaigle August 24, 2026 22:20 View session

mdaigle added this to the 8.0.0-preview1 milestone Aug 24, 2026

Copilot AI reviewed Aug 24, 2026

View reviewed changes

paulmedynski moved this from To triage to In review in SqlClient Board Aug 25, 2026

paulmedynski requested changes Aug 25, 2026

View reviewed changes

github-project-automation Bot moved this from In review to Waiting for customer in SqlClient Board Aug 25, 2026

mdaigle marked this pull request as draft August 25, 2026 16:36

Fix retry ledger test across frameworks

89eda62

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings August 25, 2026 16:41

Copilot started reviewing on behalf of mdaigle August 25, 2026 16:42 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Make retry ledger timeout test framework-neutral

8c555d6

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings August 25, 2026 17:21

Copilot started reviewing on behalf of mdaigle August 25, 2026 17:22 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Rerun flaky Azure SQL checks

b559e97

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Navigation Menu

Uh oh!

Improve connection pool failure diagnostics - #4590

Improve connection pool failure diagnostics#4590
mdaigle wants to merge 4 commits into
dotnet:mainfrom
mdaigle:mdaigle-evaluate-pool-3545

mdaigle commented Aug 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

paulmedynski Aug 25, 2026

Uh oh!

paulmedynski Aug 25, 2026

Uh oh!

paulmedynski Aug 25, 2026

Uh oh!

paulmedynski Aug 25, 2026

Uh oh!

paulmedynski Aug 25, 2026

Uh oh!

paulmedynski Aug 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

mdaigle commented Aug 24, 2026

Description

Issues

Testing

Guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

paulmedynski Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

paulmedynski Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

paulmedynski Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

paulmedynski Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

paulmedynski Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

paulmedynski Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants