Reap connections more frequently. Fixes #442

If ConnectionIdleTimeout is set to a low value for a particular connection pool, clean up connections in that pool more frequently. (Since most applications have just one connection string, switching from a global reaper Task to a per-pool Task won't create extra background tasks for most applications.)

Since the reaper interval is now set per connection pool, the DebugOnlyTests are no longer necessary (and can be executed with the other ConnectionPool tests).

Author: bgrainger

.ci/test.ps1

@@ -32,11 +32,6 @@ dotnet xunit -c Release
 if ($LASTEXITCODE -ne 0){
     exit $LASTEXITCODE;
 }
-echo "Executing Debug Only tests"
-dotnet xunit -c Debug -class SideBySide.DebugOnlyTests
-if ($LASTEXITCODE -ne 0){
-    exit $LASTEXITCODE;
-}
 
 echo "Executing tests with Compression, No SSL"
 Copy-Item -Force ..\..\.ci\config\config.compression.json config.json

.travis.yml

@@ -35,7 +35,6 @@ script:
 - dotnet build SideBySide.csproj -c Release -f netcoreapp2.0
 - echo 'Executing netcoreapp1.1.2 tests with No Compression, No SSL' && ../../.ci/use-config.sh config.json 172.17.0.1 3307 $NAME $OMIT_FEATURES && time dotnet xunit -c Release -f netcoreapp1.1.2
 - echo 'Executing netcoreapp2.0 tests with No Compression, No SSL' && ../../.ci/use-config.sh config.json 172.17.0.1 3307 $NAME $OMIT_FEATURES && time dotnet xunit -c Release -f netcoreapp2.0
-- echo 'Executing netcoreapp2.0 Debug Only tests' && time dotnet xunit -c Debug -f netcoreapp2.0 -class SideBySide.DebugOnlyTests
 - echo 'Executing netcoreapp2.0 tests with Compression, No SSL' && ../../.ci/use-config.sh config.compression.json 172.17.0.1 3307 $NAME $OMIT_FEATURES && time dotnet xunit -c Release -f netcoreapp2.0
 - echo 'Executing netcoreapp1.1.2 tests with No Compression, SSL' && ../../.ci/use-config.sh config.ssl.json 172.17.0.1 3307 $NAME $OMIT_FEATURES && time dotnet xunit -c Release -f netcoreapp1.1.2
 - echo 'Executing netcoreapp2.0 tests with No Compression, SSL' && ../../.ci/use-config.sh config.ssl.json 172.17.0.1 3307 $NAME $OMIT_FEATURES && time dotnet xunit -c Release -f netcoreapp2.0

src/MySqlConnector/Core/ConnectionPool.cs

@@ -178,8 +178,6 @@ public async Task ReapAsync(IOBehavior ioBehavior, CancellationToken cancellatio
 		{
 			Log.Debug("{0} reaping connection pool", m_logArguments);
 			RecoverLeakedSessions();
-			if (ConnectionSettings.ConnectionIdleTimeout == 0)
-				return;
 			await CleanPoolAsync(ioBehavior, session => (DateTime.UtcNow - session.LastReturnedUtc).TotalSeconds >= ConnectionSettings.ConnectionIdleTimeout, true, cancellationToken).ConfigureAwait(false);
 		}
 
@@ -398,12 +396,6 @@ public static async Task ClearPoolsAsync(IOBehavior ioBehavior, CancellationToke
 				await pool.ClearAsync(ioBehavior, cancellationToken).ConfigureAwait(false);
 		}
 
-		public static async Task ReapPoolsAsync(IOBehavior ioBehavior, CancellationToken cancellationToken)
-		{
-			foreach (var pool in GetAllPools())
-				await pool.ReapAsync(ioBehavior, cancellationToken).ConfigureAwait(false);
-		}
-
 		private static IReadOnlyList<ConnectionPool> GetAllPools()
 		{
 			var pools = new List<ConnectionPool>(s_pools.Count);
@@ -430,6 +422,7 @@ private ConnectionPool(ConnectionSettings cs)
 				foreach (var hostName in cs.HostNames)
 					m_hostSessions[hostName] = 0;
 			}
+
 			m_loadBalancer = cs.ConnectionType != ConnectionType.Tcp ? null :
 				cs.HostNames.Count == 1 || cs.LoadBalance == MySqlLoadBalance.FailOver ? FailOverLoadBalancer.Instance :
 				cs.LoadBalance == MySqlLoadBalance.Random ? RandomLoadBalancer.Instance :
@@ -440,6 +433,28 @@ private ConnectionPool(ConnectionSettings cs)
 			m_logArguments = new object[] { "Pool{0}".FormatInvariant(Id) };
 			if (Log.IsInfoEnabled())
 				Log.Info("{0} creating new connection pool for {1}", m_logArguments[0], cs.ConnectionStringBuilder.GetConnectionString(includePassword: false));
+
+			if (cs.ConnectionIdleTimeout > 0)
+			{
+				var reaperInterval = TimeSpan.FromSeconds(Math.Max(1, Math.Min(60, cs.ConnectionIdleTimeout / 2)));
+				m_reaperTask = Task.Run(async () =>
+				{
+					while (true)
+					{
+						var task = Task.Delay(reaperInterval);
+						try
+						{
+							using (var source = new CancellationTokenSource(reaperInterval))
+								await ReapAsync(IOBehavior.Asynchronous, source.Token).ConfigureAwait(false);
+						}
+						catch
+						{
+							// do nothing; we'll try to reap again
+						}
+						await task.ConfigureAwait(false);
+					}
+				});
+			}
 		}
 
 		private void AdjustHostConnectionCount(ServerSession session, int delta)
@@ -478,26 +493,6 @@ public ConnectionStringPool(string connectionString, ConnectionPool pool)
 
 		static readonly IMySqlConnectorLogger Log = MySqlConnectorLogManager.CreateLogger(nameof(ConnectionPool));
 		static readonly ConcurrentDictionary<string, ConnectionPool> s_pools = new ConcurrentDictionary<string, ConnectionPool>();
-#if DEBUG
-		static readonly TimeSpan ReaperInterval = TimeSpan.FromSeconds(1);
-#else
-		static readonly TimeSpan ReaperInterval = TimeSpan.FromMinutes(1);
-#endif
-		static readonly Task Reaper = Task.Run(async () => {
-			while (true)
-			{
-				var task = Task.Delay(ReaperInterval);
-				try
-				{
-					await ReapPoolsAsync(IOBehavior.Asynchronous, new CancellationTokenSource(ReaperInterval).Token).ConfigureAwait(false);
-				}
-				catch
-				{
-					// do nothing; we'll try to reap again
-				}
-				await task.ConfigureAwait(false);
-			}
-		});
 
 		static int s_poolId;
 		static ConnectionStringPool s_mruCache;
@@ -510,6 +505,7 @@ public ConnectionStringPool(string connectionString, ConnectionPool pool)
 		readonly ILoadBalancer m_loadBalancer;
 		readonly Dictionary<string, int> m_hostSessions;
 		readonly object[] m_logArguments;
+		readonly Task m_reaperTask;
 		uint m_lastRecoveryTime;
 		int m_lastSessionId;
 		Dictionary<string, CachedProcedure> m_procedureCache;

tests/SideBySide/ConnectionPool.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
@@ -146,6 +146,7 @@ public void LeakConnections()
 
 				// have to GC for leaked connections to be removed from the pool
 				GC.Collect();
+				Thread.Sleep(400);
 			}
 		}
 
