Flaky tests / lighter test queries

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

@@ -357,7 +357,21 @@ public bool IsRunning
 
         public TransactedConnectionPool TransactedConnectionPool => _transactedConnectionPool;
 
-        internal void CleanupCallback(object state)
+        /// <summary>
+        /// Periodic pruning callback invoked by <see cref="_cleanupTimer"/>. Destroys idle
+        /// connections that have aged out of the general pool (those above
+        /// <see cref="MinPoolSize"/> that have been on <c>_stackOld</c> for a full cleanup
+        /// period) and promotes the remaining <c>_stackNew</c> entries onto <c>_stackOld</c>
+        /// so they become eligible for pruning on the next tick. Connections held in the
+        /// <see cref="TransactedConnectionPool"/> are not touched here; the transaction-end
+        /// event is responsible for releasing them.
+        /// </summary>
+        /// <remarks>
+        /// Exposed as <c>internal</c> (rather than <c>private</c>) solely so unit tests can
+        /// invoke a deterministic prune cycle without waiting on the cleanup timer.
+        /// </remarks>
+#nullable enable annotations
+        internal void CleanupCallback(object? state)
         {
             // If the pool is not Running, skip work. Shutdown disposes the timer, but
             // a callback may already be in-flight when Shutdown runs; this guard ensures it does
@@ -483,6 +497,7 @@ internal void CleanupCallback(object state)
             // Queue up a request to bring us up to MinPoolSize
             QueuePoolCreateRequest();
         }
+#nullable restore annotations
 
         public void Clear()
         {

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs

@@ -81,54 +81,5 @@ public static async Task BasicTransactionPoolTest(string connectionString)
 
             Assert.Equal(2, connectionPool.ConnectionCount);
         }
-
-        /// <summary>
-        /// Checks that connections in the transaction pool are not cleaned out, and the root transaction is put into "stasis" when it ages
-        /// Synapse: only supports local transaction request.
-        /// </summary>
-        /// <param name="connectionString"></param>
-        //
-        // Flaky under CI load only: the connection pool intermittently reports one more
-        // connection than expected because the process-global pool (keyed by connection string)
-        // is contaminated by a connection opened elsewhere while this test runs, i.e. it is a
-        // test-isolation / pool-count race, not a product defect. It cannot be made deterministic
-        // without isolating the shared pool, so it is quarantined until that isolation is added.
-        //
-        //     Failed Microsoft.Data.SqlClient.ManualTesting.Tests.TransactionPoolTest.TransactionCleanupTest(connectionString: "Data Source=tcp:localhost;Initial Catalog=Northwin"...) [2 s]
-        //   Assert.Equal() Failure: Values differ
-        //     Expected: 2
-        //     Actual:   3
-        //     at Microsoft.Data.SqlClient.ManualTesting.Tests.TransactionPoolTest.TransactionCleanupTest(String connectionString) in TransactionPoolTest.cs:line 101
-        [Trait("category", "flaky")]
-        [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
-        [ClassData(typeof(ConnectionPoolConnectionStringProvider))]
-        public static async Task TransactionCleanupTest(string connectionString)
-        {
-            SqlConnection.ClearAllPools();
-            ConnectionPoolWrapper connectionPool = null;
-
-            using (TransactionScope transScope = new(TransactionScopeAsyncFlowOption.Enabled))
-            {
-                using SqlConnection connection1 = new(connectionString);
-                using SqlConnection connection2 = new(connectionString);
-                await OpenWithAccessTokenAsync(connection1);
-                await OpenWithAccessTokenAsync(connection2);
-                InternalConnectionWrapper internalConnection1 = new(connection1);
-                connectionPool = new ConnectionPoolWrapper(connection1);
-
-                connectionPool.Cleanup();
-                Assert.Equal(2, connectionPool.ConnectionCount);
-
-                connection1.Close();
-                connection2.Close();
-                connectionPool.Cleanup();
-                Assert.Equal(2, connectionPool.ConnectionCount);
-
-                connectionPool.Cleanup();
-                Assert.Equal(2, connectionPool.ConnectionCount);
-
-                transScope.Complete();
-            }
-        }
     }
 }

src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs

