Merge pull request #164 from DataAction/163-pooling-remove-connection

Don't double-call `RemoveConnection` in fill-to-min and try-replace pool behaviours

Author: senseibaka

src/AdoNetCore.AseClient/Internal/ConnectionPool.cs

@@ -212,7 +212,6 @@ private async Task TryFillPoolToMinSize()
                 catch(Exception ex)
                 {
                     Logger.Instance?.WriteLine($"{nameof(TryFillPoolToMinSize)} exception: {ex}");
-                    RemoveConnection();
                 }
             }
             Logger.Instance?.WriteLine($"{nameof(TryFillPoolToMinSize)} end");
@@ -231,7 +230,6 @@ private async Task TryReplaceConnection()
                 catch (Exception ex)
                 {
                     Logger.Instance?.WriteLine($"{nameof(TryReplaceConnection)} exception: {ex}");
-                    RemoveConnection();
                 }
             }
         }

test/AdoNetCore.AseClient.Tests/Unit/ConnectionPoolTests.cs

@@ -44,6 +44,68 @@ public void WhenMinPoolSizeIsSet_NewPoolFillsToMin()
             Task.Delay(1000).Wait();
 
             Assert.AreEqual(10, pool.PoolSize);
+            Assert.AreEqual(10, pool.Available);
+        }
+
+        [Test]
+        public void WhenMinPoolSizeIsSet_ButThereIsChanceOfFailure_PoolSizeMatchesAvailableCount()
+        {
+            var parameters = new TestConnectionParameters
+            {
+                MinPoolSize = 10
+            };
+            var pool = new ConnectionPool(parameters, new SequenceSuccessImmediateConnectionFactory(true, false));
+
+            Task.Delay(1000).Wait();
+
+            Assert.AreEqual(10, pool.PoolSize);
+            Assert.AreEqual(10, pool.Available);
+        }
+
+        [Test]
+        public void RemoveAndReplace_ReplacesToMin()
+        {
+            var parameters = new TestConnectionParameters
+            {
+                MinPoolSize = 10
+            };
+            var pool = new ConnectionPool(parameters, new ImmediateConnectionFactory());
+
+            Task.Delay(1000).Wait();
+
+            Assert.AreEqual(10, pool.PoolSize);
+            Assert.AreEqual(10, pool.Available);
+
+            var c = pool.Reserve(null);
+            c.IsDoomed = true;
+            pool.Release(c);
+            Task.Delay(1000).Wait();
+
+            Assert.AreEqual(10, pool.PoolSize);
+            Assert.AreEqual(10, pool.Available);
+        }
+
+        [Test]
+        public void RemoveAndReplace_ButThereIsReplacementFailure_DoesNotReplace()
+        {
+            var parameters = new TestConnectionParameters
+            {
+                MinPoolSize = 2
+            };
+            var pool = new ConnectionPool(parameters, new SequenceSuccessImmediateConnectionFactory(true, true, false));
+
+            Task.Delay(1000).Wait();
+
+            Assert.AreEqual(2, pool.PoolSize);
+            Assert.AreEqual(2, pool.Available);
+
+            var c = pool.Reserve(null);
+            c.IsDoomed = true;
+            pool.Release(c);
+            Task.Delay(1000).Wait();
+
+            Assert.AreEqual(1, pool.PoolSize);
+            Assert.AreEqual(1, pool.Available);
         }
 
         [Test]
@@ -57,11 +119,12 @@ public void WhenChangeDatabaseThrows_PoolDoesNotLeak()
 
             var pool = new ConnectionPool(parameters, new ImmediateConnectionFactory(changeDatabaseThrows: true));
 
-            for (int i=0; i<5; i++)
+            for (int i = 0; i < 5; i++)
             {
                 Assert.Throws<AseException>(() => pool.Reserve(null));
             }
             Assert.AreEqual(0, pool.PoolSize);
+            Assert.AreEqual(0, pool.Available);
         }
 
         [Test]
@@ -78,7 +141,10 @@ public void NewOpenCall_TimesOut_ShouldThrow()
             Assert.Throws<AseException>(() => pool.Reserve(null));
 
             Assert.AreEqual(1, pool.PoolSize);
+            Assert.AreEqual(0, pool.Available);
             pool.Release(c1);
+            Assert.AreEqual(1, pool.PoolSize);
+            Assert.AreEqual(1, pool.Available);
         }
 
         /// <summary>
@@ -127,7 +193,7 @@ public void PoolSpam_Waves(short size, int waves)
                     ExceptionDispatchInfo.Capture(ae.InnerException ?? ae).Throw();
                     throw;
                 }
-                
+
                 connections = reserveTask.Result;
             }
 
@@ -185,6 +251,35 @@ public async Task<IInternalConnection> GetNewConnection(CancellationToken token,
             }
         }
 
+        private class SequenceSuccessImmediateConnectionFactory : IInternalConnectionFactory
+        {
+            private int _idxNext;
+            private readonly bool[] _sequence;
+
+            public SequenceSuccessImmediateConnectionFactory(params bool[] sequence)
+            {
+                _idxNext = 0;
+                _sequence = sequence ?? new[] { true, false };
+            }
+
+            private bool IsSuccessful()
+            {
+                var next = _sequence[_idxNext];
+                _idxNext = (_idxNext + 1) % _sequence.Length;
+                return next;
+            }
+
+            public async Task<IInternalConnection> GetNewConnection(CancellationToken token, IInfoMessageEventNotifier eventNotifier)
+            {
+                if (IsSuccessful())
+                {
+                    return await Task.FromResult<IInternalConnection>(new DoNothingInternalConnection());
+                }
+
+                throw new Exception();
+            }
+        }
+
         [SuppressMessage("ReSharper", "UnassignedGetOnlyAutoProperty")]
         [SuppressMessage("ReSharper", "UnusedMember.Local")]
         private class DoNothingInternalConnection : IInternalConnection