Fix for CONPY-245:

Instead of iterating through all connections and checking the health
status via ping, used and unused connections were separated in different
lists. This ensures that the last used connection will be always the first.

Author: 9EOR9

mariadb/connectionpool.py

@@ -19,7 +19,6 @@
 
 import mariadb
 import _thread
-import time
 
 MAX_POOL_SIZE = 64
 POOL_IDLE_TIMEOUT = 1800
@@ -63,7 +62,8 @@ def __init__(self, *args, **kwargs):
             Will reset the connection before returning it to the pool.
             Default value is True.
         """
-        self._connections = []
+        self._connections_free = []
+        self._connections_used = []
         self._pool_args = {}
         self._conn_args = {}
         self._lock_pool = _thread.RLock()
@@ -107,15 +107,15 @@ def __init__(self, *args, **kwargs):
                     except mariadb.Error:
                         # if an error occurred, close all connections
                         # and raise exception
-                        for j in range(0, len(self._connections)):
+                        for j in range(0, len(self._connections_free)):
                             try:
-                                self._connections[j].close()
+                                self._connections_free[j].close()
                             except mariadb.Error:
                                 # connect failed, so we are not
                                 # interested in errors
                                 # from close() method
                                 pass
-                            del self._connections[j]
+                            del self._connections_free[j]
                         raise
                     self.add_connection(connection)
 
@@ -151,46 +151,41 @@ def add_connection(self, connection=None):
             raise mariadb.PoolError("Can't get configuration for pool %s" %
                                     self._pool_args["name"])
 
-        if len(self._connections) >= self._pool_args["size"]:
+        total = len(self._connections_free) + len(self._connections_used)
+        if total >= self._pool_args["size"]:
             raise mariadb.PoolError("Can't add connection to pool %s: "
                                     "No free slot available (%s)." %
                                     (self._pool_args["name"],
-                                     len(self._connections)))
+                                     total))
 
         with self._lock_pool:
             if connection is None:
                 connection = mariadb.Connection(**self._conn_args)
 
             connection._Connection__pool = self
-            connection._Connection__in_use = 0
-            connection._Connection__last_used = time.monotonic()
-            self._connections.append(connection)
+            self._connections_free.append(connection)
 
     def get_connection(self):
         """
         Returns a connection from the connection pool or raises a PoolError
         exception if a connection is not available.
         """
 
-        now = time.monotonic()
         conn = None
-        timediff = -1
 
         with self._lock_pool:
-            for i in range(0, len(self._connections)):
-                if not self._connections[i]._Connection__in_use:
-                    try:
-                        self._connections[i].ping()
-                    except mariadb.Error:
-                        continue
-                    t = now - self._connections[i]._Connection__last_used
-                    if t > timediff:
-                        conn = self._connections[i]
-                        timediff = t
-
-            if conn:
-                conn._Connection__in_use = 1
-        return conn
+            for i in range(0, len(self._connections_free)):
+                try:
+                    self._connections_free[i].ping()
+                except mariadb.Error:
+                    continue
+                conn = self._connections_free[i]
+                conn._used += 1
+                self._connections_used.append(conn)
+                del self._connections_free[i]
+                return conn
+
+        return None
 
     def _close_connection(self, connection):
         """
@@ -201,8 +196,12 @@ def _close_connection(self, connection):
         with self._lock_pool:
             if self._pool_args["reset_connection"]:
                 connection.reset()
-            connection._Connection__in_use = 0
-            connection._Connection__last_used = time.monotonic()
+
+            for i in range(0, len(self._connections_used)):
+                if self._connections_used[i] == connection:
+                    del self._connections_used[i]
+                    self._connections_free.append(connection)
+                    return
 
     def set_config(self, **kwargs):
         """
@@ -218,11 +217,15 @@ def set_config(self, **kwargs):
     def close(self):
         """Closes connection pool and all connections."""
         try:
-            for c in self._connections:
+            for c in self._connections_free:
+                c._Connection__pool = None
+                c.close()
+            for c in self._connections_used:
                 c._Connection__pool = None
                 c.close()
         finally:
-            self._connections = None
+            self._connections_free = None
+            self._connections_used = None
             del mariadb._CONNECTION_POOLS[self._pool_args["name"]]
 
     @property

mariadb/connections.py

@@ -51,10 +51,9 @@ def __init__(self, *args, **kwargs):
         """
 
         self._socket = None
-        self.__in_use = 0
+        self._used = 0
         self._last_executed_statement = None
         self._socket = None
-        self.__in_use = 0
         self.__pool = None
         self.__last_used = 0
         self.tpc_state = TPC_STATE.NONE

testing/test/integration/test_pooling.py

@@ -36,6 +36,29 @@ def test_conpy39(self):
         except mariadb.ProgrammingError:
             pass
 
+    def test_conpy245(self):
+        # we can't test performance here, but we can check if LRU works.
+        # All connections must have been used the same number of times.
+
+        default_conf = conf()
+        pool_size = 64
+        iterations = 100
+
+        pool = mariadb.ConnectionPool(pool_name="CONPY245",
+                                      pool_size=pool_size,
+                                      **default_conf)
+        for i in range(0, iterations):
+            for j in range(0, pool_size):
+                conn = pool.get_connection()
+                conn.close()
+
+        for i in range(0, pool_size):
+            conn = pool.get_connection()
+            self.assertEqual(conn._used, iterations + 1)
+            conn.close()
+
+        pool.close()
+
     def test_connection_pool_conf(self):
         pool = mariadb.ConnectionPool(pool_name="test_conf")
         default_conf = conf()