diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs index ecf8840ebb..5b40a38103 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs @@ -329,7 +329,20 @@ public bool IsRunning public TransactedConnectionPool TransactedConnectionPool => _transactedConnectionPool; - private void CleanupCallback(object state) + /// + /// Periodic pruning callback invoked by . Destroys idle + /// connections that have aged out of the general pool (those above + /// that have been on _stackOld for a full cleanup + /// period) and promotes the remaining _stackNew entries onto _stackOld + /// so they become eligible for pruning on the next tick. Connections held in the + /// are not touched here — the transaction-end + /// event is responsible for releasing them. + /// + /// + /// Exposed as internal (rather than private) solely so unit tests can + /// invoke a deterministic prune cycle without waiting on the cleanup timer. + /// + internal void CleanupCallback(object state) { // Called when the cleanup-timer ticks over. diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs index 6e358cc63a..45f57466aa 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/TransactionPoolTest.cs @@ -67,41 +67,5 @@ public static void BasicTransactionPoolTest(string connectionString) Assert.Equal(2, connectionPool.ConnectionCount); } - - /// - /// 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. - /// - /// - [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))] - [ClassData(typeof(ConnectionPoolConnectionStringProvider))] - public static void TransactionCleanupTest(string connectionString) - { - SqlConnection.ClearAllPools(); - ConnectionPoolWrapper connectionPool = null; - - using (TransactionScope transScope = new()) - { - using SqlConnection connection1 = new(connectionString); - using SqlConnection connection2 = new(connectionString); - connection1.Open(); - connection2.Open(); - 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(); - } - } } } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs index 14b5ab54c4..bd278b7d26 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/MARSSessionPoolingTest/MarsSessionPoolingTest.cs @@ -376,7 +376,14 @@ private static DisposableArray GetCommands(SqlConnection connection, switch (commandType) { case CommandType.Text: - string commandText = string.Join(" ", Enumerable.Repeat(@"SELECT * FROM sys.databases;", 20)); + // 100 rows from an in-memory VALUES generator, repeated as 20 result sets. + // Produces enough output to span multiple 512-byte TDS packets (so the + // request stays observable in dm_exec_requests for the MARS tests) without + // the per-call CPU cost of scanning sys.databases on Azure SQL. + const string rowGen = + "SELECT a.n FROM (VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10)) AS a(n) " + + "CROSS JOIN (VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10)) AS b(n);"; + string commandText = string.Join(" ", Enumerable.Repeat(rowGen, 20)); commandText += @" PRINT 'THIS IS THE END!'"; result[i] = new SqlCommand @@ -389,9 +396,11 @@ private static DisposableArray GetCommands(SqlConnection connection, break; case CommandType.StoredProcedure: + // sp_server_info returns a small, fixed result set and avoids the + // server-wide session scan that sp_who performs on shared Azure SQL. result[i] = new SqlCommand { - CommandText = "sp_who", + CommandText = "sp_server_info", CommandTimeout = 120, CommandType = CommandType.StoredProcedure, Connection = connection diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs index 555de74b29..116dd08abf 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/ConnectionPool/WaitHandleDbConnectionPoolTransactionTest.cs @@ -895,6 +895,56 @@ public void SequentialTransactions_CanReuseConnections() #endregion + #region Pruning Tests + + [Fact] + public void Pruning_IgnoresTransactedConnections() + { + // 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); + 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(poolCountBefore, _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 diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs index ea3e2d3da7..2514690087 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionFailoverTests.cs @@ -671,6 +671,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 + // ↓ (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. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs index 57e8d17632..b517ec8f56 100644 --- a/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/SimulatedServerTests/ConnectionTests.cs @@ -79,6 +79,7 @@ public async Task RequestEncryption_ServerDoesNotSupportEncryption_ShouldFail() [InlineData(40613)] [InlineData(42108)] [InlineData(42109)] + [Trait("Category", "flaky")] public async Task TransientFault_RetryEnabled_ShouldSucceed_Async(uint errorCode) { using TransientTdsErrorTdsServer server = new( @@ -161,6 +162,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(