From: Daniele Varrazzo <daniele.varrazzo@gmail.com>
Date: Tue, 6 Sep 2022 23:49:14 +0000 (+0100)
Subject: fix(pool): continue growing if the pool is not full yet
X-Git-Tag: pool-3.1.2~1^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f627821400678d1e3cb82bffe73b95a81a64fc2f;p=thirdparty%2Fpsycopg.git

fix(pool): continue growing if the pool is not full yet

Previously, starting from an empty pool, we would have stopped at the
first connection created.
---

diff --git a/psycopg_pool/psycopg_pool/pool.py b/psycopg_pool/psycopg_pool/pool.py
index 0f3cf535a..c3c9579ee 100644
--- a/psycopg_pool/psycopg_pool/pool.py
+++ b/psycopg_pool/psycopg_pool/pool.py
@@ -83,7 +83,7 @@ class ConnectionPool(BasePool[Connection[Any]]):
 
         with self._lock:
             assert not self._pool_full_event
-            if len(self._pool) >= self._nconns:
+            if len(self._pool) >= self._min_size:
                 return
             self._pool_full_event = threading.Event()
 
@@ -543,7 +543,11 @@ class ConnectionPool(BasePool[Connection[Any]]):
         self._add_to_pool(conn)
         if growing:
             with self._lock:
-                if self._nconns < self._max_size and self._waiting:
+                # Keep on growing if the pool is not full yet, or if there are
+                # clients waiting and the pool can extend.
+                if self._nconns < self._min_size or (
+                    self._nconns < self._max_size and self._waiting
+                ):
                     self._nconns += 1
                     logger.info("growing pool %r to %s", self.name, self._nconns)
                     self.run_task(AddConnection(self, growing=True))
@@ -600,7 +604,7 @@ class ConnectionPool(BasePool[Connection[Any]]):
 
                 # If we have been asked to wait for pool init, notify the
                 # waiter if the pool is full.
-                if self._pool_full_event and len(self._pool) >= self._nconns:
+                if self._pool_full_event and len(self._pool) >= self._min_size:
                     self._pool_full_event.set()
 
     def _reset_connection(self, conn: Connection[Any]) -> None:
diff --git a/psycopg_pool/psycopg_pool/pool_async.py b/psycopg_pool/psycopg_pool/pool_async.py
index e54d7b19a..11d7cef42 100644
--- a/psycopg_pool/psycopg_pool/pool_async.py
+++ b/psycopg_pool/psycopg_pool/pool_async.py
@@ -62,7 +62,7 @@ class AsyncConnectionPool(BasePool[AsyncConnection[Any]]):
 
         async with self._lock:
             assert not self._pool_full_event
-            if len(self._pool) >= self._nconns:
+            if len(self._pool) >= self._min_size:
                 return
             self._pool_full_event = asyncio.Event()
 
@@ -444,7 +444,11 @@ class AsyncConnectionPool(BasePool[AsyncConnection[Any]]):
         await self._add_to_pool(conn)
         if growing:
             async with self._lock:
-                if self._nconns < self._max_size and self._waiting:
+                # Keep on growing if the pool is not full yet, or if there are
+                # clients waiting and the pool can extend.
+                if self._nconns < self._min_size or (
+                    self._nconns < self._max_size and self._waiting
+                ):
                     self._nconns += 1
                     logger.info("growing pool %r to %s", self.name, self._nconns)
                     self.run_task(AddConnection(self, growing=True))
@@ -501,7 +505,7 @@ class AsyncConnectionPool(BasePool[AsyncConnection[Any]]):
 
                 # If we have been asked to wait for pool init, notify the
                 # waiter if the pool is full.
-                if self._pool_full_event and len(self._pool) >= self._nconns:
+                if self._pool_full_event and len(self._pool) >= self._min_size:
                     self._pool_full_event.set()
 
     async def _reset_connection(self, conn: AsyncConnection[Any]) -> None:
diff --git a/tests/pool/test_pool.py b/tests/pool/test_pool.py
index 51d800a82..b83ca08e9 100644
--- a/tests/pool/test_pool.py
+++ b/tests/pool/test_pool.py
@@ -922,7 +922,9 @@ def test_reconnect_failure(proxy):
 
 
 @pytest.mark.slow
-def test_reconnect_retry(proxy):
+def test_reconnect_after_grow_failed(proxy):
+    # Retry reconnection after a failed connection attempt has put the pool
+    # in grow mode. See issue #370.
     proxy.stop()
 
     ev = Event()
@@ -931,7 +933,7 @@ def test_reconnect_retry(proxy):
         ev.set()
 
     with pool.ConnectionPool(
-        proxy.client_dsn, reconnect_timeout=1.0, reconnect_failed=failed
+        proxy.client_dsn, min_size=4, reconnect_timeout=1.0, reconnect_failed=failed
     ) as p:
         assert ev.wait(timeout=2)
 
@@ -947,6 +949,9 @@ def test_reconnect_retry(proxy):
         with p.connection(timeout=2) as conn:
             conn.execute("select 1")
 
+        p.wait(timeout=3.0)
+        assert len(p._pool) == p.min_size == 4
+
 
 @pytest.mark.slow
 def test_uniform_use(dsn):
diff --git a/tests/pool/test_pool_async.py b/tests/pool/test_pool_async.py
index 6c923e638..7d6ca6d79 100644
--- a/tests/pool/test_pool_async.py
+++ b/tests/pool/test_pool_async.py
@@ -874,7 +874,9 @@ async def test_reconnect_failure(proxy):
 
 
 @pytest.mark.slow
-async def test_reconnect_retry(proxy):
+async def test_reconnect_after_grow_failed(proxy):
+    # Retry reconnection after a failed connection attempt has put the pool
+    # in grow mode. See issue #370.
     proxy.stop()
 
     ev = asyncio.Event()
@@ -883,7 +885,7 @@ async def test_reconnect_retry(proxy):
         ev.set()
 
     async with pool.AsyncConnectionPool(
-        proxy.client_dsn, reconnect_timeout=1.0, reconnect_failed=failed
+        proxy.client_dsn, min_size=4, reconnect_timeout=1.0, reconnect_failed=failed
     ) as p:
         await asyncio.wait_for(ev.wait(), 2.0)
 
@@ -899,6 +901,9 @@ async def test_reconnect_retry(proxy):
         async with p.connection(timeout=2) as conn:
             await conn.execute("select 1")
 
+        await p.wait(timeout=3.0)
+        assert len(p._pool) == p.min_size == 4
+
 
 @pytest.mark.slow
 async def test_uniform_use(dsn):