@@ -47,7 +47,8 @@ public void Dispose()
     private WaitHandleDbConnectionPool CreatePool(
         int maxPoolSize = DefaultMaxPoolSize,
         int minPoolSize = DefaultMinPoolSize,
-        bool hasTransactionAffinity = true)
+        bool hasTransactionAffinity = true,
+        int idleTimeout = 0)
     {
         var poolGroupOptions = new DbConnectionPoolGroupOptions(
             poolByIdentity: false,
@@ -56,7 +57,7 @@ private WaitHandleDbConnectionPool CreatePool(
             creationTimeout: DefaultCreationTimeoutInMilliseconds,
             loadBalanceTimeout: 0,
             hasTransactionAffinity: hasTransactionAffinity,
-            idleTimeout: 0
+            idleTimeout: idleTimeout
         );
 
         var dbConnectionPoolGroup = new DbConnectionPoolGroup(
@@ -915,6 +916,76 @@ public void SequentialTransactions_CanReuseConnections()
 
     #endregion
 
+    #region Pruning Tests
+
+    /// <summary>
+    /// Verifies that cleanup prunes general idle connections but preserves and reuses
+    /// connections held by an active transaction.
+    /// </summary>
+    [Fact]
+    public void Pruning_IgnoresTransactedConnections()
+    {
+        _pool.Shutdown();
+        _pool.Clear();
+        _pool = CreatePool(idleTimeout: 60);
+
+        // Arrange - place a connection into the transacted pool by returning it
+        // while a transaction is active. The connection lives in
+        // TransactedConnectionPool, not in the general pool's _stackOld/_stackNew.
+        using var scope = new TransactionScope();
+        var transaction = Transaction.Current;
+        Assert.NotNull(transaction);
+
+        var owner = new SqlConnection();
+        var conn = GetConnection(owner);
+        Assert.NotNull(conn);
+        ReturnConnection(conn, owner);
+
+        // Sanity: connection sits in the transacted pool, not the general pool.
+        Assert.Single(_pool.TransactedConnectionPool.TransactedConnections);
+        Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]);
+        Assert.Equal(0, _pool.IdleCount);
+
+        // A general idle connection proves cleanup actually prunes rather than being disabled.
+        using (var suppressedScope = new TransactionScope(TransactionScopeOption.Suppress))
+        {
+            using var idleOwner = new SqlConnection();
+            var idleConnection = GetConnection(idleOwner);
+            Assert.NotNull(idleConnection);
+            ReturnConnection(idleConnection, idleOwner);
+            suppressedScope.Complete();
+        }
+        Assert.Equal(1, _pool.IdleCount);
+        int poolCountBefore = _pool.Count;
+
+        // Act - invoke pruning twice. The cleanup pass moves connections from
+        // _stackNew to _stackOld on one tick and destroys aged entries on the
+        // next, so running it twice mirrors a full prune cycle.
+        var waitHandlePool = (WaitHandleDbConnectionPool)_pool;
+        waitHandlePool.CleanupCallback(null);
+        waitHandlePool.CleanupCallback(null);
+
+        // Assert - the transacted connection must still be tracked in the
+        // transacted pool and must not have been destroyed.
+        Assert.Single(_pool.TransactedConnectionPool.TransactedConnections);
+        Assert.True(_pool.TransactedConnectionPool.TransactedConnections.ContainsKey(transaction));
+        Assert.Single(_pool.TransactedConnectionPool.TransactedConnections[transaction]);
+        Assert.Equal(0, _pool.IdleCount);
+        Assert.Equal(poolCountBefore - 1, _pool.Count);
+        Assert.False(conn.IsConnectionDoomed,
+            "Transacted connection should not be doomed by the pruning process.");
+
+        // The transacted connection must still be reusable for the same transaction.
+        var owner2 = new SqlConnection();
+        var conn2 = GetConnection(owner2);
+        Assert.Same(conn, conn2);
+        ReturnConnection(conn2, owner2);
+
+        scope.Complete();
+    }
+
+    #endregion
+
     #region Mock Classes
 
     internal class MockSqlConnectionFactory : SqlConnectionFactory

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs

@@ -747,6 +747,12 @@ public async Task TransientFault_Async_ShouldConnectToPrimary_NotFailover(uint e
         [InlineData(40613)]
         [InlineData(42108)]
         [InlineData(42109)]
+        // Quarantined due to intermittent failure:
+        //     Assert.Equal() Failure: Strings differ
+        //                            v (pos 14)
+        //   Expected: "localhost,56862"
+        //   Actual:   "localhost,56861"
+        [Trait("Category", "flaky")]
         public async Task TransientFault_WithUserProvidedPartner_Async_ShouldConnectToPrimary_NotFailover(uint errorCode)
         {
             // Async parity for TransientFault_WithUserProvidedPartner_ShouldConnectToPrimary.

src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs

@@ -98,6 +98,8 @@ public async Task RequestEncryption_ServerDoesNotSupportEncryption_ShouldFail()
         [InlineData(40613)]
         [InlineData(42108)]
         [InlineData(42109)]
+        // Quarantine tracked in https://github.com/dotnet/SqlClient/pull/4344.
+        // The original intermittent async retry failure did not retain assertion/exception details.
         public async Task TransientFault_RetryEnabled_ShouldSucceed_Async(uint errorCode)
         {
             using TransientTdsErrorTdsServer server = new(
@@ -183,6 +185,11 @@ public async Task TransientFault_RetryDisabled_ShouldFail_Async(uint errorCode)
         [InlineData(40613)]
         [InlineData(42108)]
         [InlineData(42109)]
+        // Quarantined due to intermittent failure:
+        //   Assert.Equal() Failure: Values differ
+        //   Expected: 40613
+        //   Actual:   42108
+        [Trait("Category", "flaky")]
         public void TransientFault_RetryDisabled_ShouldFail(uint errorCode)
         {
             using TransientTdsErrorTdsServer server = new(