@@ -282,6 +283,41 @@ public async Task ClearConnectionPool()
 				connection.Dispose();
 		}
 
+#if !BASELINE
+		[Theory]
+		[InlineData(1u, 3u, 0u, 5u)]
+		[InlineData(1u, 3u, 3u, 5u)]
+		public async Task ConnectionLifeTime(uint idleTimeout, uint delaySeconds, uint minPoolSize, uint maxPoolSize)
+		{
+			var csb = AppConfig.CreateConnectionStringBuilder();
+			csb.Pooling = true;
+			csb.MinimumPoolSize = minPoolSize;
+			csb.MaximumPoolSize = maxPoolSize;
+			csb.ConnectionIdleTimeout = idleTimeout;
+			HashSet<int> serverThreadIdsBegin = new HashSet<int>();
+			HashSet<int> serverThreadIdsEnd = new HashSet<int>();
+
+			async Task OpenConnections(uint numConnections, HashSet<int> serverIdSet)
+			{
+				using (var connection = new MySqlConnection(csb.ConnectionString))
+				{
+					await connection.OpenAsync();
+					serverIdSet.Add(connection.ServerThread);
+					if (--numConnections <= 0)
+						return;
+					await OpenConnections(numConnections, serverIdSet);
+				}
+			}
+
+			await OpenConnections(maxPoolSize, serverThreadIdsBegin);
+			await Task.Delay(TimeSpan.FromSeconds(delaySeconds));
+			await OpenConnections(maxPoolSize, serverThreadIdsEnd);
+
+			serverThreadIdsEnd.IntersectWith(serverThreadIdsBegin);
+			Assert.Equal((int) minPoolSize, serverThreadIdsEnd.Count);
+		}
+#endif
+
 		private Task ClearPoolAsync(MySqlConnection connection)
 		{
 #if BASELINE

tests/SideBySide/DebugOnlyTests.cs

@@ -44,10 +44,6 @@
     <PackageReference Include="MySql.Data" Version="6.9.9" />
   </ItemGroup>
 
-  <ItemGroup Condition=" '$(Configuration)' != 'Debug' ">
-    <Compile Remove="DebugOnlyTests.cs" />
-  </ItemGroup>
-
   <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp1.1.2' ">
     <Compile Remove="TransactionScopeTests.cs" />
   </ItemGroup>