connection pool in multi-thread is not thread safe

[asynchronoust]

when multi thread share one instance of connectionpool, the self._in_use_connections and self._available_connections and the self._created_connections are not thread safe.

i do not see lock with these variables, so the connection pool may create more connections than the max_connections variable in the ConnectionPool class

the only using of lock is the reset() method, but this method is for checking the change of pid, but not the id of thread.

thanks !!

[PayneJoe]

same question-)

[rolette]

Are you taking into account the GIL when you are looking at the code?

[wynemo]

@rolette GIL cannot guarantee of list's threading safety.
#edit it seems list is threading safety.

if self._created_connections >= self.max_connections:
but this line is not threading safe.

    self.redis = redis.Redis(
        connection_pool=redis.BlockingConnectionPool(host=host, port=port, password=password,
                                                     max_connections=20, socket_keepalive=False,
                                                     retry_on_timeout=False, socket_connect_timeout=15,
                                                     db=db, socket_timeout=default_timeout, **kwargs))

i have to to use redis.BlockingConnectionPool,, it's threading safe since it uese queue.Queue.
we used the default connectionpool, but it creates too many connections since we use flask, beacuase it uses multipe threading by default.

[gerazenobi]

Hi there. I landed here because I was experimenting some strange behaviour as well and thought of asking in this thread to understand a bit more about the client and its connection pool options.

One difference though is that my environment is based on greenlets: our code base uses a certain lib that uses grequests hence there is the gevent monkey pathing this lib does when being imported ( curious_george.patch_all(thread=False, select=False) )

So we have a piece of code doing a certain number of concurrent HTTP requests using grequests which at the same time we have patched ( the session we pass to grequests) with requests-cache using a redis backend:

# simplified version ...
self.session = CachedSession(
            backend="redis", cache_name=cache_name, allowable_codes=(200, 201), allowable_methods=("GET", "POST")
        )
# ...
reqs = (grequests.post(url, session=self.session, auth=self.AUTH, json=data, timeout=self.TIMEOUT)
                for url in urls)
resps = grequests.map(reqs)

Everything seem to work fine (in this adventurous mix of having grequests with requests-cache and redis) until we decided to limit the number of redis connections being created (previously this would equal to the number of HTTP requests being launched as there was not max_connections being set)

After we provided the redis client used by the session above with a connection pool of 1 connection like so:

pool = redis.ConnectionPool(host="localhost", port=6379, db=0, max_connections=1)

we started getting inexplicable (random) results (inconsistent data, nulls, etc), I would assume due to thread unsafety conditions... It was strange that I didn't get any exceptions as the doc points out:

The difference is that, in the event that a client tries to get a
connection from the pool when all of connections are in use, rather than
raising a :py:class: ~redis.exceptions.ConnectionError (as the default
:py:class: ~redis.connection.ConnectionPool implementation does), it
makes the client wait ("blocks") for a specified number of seconds until
a connection becomes available.

When trying the blocking pool:

pool = redis.BlockingConnectionPool(host="localhost", port=6379, db=0, max_connections=1)

we went back to receiving correct results (as in the first use case when we didn't provide any pool or connection limits) whilst having 1 connection shared (correctly?) among all the coroutines.

My question: how would then ConnectionPool and BlockingConnectionPool be different in this case? It seem that when using the non blocking pool the connection was shared (no exceptions being thrown) even when already in use by certain greenlets? (that could potentially explain the wrong results)

Any information to understand a bit more would be much appreciated :)