diff --git a/.github/instructions/features.instructions.md b/.github/instructions/features.instructions.md index 34262b8db6..f7f5fe607a 100644 --- a/.github/instructions/features.instructions.md +++ b/.github/instructions/features.instructions.md @@ -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 | diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs index a547ce0fd4..6cf6ab34cc 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs @@ -1046,9 +1046,11 @@ public bool TryGetConnection( /// Opens a new internal connection to the database, throttled by the pool's rate limiter. /// /// The owning connection. - /// The cancellation token to cancel the operation. /// The overall timeout budget. Passed through to the physical connection /// so it uses the remaining budget rather than starting a fresh timeout. + /// 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. /// 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 @@ -1058,8 +1060,8 @@ public bool TryGetConnection( /// private DbConnectionInternal? OpenNewInternalConnection( DbConnection? owningConnection, - CancellationToken cancellationToken, - TimeoutTimer timeout) + TimeoutTimer timeout, + CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); @@ -1507,7 +1509,6 @@ private async Task 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 @@ -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) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 06bf6c4f0e..269b74c97c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -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. /// public static bool UseConnectionPoolV2 => AcquireAndReturn( UseConnectionPoolV2String, - defaultValue: false, + defaultValue: true, ref s_useConnectionPoolV2); /// diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs index a3ae028a5d..4ca10e4938 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs @@ -57,6 +57,9 @@ public IEnumerator 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 { diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs index a8baee786c..6d65189abf 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ParameterTest/TvpTest.cs @@ -65,15 +65,26 @@ public async Task TestPacketNumberWraparound() Stopwatch stopwatch = new(); stopwatch.Start(); + // Task.Factory.StartNew with an async delegate returns a 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, diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index d647ab0914..795a598730 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -588,6 +588,31 @@ out DbConnectionInternal? internalConnection Assert.Equal(ADP.PooledOpenTimeout().Message, ex.Message); } + /// + /// 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. + /// + [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(() => + pool.TryGetConnection( + new SqlConnection(), + taskCompletionSource: null, + TimeoutTimer.StartExpired(), + out _)); + + // Assert + Assert.Same(physicalConnectionException, exception); + Assert.Equal(1, connectionFactory.CreateCount); + } + /// /// Verifies under concurrent synchronous load that the pool never grows beyond its /// configured maximum size and continues to serve requests safely. @@ -1427,6 +1452,7 @@ protected override DbConnectionInternal CreateConnection( } } + /// /// Test connection factory that always throws the pooled-open timeout to exercise failure /// paths in the pool. @@ -2182,6 +2208,18 @@ protected override DbConnectionInternal CreateConnection( /// internal sealed class CountingTimeoutConnectionFactory : SqlConnectionFactory { + private readonly Exception? _exception; + + /// + /// Creates a factory that throws either the supplied marker exception or the standard + /// pooled-open timeout when physical connection creation is requested. + /// + /// Optional exception to throw from physical creation. + internal CountingTimeoutConnectionFactory(Exception? exception = null) + { + _exception = exception; + } + /// /// Gets the number of times the pool asked the factory to create a physical connection. /// @@ -2200,7 +2238,7 @@ protected override DbConnectionInternal CreateConnection( TimeoutTimer timeout) { CreateCount++; - throw ADP.PooledOpenTimeout(); + throw _exception ?? ADP.PooledOpenTimeout(); } } diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs index ff70c17f4b..f0ff098bd1 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs @@ -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);