Extra connections stay open when concurrent connections overpass the pool size

[naujoh]

I'm using this driver to make a connection in a tornado rest api through a connection pool, everything seemed normal until I made some test to prove the response of the api in concurrent connections.

I'm using apache ab tool to perform the request test to the api, the problem come when I configure apache ab tool to perform more concurrent request than the connection pool size, the problem is that all the connections that are created apart from the connections of the pool are kept open for some time until I got the message 'Close unknown Connection'.

Looking for the problem of this I'm checked the number of the connections to the database that are establish during the test and I found that the number of connections duplicates the connection pool size in almost all the cases and when the test ends all the connections of the pool are close less those that were created once the pool was full I don't know if this is the normal behavior of the driver but for me doesn't seems correct because I don have control of this extra connections.

So I read the code of the driver specifically the pool.py file and I understood a little how this works and I found that in the connection_connected_callback function are created extra connections to fulfill the wait_connections list when is true that _wait_connections list is not empty and __connections_count - 1 < max_connections it checks the timeout of the wait_connection and if not timeout then initiates a new connection to the database and I believe that here is the problem. I removed the -1 in this condition and everything seems to work fine as I expected I mean there are no more extra connections created (no more than the connection pool specified size) and every connection is closed once the idle_seconds property is reach.

Here I'm going to leave both fragments of code the original an the modification that I made in case that the changes that I made would be the expected behavior it could be added to the master branch or if anyone could tell me what is the correct behavior it would be great too 😄

[original code]

if self._wait_connections and self._connections_count - 1 < self._max_connections:
  wait_future, create_time = self._wait_connections.popleft()
  wait_time = time.time() - create_time
  if wait_time >= self._wait_connection_timeout:
    self._loop.call_soon(wait_future.set_exception, WaitConnectionTimeoutError(
        "Wait connection timeout, used time %.2fs." % wait_time))
  else:
    self._loop.call_soon(self.init_connection, wait_future)

[modified]

if self._wait_connections and self._connections_count < self._max_connections:
  wait_future, create_time = self._wait_connections.popleft()
  wait_time = time.time() - create_time
  if wait_time >= self._wait_connection_timeout:
    self._loop.call_soon(wait_future.set_exception, WaitConnectionTimeoutError(
        "Wait connection timeout, used time %.2fs." % wait_time))
  else:
    self._loop.call_soon(self.init_connection, wait_future)

Also I made some modifications to the continue_next_wait function in the same file (pool.py) I think that is more clever this way but if I missed something would be awesome if anyone could tell me too 😄 just to repair mi code haha.

[original function]

def continue_next_wait(self, connection):
    now = time.time()
    while self._wait_connections:
        wait_future, create_time = self._wait_connections.popleft()
        wait_time = now - create_time
        if wait_time >= self._wait_connection_timeout:
            self._wait_connection_timeout_futures.append((wait_future, wait_time))
            continue
        connection.used_time = now
        self._loop.call_soon(wait_future.set_result, connection)
        if self._wait_connection_timeout_futures:
            self._loop.call_soon(self.do_wait_future_exception_timeout)
        return True

    if self._wait_connection_timeout_futures:
        self._loop.call_soon(self.do_wait_future_exception_timeout)
    return False

[my modification]

def continue_next_wait(self, connection):
    now = time.time()
    if self._wait_connections:
        wait_future, create_time = self._wait_connections.popleft()
        wait_time = now - create_time
        if wait_time >= self._wait_connection_timeout:
            self._wait_connection_timeout_futures.append((wait_future, wait_time))
            self._loop.call_soon(self.do_wait_future_exception_timeout)
            return False
        else:
            connection.used_time = now
            self._loop.call_soon(wait_future.set_result, connection)
            return True

[JustinhoCHN]

any update or any official suggestion?