From 8d5ea7563d4a103b92c52c295906fb052e52ef7c Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 13 Aug 2026 12:19:42 -0700 Subject: [PATCH 1/6] Enable UseConnectionPoolV2 by default Flip the default value of the UseConnectionPoolV2 AppContext switch from false to true, making the new Channel-based connection pool (ChannelDbConnectionPool) the default implementation. The legacy V1 pool (WaitHandleDbConnectionPool) remains available by explicitly setting the switch to false. - Update XML doc comment on the switch to reflect the new default - Update features.instructions.md default value table - Update LocalAppContextSwitchesTest default-value assertion Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/instructions/features.instructions.md | 2 +- .../src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs | 4 ++-- .../Microsoft/Data/SqlClient/LocalAppContextSwitchesTest.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) 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/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/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); From b75f29f3cf344d4cb218c9044174f0f1beff33aa Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Wed, 19 Aug 2026 16:40:30 -0700 Subject: [PATCH 2/6] Add TODO: run manual pool tests against both implementations Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs | 3 +++ 1 file changed, 3 insertions(+) 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 { From 6941f0c63862bf43f933828acad03f82178c1387 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Thu, 20 Aug 2026 10:02:12 -0700 Subject: [PATCH 3/6] Fix Task unwrap bug in TvpTest.TestPacketNumberWraparound Task.Factory.StartNew with an async lambda returns Task. Without Unwrap(), Task.WhenAny observed only the outer task, which completed as soon as the async lambda hit its first await, rather than waiting for RunPacketNumberWraparound to actually finish. This masked itself under the legacy WaitHandleDbConnectionPool's synchronous-leaning timing, but was exposed by ChannelDbConnectionPool's genuinely asynchronous pooled open path, producing spurious low-enumerator-count failures. Also capture and await the winning task when it is actionTask so any unexpected failure (e.g. a connection open failure) propagates as a real exception instead of surfacing only as a generic count mismatch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ManualTests/SQL/ParameterTest/TvpTest.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) 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, From 00e17ff4f8c98678493a8f38a19550776d48b831 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 21 Aug 2026 08:40:21 -0700 Subject: [PATCH 4/6] Preserve physical open errors after timeout Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../ConnectionPool/ChannelDbConnectionPool.cs | 13 +++--- .../ChannelDbConnectionPoolTest.cs | 40 ++++++++++++++++++- 2 files changed, 46 insertions(+), 7 deletions(-) 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/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index d647ab0914..9ff51ba6e0 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, preserving the legacy pool's connection-error behavior. + /// + [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(); } } From 0fe8944f2de0485c80ec0c487858b71c3f9346b0 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 21 Aug 2026 14:03:55 -0700 Subject: [PATCH 5/6] Rerun CI after infrastructure failures Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> From ead2ea3ac5eb24d422559da16b4b91c603fe7681 Mon Sep 17 00:00:00 2001 From: Malcolm Daigle Date: Fri, 21 Aug 2026 14:07:11 -0700 Subject: [PATCH 6/6] Clarify timeout regression coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs index 9ff51ba6e0..795a598730 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/ChannelDbConnectionPoolTest.cs @@ -590,7 +590,7 @@ out DbConnectionInternal? internalConnection /// /// Verifies that an empty pool still delegates physical connection creation when the caller's - /// timeout budget has just expired, preserving the legacy pool's connection-error behavior. + /// timeout budget has just expired and propagates the physical connection error unchanged. /// [Fact] public void GetConnectionExpiredTimeout_EmptyPoolStillAttemptsPhysicalConnection()