From 4489ed35ca5beed569fde32986c7b082d40da0b5 Mon Sep 17 00:00:00 2001
From: Nicolas Martinelli <nim@odoo.com>
Date: Wed, 15 Feb 2023 14:10:00 +0000
Subject: [PATCH] [IMP] sql_db: close idle connections

Before this commit, the connection pool works greedily regarding the
number of connections opened. Once a new connection is needed, it is
added to the pool and never closed unless the maximum number of
connections is reached.

We introduce 2 changes. First, we do not cycle on the available
connections anymore. We take the first one available and let it at its
position in the list. It opens the possibility to use
`idle_session_timeout` introduced in PostgreSQL 14.

Moreover, we introduce a garbage collection of the unused connections.
If the connection has been unused for more than `MAX_IDLE_TIMEOUT`
seconds, it is closed and removed from the pool.

This makes easier to overcommit `--db-maxconn` when running in worker
mode. We can set a value which is large enough for the websocket
requirements without the WorkerHttp having a large number of useless
opened connections.

closes odoo/odoo#113110

X-original-commit: 9fcf3de6a9f10d4171bb48feb65fd789db19d4c6
Signed-off-by: Nicolas Martinelli (nim) <nim@odoo.com>
---
 odoo/sql_db.py | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/odoo/sql_db.py b/odoo/sql_db.py
index 4819837e526..2f3243d106d 100644
--- a/odoo/sql_db.py
+++ b/odoo/sql_db.py
@@ -48,6 +48,8 @@ re_into = re.compile('.* into "?([a-zA-Z_0-9]+)"? .*$')
 
 sql_counter = 0
 
+MAX_IDLE_TIMEOUT = 60 * 10
+
 
 class Savepoint:
     """ Reifies an active breakpoint, allows :meth:`BaseCursor.savepoint` users
@@ -573,22 +575,28 @@ class ConnectionPool(object):
     @locked
     def borrow(self, connection_info):
         """
+        Borrow a PsycoConnection from the pool. If no connection is available, create a new one
+        as long as there are still slots available. Perform some garbage-collection in the pool:
+        idle, dead and leaked connections are removed.
+
         :param dict connection_info: dict of psql connection keywords
         :rtype: PsycoConnection
         """
-        # free dead and leaked connections
-        for i, (cnx, _) in tools.reverse_enumerate(self._connections):
+        # free idle, dead and leaked connections
+        for i, (cnx, used, last_used) in tools.reverse_enumerate(self._connections):
+            if not used and not cnx.closed and time.time() - last_used > MAX_IDLE_TIMEOUT:
+                self._debug('Close connection at index %d: %r', i, cnx.dsn)
+                cnx.close()
             if cnx.closed:
                 self._connections.pop(i)
                 self._debug('Removing closed connection at index %d: %r', i, cnx.dsn)
                 continue
             if getattr(cnx, 'leaked', False):
                 delattr(cnx, 'leaked')
-                self._connections.pop(i)
-                self._connections.append((cnx, False))
+                self._connections[i][1] = False
                 _logger.info('%r: Free leaked connection to %r', self, cnx.dsn)
 
-        for i, (cnx, used) in enumerate(self._connections):
+        for i, (cnx, used, _) in enumerate(self._connections):
             if not used and cnx._original_dsn == connection_info:
                 try:
                     cnx.reset()
@@ -598,15 +606,14 @@ class ConnectionPool(object):
                     if not cnx.closed:
                         cnx.close()
                     continue
-                self._connections.pop(i)
-                self._connections.append((cnx, True))
+                self._connections[i][1] = True
                 self._debug('Borrow existing connection to %r at index %d', cnx.dsn, i)
 
                 return cnx
 
         if len(self._connections) >= self._maxconn:
             # try to remove the oldest connection not used
-            for i, (cnx, used) in enumerate(self._connections):
+            for i, (cnx, used, _) in enumerate(self._connections):
                 if not used:
                     self._connections.pop(i)
                     if not cnx.closed:
@@ -625,20 +632,22 @@ class ConnectionPool(object):
             _logger.info('Connection to the database failed')
             raise
         result._original_dsn = connection_info
-        self._connections.append((result, True))
+        self._connections.append([result, True, 0])
         self._debug('Create new connection backend PID %d', result.get_backend_pid())
         return result
 
     @locked
     def give_back(self, connection, keep_in_pool=True):
         self._debug('Give back connection to %r', connection.dsn)
-        for i, (cnx, used) in enumerate(self._connections):
+        for i, (cnx, _, _) in enumerate(self._connections):
             if cnx is connection:
-                self._connections.pop(i)
                 if keep_in_pool:
-                    self._connections.append((cnx, False))
+                    # Release the connection and record the last time used
+                    self._connections[i][1] = False
+                    self._connections[i][2] = time.time()
                     self._debug('Put connection to %r in pool', cnx.dsn)
                 else:
+                    self._connections.pop(i)
                     self._debug('Forgot connection to %r', cnx.dsn)
                     cnx.close()
                 break
@@ -649,7 +658,7 @@ class ConnectionPool(object):
     def close_all(self, dsn=None):
         count = 0
         last = None
-        for i, (cnx, used) in tools.reverse_enumerate(self._connections):
+        for i, (cnx, _, _) in tools.reverse_enumerate(self._connections):
             if dsn is None or cnx._original_dsn == dsn:
                 cnx.close()
                 last = self._connections.pop(i)[0]