Skip to content
Open
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
2 changes: 1 addition & 1 deletion .github/instructions/features.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ AppContext switches allow runtime behavior changes without modifying connection
| `Switch.Microsoft.Data.SqlClient.TruncateScaledDecimal` | `false` | Truncates scaled decimal values instead of rounding |
| `Switch.Microsoft.Data.SqlClient.UseCompatibilityAsyncBehaviour` | `false` | Uses legacy async behavior for compatibility |
| `Switch.Microsoft.Data.SqlClient.UseCompatibilityProcessSni` | `false` | Uses legacy SNI processing path |
| `Switch.Microsoft.Data.SqlClient.UseConnectionPoolV2` | `false` | Enables the new `ChannelDbConnectionPool` implementation |
| `Switch.Microsoft.Data.SqlClient.UseConnectionPoolV2` | `true` | Enables the new `ChannelDbConnectionPool` implementation; set to `false` to restore the legacy `WaitHandleDbConnectionPool` |
| `Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows` | `false` | Forces managed SNI on Windows (instead of native SNI) |
| `Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin` | `false` | Sets 1-second minimum in login timeout calculations |

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1046,9 +1046,11 @@ public bool TryGetConnection(
/// Opens a new internal connection to the database, throttled by the pool's rate limiter.
/// </summary>
/// <param name="owningConnection">The owning connection.</param>
/// <param name="cancellationToken">The cancellation token to cancel the operation.</param>
/// <param name="timeout">The overall timeout budget. Passed through to the physical connection
/// so it uses the remaining budget rather than starting a fresh timeout.</param>
/// <param name="cancellationToken">An optional cancellation token used by background warmup.
/// Caller timeout cancellation is reserved for pool waits so physical connection failures
/// retain the same exception behavior as the legacy pool.</param>
/// <returns>The new internal connection, or null if the pool has no available slot or the
/// rate limiter is currently saturated. In the latter case the caller should fall back to
/// the idle-channel wait; the rate limiter will write a null to the idle channel when a
Expand All @@ -1058,8 +1060,8 @@ public bool TryGetConnection(
/// </exception>
private DbConnectionInternal? OpenNewInternalConnection(
DbConnection? owningConnection,
CancellationToken cancellationToken,
TimeoutTimer timeout)
TimeoutTimer timeout,
CancellationToken cancellationToken = default)
{
cancellationToken.ThrowIfCancellationRequested();

Expand Down Expand Up @@ -1507,7 +1509,6 @@ private async Task<DbConnectionInternal> GetInternalConnection(
// in either case the caller falls through to the idle-channel wait below.
connection ??= OpenNewInternalConnection(
owningConnection,
cancellationToken,
timeout);

// If we're at max capacity and couldn't open a connection. Block on the idle channel with a
Expand Down Expand Up @@ -1939,8 +1940,8 @@ private async Task RunWarmupLoopAsync()
// saturated; a thrown exception means the physical open genuinely failed.
connection = OpenNewInternalConnection(
owningConnection: null,
cancellationToken: token,
timeout: timeout);
timeout: timeout,
cancellationToken: token);
}
catch (OperationCanceledException)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,12 +580,12 @@ public static bool UseCompatibilityAsyncBehaviour
/// pool implementation. When set to false, the connection pool will use
/// the legacy V1 implementation.
///
/// The default value of this switch is false.
/// The default value of this switch is true.
/// </summary>
public static bool UseConnectionPoolV2 =>
AcquireAndReturn(
UseConnectionPoolV2String,
defaultValue: false,
defaultValue: true,
ref s_useConnectionPoolV2);

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ public IEnumerator<object[]> GetEnumerator()
}

// TODO Synapse: Fix these tests for Azure Synapse.
// TODO PoolV2: All manual connection-pool tests should eventually run against both pool
// implementations (legacy WaitHandleDbConnectionPool and ChannelDbConnectionPool), not just
// whichever UseConnectionPoolV2 defaults to.
[Trait("Set", "3")]
public static class ConnectionPoolTest
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,26 @@ public async Task TestPacketNumberWraparound()
Stopwatch stopwatch = new();
stopwatch.Start();

