Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down
Loading
Loading