Add pool tracing/metrics parity and surface pooled-open timeout cause

src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs

@@ -1337,6 +1337,19 @@ internal static Exception UndefinedPopulationMechanism(string populationMechanis
         internal static Exception PooledOpenTimeout()
             => ADP.InvalidOperation(StringsHelper.GetString(Strings.ADP_PooledOpenTimeout));
 
+        /// <summary>
+        /// Builds the pooled-open timeout exception, attaching <paramref name="inner"/> (the most
+        /// recent physical connection creation failure observed by the pool) so a timeout caused by
+        /// repeated connection failures reports the underlying cause rather than only reporting
+        /// pool exhaustion. Falls back to the parameterless form when there is no such failure.
+        /// </summary>
+#nullable enable
+        internal static Exception PooledOpenTimeout(Exception? inner)
+            => inner is null
+                ? PooledOpenTimeout()
+                : ADP.InvalidOperation(StringsHelper.GetString(Strings.ADP_PooledOpenTimeout), inner);
+#nullable restore
+
         internal static Exception NonPooledOpenTimeout()
             => ADP.TimeoutException(StringsHelper.GetString(Strings.ADP_NonPooledOpenTimeout));
 #endregion

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs

@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Threading;
 using Microsoft.Data.ProviderBase;
@@ -60,6 +61,7 @@ internal void Keep()
         private readonly DbConnectionInternal?[] _connections;
         private readonly uint _capacity;
         private volatile int _reservations;
+        private volatile int _connectionCount;
 
         /// <summary>
         /// Constructs a ConnectionPoolSlots instance with the given fixed capacity.
@@ -82,14 +84,23 @@ internal ConnectionPoolSlots(uint fixedCapacity)
 
             _capacity = fixedCapacity;
             _reservations = 0;
+            _connectionCount = 0;
             _connections = new DbConnectionInternal?[fixedCapacity];
         }
 
         /// <summary>
-        /// Gets the total number of reservations currently held.
+        /// Gets the total number of reservations currently held. This includes reservations held on
+        /// behalf of connections that are still being opened and are therefore not yet tracked.
         /// </summary>
         internal int ReservationCount => _reservations;
 
+        /// <summary>
+        /// Gets the number of connections currently tracked by this collection. Unlike
+        /// <see cref="ReservationCount"/>, this excludes reservations held for connections that are
+        /// still being opened, so it reports connections that actually belong to the pool.
+        /// </summary>
+        internal int ConnectionCount => _connectionCount;
+
         /// <summary>
         /// Adds a connection to the collection.
         /// </summary>