// Task.Factory.StartNew with an async delegate returns a Task<Task>, so it must be
// unwrapped before use in Task.WhenAny below. Without Unwrap(), WhenAny would observe
// only the outer task (which completes as soon as the async lambda hits its first
// await) instead of the actual completion of RunPacketNumberWraparound.
Task actionTask = Task.Factory.StartNew(
async () => await RunPacketNumberWraparound(enumerator, cancellationTokenSource.Token),
TaskCreationOptions.DenyChildAttach | TaskCreationOptions.LongRunning);
() => RunPacketNumberWraparound(enumerator, cancellationTokenSource.Token),
TaskCreationOptions.DenyChildAttach | TaskCreationOptions.LongRunning).Unwrap();
Task timeoutTask = Task.Delay(TimeSpan.FromSeconds(60), cancellationTokenSource.Token);
await Task.WhenAny(actionTask, timeoutTask);
Task completedTask = await Task.WhenAny(actionTask, timeoutTask);

stopwatch.Stop();
cancellationTokenSource.Cancel();

// Propagate any unexpected failure from the action task (e.g. a connection open
// failure) instead of letting it surface only as a low enumerator count below.
if (completedTask == actionTask)
{
await actionTask;
}

// Assert
Assert.True(
enumerator.MaxCount == enumerator.Count,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,31 @@ out DbConnectionInternal? internalConnection
Assert.Equal(ADP.PooledOpenTimeout().Message, ex.Message);
}

/// <summary>
/// Verifies that an empty pool still delegates physical connection creation when the caller's
/// timeout budget has just expired and propagates the physical connection error unchanged.
/// </summary>
[Fact]
public void GetConnectionExpiredTimeout_EmptyPoolStillAttemptsPhysicalConnection()
{
// Arrange
var physicalConnectionException = new NotSupportedException("Physical connection failed.");
var connectionFactory = new CountingTimeoutConnectionFactory(physicalConnectionException);
var pool = ConstructPool(connectionFactory);

// Act
NotSupportedException exception = Assert.Throws<NotSupportedException>(() =>
pool.TryGetConnection(
new SqlConnection(),
taskCompletionSource: null,
TimeoutTimer.StartExpired(),
out _));

// Assert
Assert.Same(physicalConnectionException, exception);
Assert.Equal(1, connectionFactory.CreateCount);
}

/// <summary>
/// Verifies under concurrent synchronous load that the pool never grows beyond its
/// configured maximum size and continues to serve requests safely.
Expand Down Expand Up @@ -1427,6 +1452,7 @@ protected override DbConnectionInternal CreateConnection(
}
}


/// <summary>
/// Test connection factory that always throws the pooled-open timeout to exercise failure
/// paths in the pool.
Expand Down Expand Up @@ -2182,6 +2208,18 @@ protected override DbConnectionInternal CreateConnection(
/// </summary>
internal sealed class CountingTimeoutConnectionFactory : SqlConnectionFactory
{
private readonly Exception? _exception;

/// <summary>
/// Creates a factory that throws either the supplied marker exception or the standard
/// pooled-open timeout when physical connection creation is requested.
/// </summary>
/// <param name="exception">Optional exception to throw from physical creation.</param>
internal CountingTimeoutConnectionFactory(Exception? exception = null)
{
_exception = exception;
}

/// <summary>
/// Gets the number of times the pool asked the factory to create a physical connection.
/// </summary>
Expand All @@ -2200,7 +2238,7 @@ protected override DbConnectionInternal CreateConnection(
TimeoutTimer timeout)
{
CreateCount++;
throw ADP.PooledOpenTimeout();
throw _exception ?? ADP.PooledOpenTimeout();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void TestDefaultAppContextSwitchValues()
Assert.True(switchesHelper.UseCompatibilityProcessSni);
Assert.True(switchesHelper.UseCompatibilityAsyncBehaviour);
Assert.True(switchesHelper.UseLegacyIdleTimeoutBehavior);
Assert.False(switchesHelper.UseConnectionPoolV2);
Assert.True(switchesHelper.UseConnectionPoolV2);
Assert.False(switchesHelper.UseOverallConnectTimeoutForPoolWait);
Assert.False(switchesHelper.TruncateScaledDecimal);
Assert.False(switchesHelper.IgnoreServerProvidedFailoverPartner);
Expand Down