@@ -127,6 +138,7 @@ internal ConnectionPoolSlots(uint fixedCapacity)
                 {
                     if (Interlocked.CompareExchange(ref _connections[i], connection, null) == null)
                     {
+                        Interlocked.Increment(ref _connectionCount);
                         reservation.Keep();
                         return connection;
                     }
@@ -162,6 +174,7 @@ internal bool TryRemove(DbConnectionInternal connection)
             {
                 if (Interlocked.CompareExchange(ref _connections[i], null, connection) == connection)
                 {
+                    Interlocked.Decrement(ref _connectionCount);
                     ReleaseReservation();
                     return true;
                 }
@@ -170,6 +183,48 @@ internal bool TryRemove(DbConnectionInternal connection)
             return false;
         }
 
+        /// <summary>
+        /// Atomically replaces an existing connection with a new one in the same slot.
+        /// The reservation count is unchanged because the slot is reused.
+        /// </summary>
+        /// <param name="oldConnection">The connection currently occupying the slot.</param>
+        /// <param name="newConnection">The connection to place into the slot.</param>
+        /// <returns>True if the old connection was found and replaced; otherwise, false.</returns>
+        internal bool TryReplace(DbConnectionInternal oldConnection, DbConnectionInternal newConnection)
+        {
+            for (int i = 0; i < _connections.Length; i++)
+            {
+                if (Interlocked.CompareExchange(ref _connections[i], newConnection, oldConnection) == oldConnection)
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        /// <summary>
+        /// Returns a point-in-time snapshot of the connections currently tracked by this collection.
+        /// The snapshot is best-effort: connections may be added or removed while it is being taken,
+        /// so callers must tolerate entries that have since left the pool. Intended for infrequent
+        /// bookkeeping passes (e.g. reclaiming emancipated connections), not for hot paths.
+        /// </summary>
+        internal List<DbConnectionInternal> Snapshot()
+        {
+            List<DbConnectionInternal> snapshot = new(_connections.Length);
+
+            for (int i = 0; i < _connections.Length; i++)
+            {
+                DbConnectionInternal? connection = Volatile.Read(ref _connections[i]);
+                if (connection is not null)
+                {
+                    snapshot.Add(connection);
+                }
+            }
+
+            return snapshot;
+        }
+
         /// <summary>
         /// Attempts to reserve a spot in the collection.
         /// </summary>

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IDbConnectionPool.cs

@@ -49,6 +49,19 @@ internal interface IDbConnectionPool
         /// TODO: rename to indicate that this relates to the blocking period
         bool ErrorOccurred { get; }
 
+        /// <summary>
+        /// The exception thrown by the most recent failed attempt to open a physical connection,
+        /// or null if no attempt has failed since the last successful open.
+        /// <para>
+        /// A caller that waits for a pooled connection and ultimately times out cannot otherwise
+        /// tell whether the pool was merely saturated or whether every creation attempt behind the
+        /// scenes was failing (e.g. the server refused the TCP connection). This property lets the
+        /// timeout be reported with the underlying failure attached as an inner exception. It is
+        /// diagnostic only and is not used to make control-flow decisions.
+        /// </para>
+        /// </summary>
+        Exception? LastConnectionCreateException { get; }
+
         /// <summary>
         /// An id that uniqely identifies this connection pool.
         /// </summary>

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/IdleConnectionChannel.cs

@@ -56,6 +56,7 @@ internal bool TryWrite(DbConnectionInternal? connection)
                 if (connection is not null)
                 {
                     Interlocked.Increment(ref _count);
+                    SqlClientDiagnostics.Metrics.EnterFreeConnection();
                 }
                 return true;
             }
@@ -74,6 +75,7 @@ internal bool TryRead(out DbConnectionInternal? connection)
                 if (connection is not null)
                 {
                     Interlocked.Decrement(ref _count);
+                    SqlClientDiagnostics.Metrics.ExitFreeConnection();
                 }
 
                 return true;
@@ -93,6 +95,7 @@ internal bool TryRead(out DbConnectionInternal? connection)
             if (connection is not null)
             {
                 Interlocked.Decrement(ref _count);
+                SqlClientDiagnostics.Metrics.ExitFreeConnection();
             }
 
             return connection;

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

@@ -195,6 +195,15 @@ public void Dispose()
         private readonly TimeProvider _timeProvider;
         private readonly BlockingPeriodErrorState _errorState;
 
+        /// <summary>
+        /// The exception from the most recent failed physical connection open, retained purely so
+        /// that a subsequent pooled-open timeout can report it as an inner exception. Cleared on the
+        /// next successful open. Volatile rather than lock-protected: this is a best-effort
+        /// diagnostic snapshot, and a torn read across concurrent failures would at worst attach a
+        /// slightly older failure. See GH#3545.
+        /// </summary>
+        private volatile Exception _lastConnectionCreateException;
+
         internal Timer _cleanupTimer;
 
         private readonly TransactedConnectionPool _transactedConnectionPool;
@@ -288,6 +297,9 @@ private int CreationTimeout
 
         public bool ErrorOccurred => _errorState.HasError;
 
+        /// <inheritdoc/>
+        public Exception LastConnectionCreateException => _lastConnectionCreateException;
+
         private bool HasTransactionAffinity => PoolGroupOptions.HasTransactionAffinity;
 
         public TimeSpan LoadBalanceTimeout => PoolGroupOptions.LoadBalanceTimeout;
@@ -551,13 +563,23 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio
 
                 SqlClientEventSource.Log.TryPoolerTraceEvent("<prov.DbConnectionPool.CreateObject|RES|CPOOL> {0}, Connection {1}, Added to pool.", Id, newObj?.ObjectID);
 
+                // A successful open proves the server is reachable, so a previously recorded
+                // failure is no longer a useful explanation for a later timeout. See GH#3545.
+                _lastConnectionCreateException = null;
+
                 // A successful creation clears any prior error state and resets backoff.
                 _errorState.Clear();
             }
             catch (Exception e) when (ADP.IsCatchableExceptionType(e))
             {
                 ADP.TraceExceptionWithoutRethrow(e);
 
+                // Retain the failure so a caller that ultimately times out waiting for a pooled
+                // connection can report why creation kept failing. Recorded before the
+                // blocking-period check below so it is captured even when blocking is disabled
+                // and this method rethrows immediately. See GH#3545.
+                _lastConnectionCreateException = e;
+
                 if (!_connectionPoolGroup.IsBlockingPeriodEnabled())
                 {
                     throw;
@@ -809,7 +831,8 @@ private void WaitForPendingOpen()
                         }
                         else if (timeout)
                         {
-                            next.Completion.TrySetException(ADP.ExceptionWithStackTrace(ADP.PooledOpenTimeout()));
+                            next.Completion.TrySetException(
+                                ADP.ExceptionWithStackTrace(ADP.PooledOpenTimeout(_lastConnectionCreateException)));
                         }
                         else
                         {

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs

@@ -1616,7 +1616,9 @@ public void Open(SqlConnectionOverrides overrides)
                 {
                     statistics = SqlStatistics.StartTimer(Statistics);
 
-                    if (!(IsProviderRetriable ? TryOpenWithRetry(null, false, overrides) : TryOpen(null, false, overrides)))
+                    if (!(IsProviderRetriable ?
+                            TryOpenWithRetry(retry: null, forceNewConnection: false, overrides: overrides) :
+                            TryOpen(retry: null, forceNewConnection: false, overrides: overrides)))
                     {
                         throw ADP.InternalError(ADP.InternalErrorCode.SynchronousConnectReturnedPending);
                     }
@@ -2252,16 +2254,22 @@ private bool TryOpen(TaskCompletionSource<DbConnectionInternal> retry, bool forc
         /// Completes the inner open/replace operation and initializes parser state for the active inner connection.
         /// </summary>
         /// <param name="retry">Retry continuation used by async open paths.</param>
-        /// <param name="forceNewConnection">Provide true to forcibly overwrite the existing connection. Provide false if connecting for the first time.</param>
+        /// <param name="forceNewConnection">Provide <see langword="true"/> to replace the existing inner connection with a freshly established one (for example, during reconnect after a transient fault); provide <see langword="false"/> when opening for the first time.</param>
         /// <returns><see langword="true"/> when open initialization completed synchronously; otherwise <see langword="false"/>.</returns>
         /// <remarks>
         /// The inner connection is snapshotted after the open call so downstream parser access uses a single observed
         /// instance and does not rely on a second racy read of <see cref="InnerConnection"/>.
-        /// 
-        /// forceNewConnection may only be true when the connection is already open (or was open) and needs to be replaced. If the connection has never
-        /// been opened, passing true will result in an exception. It may only be false when the connection has never been opened or is
-        /// currently disconnected. If the connection is currently open, passing false will result in an exception. See SqlConnection state
-        /// transitions and subclasses for more details.
+        /// <para>
+        /// <paramref name="forceNewConnection"/> may be <see langword="true"/> when the connection is currently open, or when
+        /// it was previously opened and is now disconnected (the reconnect case handled by
+        /// <c>DbConnectionClosedPreviouslyOpened</c> and <c>DbConnectionClosedConnecting</c>). Passing <see langword="true"/>
+        /// on a connection that has never been opened will result in an exception.
+        /// </para>
+        /// <para>
+        /// <paramref name="forceNewConnection"/> may be <see langword="false"/> when the connection has never been opened or is
+        /// currently disconnected. Passing <see langword="false"/> on a connection that is already open will result in an
+        /// exception.
+        /// </para>
         /// </remarks>
         internal bool TryOpenInner(TaskCompletionSource<DbConnectionInternal> retry, bool forceNewConnection)
         {

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionFactory.cs

@@ -337,6 +337,12 @@ internal bool TryGetConnection(
             int retriesLeft = 10;
             int timeBetweenRetriesMilliseconds = 1;
 
+            // Tracks the most recent physical connection failure observed by the pool we last
+            // consulted, so a pooled-open timeout can report it as an inner exception rather than
+            // only reporting pool exhaustion. Hoisted out of the loop because the final give-up
+            // throw below is outside the pool variable's scope. See GH#3545.
+            Exception lastConnectionCreateException = null;
+
             do
             {
                 DbConnectionPoolGroup poolGroup = GetConnectionPoolGroup(owningConnection);
@@ -436,12 +442,14 @@ internal bool TryGetConnection(
 
                     if (connection is null)
                     {
+                        lastConnectionCreateException = connectionPool.LastConnectionCreateException;
+
                         // connection creation failed on semaphore waiting or if max pool reached
                         if (connectionPool.IsRunning)
                         {
                             SqlClientEventSource.Log.TryTraceEvent("<prov.SqlConnectionFactory.GetConnection|RES|CPOOL> {0}, GetConnection failed because a pool timeout occurred.", ObjectId);
                             // If GetConnection failed while the pool is running, the pool timeout occurred.
-                            throw ADP.PooledOpenTimeout();
+                            throw ADP.PooledOpenTimeout(lastConnectionCreateException);
                         }
 
                         // We've hit the race condition, where the pool was shut down after we
@@ -458,7 +466,7 @@ internal bool TryGetConnection(
             {
                 SqlClientEventSource.Log.TryTraceEvent("<prov.SqlConnectionFactory.GetConnection|RES|CPOOL> {0}, GetConnection failed because a pool timeout occurred and all retries were exhausted.", ObjectId);
                 // exhausted all retries or timed out - give up
-                throw ADP.PooledOpenTimeout();
+                throw ADP.PooledOpenTimeout(lastConnectionCreateException);
             }
 
             return true;

src/Microsoft.Data.SqlClient/src/Resources/Strings.resx

@@ -2157,6 +2157,9 @@
   <data name="SQL_ConnectionPoolNoEmptySlot" xml:space="preserve">
     <value>Could not find an empty slot in the connection pool.</value>
   </data>
+  <data name="SQL_ConnectionPoolReplaceConnectionFailed" xml:space="preserve">
+    <value>Could not replace the connection because it is no longer in the connection pool.</value>
+  </data>
   <data name="SQL_ConnectionPoolShutDown" xml:space="preserve">
     <value>The connection pool has been shut down.</value>
   </data>

src/Microsoft.Data.SqlClient/tests/Common/ConnectionPoolVersionScope.cs

@@ -0,0 +1,62 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace Microsoft.Data.SqlClient.Tests.Common;
+
+/// <summary>
+/// Selects the connection pool implementation (<c>WaitHandleDbConnectionPool</c> or
+/// <c>ChannelDbConnectionPool</c>) for the duration of a test.
+///
+/// A pool is bound to an implementation when it is created, so simply flipping the
+/// <c>UseConnectionPoolV2</c> switch is not enough: pools created before the switch was flipped
+/// keep their original implementation, and pools created inside the scope would otherwise outlive
+/// it and leak the chosen implementation into unrelated tests. This scope therefore clears all
+/// pools both on entry and on exit.
+///
+/// This follows the RAII pattern; construct it at the start of a test and dispose it at the end.
+/// Like <see cref="LocalAppContextSwitchesHelper"/>, it manipulates global state and enforces a
+/// single-instance policy, so it must not be held for longer than necessary.
+/// </summary>
+public sealed class ConnectionPoolVersionScope : IDisposable
+{
+    private readonly LocalAppContextSwitchesHelper _switches;
+
+    /// <summary>
+    /// Clears all existing pools and selects the requested pool implementation.
+    /// </summary>
+    /// <param name="usePoolV2">
+    /// True to use <c>ChannelDbConnectionPool</c>; false to use <c>WaitHandleDbConnectionPool</c>.
+    /// </param>
+    public ConnectionPoolVersionScope(bool usePoolV2)
+    {
+        _switches = new LocalAppContextSwitchesHelper();
+
+        try
+        {
+            SqlConnection.ClearAllPools();
+            _switches.UseConnectionPoolV2 = usePoolV2;
+        }
+        catch
+        {
+            _switches.Dispose();
+            throw;
+        }
+    }
+
+    /// <summary>
+    /// Clears all pools created under the selected implementation and restores the original
+    /// switch values.
+    /// </summary>
+    public void Dispose()
+    {
+        try
+        {
+            SqlConnection.ClearAllPools();
+        }
+        finally
+        {
+            _switches.Dispose();
+        }
+    }
+}