"No connection available" errors since 5.0.1 #2995
Comments
|
Same here, I have cases where "No connection available." is raised but the pool has 4 _available_connections, and only 5 _in_use_connections out of 500 max_connections. @kristjanvalur I saw you refactored the ConnectionPool / BlockingConnectionPool in 5.0.1, do you have any idea of what would trigger this ? |
|
async Redis client and connection pools now need to be properly closed. cachews needs to ensure that the Redis instance owns the pool. Or, keep a separate pool somewhere and close it. async def init(self):
self._client = self._client_class(connection_pool=self._pool_class.from_url(self._address, **self._kwargs))
await self._client.initialize()
self.__is_init = Trueshould become async def init(self):
pool = self._pool_class.from_url(self._address, **self._kwargs)
self._client = self._client_class.from_pool(pool)
await self._client.initialize()
self.__is_init = TrueThis ensures that the created instance takes_ownership of the pool and closes it when itself is closed. The current syntax Please consider errors like this to be the result of incorrect usage in the library actually using redis-py. |
|
Hi, Thanks for the quick reply ! In my case, I'm not dependent on cashews, but I'm wrapping redis.asyncio.Redis in my own class, like so: The thing I don't understand, is that a ConnectionError is till raised even though:
|
|
can you please attach a traceback? |
|
Sure: |
|
That is weird. it is timing out trying to acquire the condition variable. That should never happen |
|
Hello @kristjanvalur Excuse me for bothering you, but why we have to close connections if redis instance never goes out of scope? For example if we have a long-running application isn't it better to simply create a pool and then use it for all requests? Does it mean that all open IDLE connection will be there forever and we should clean it all up ourselves if we don't need it at the moment? Is there any docs on intended use of a async version of redis? Personally, now I have a troubles with "ERR max number of clients reached" by redis itself, it's a completely different problem but couldn't help myself commenting, as documentation kinda glosses over async version of a client and you have to look everywhere |
|
Hi @paul-finary , Could you test with the new redis/asyncio/connection.py which is part of the above-mentioned PR? |
Hi there. You don't have to. But if you are having this problem now, when you weren't having it before, it meant that you were losing connection objects and not explicitly closing them. The old version of redis-py would try to clean them up inthe del handlers, but this is really, really bad practice. For one thing, it causes all async jobs to block. If you are seing this problem now, it means probably that there is some incorrect use of a connection pool, or a connection is not being correctly returned to the pool. What is your use case? For using a pool object and re-using it, this is the pattern you should be using: pool = redis.asyncio.ConnectionPool(...)
# make sure we close the pool when we are don
async with contextlib.aclosing(pool):
for whatever in jobs():
# make sure we return the connection to the pool after use
async with redis.asyncio.Redis(connection_pool=pool) as redis:
redis.do stuff() |
|
If you forget to return/close connections, then Python will print out a "resource warning" when the connection object gets garbage collected. |
Thanks, I'll follow up with cashews |
|
@kristjanvalur Thanks for being so reactive ! I'm currently trying to reproduce the issue as I did not yet find a consistent trigger for the "No connection available" error. I'll keep you posted ! |
My hypothesis is this: You create a pool, and immediately start a few hundred requests. However, the current implementation will make the socket connection within the condition variable lock. and so, all the tasks are waiting, on the timeout, while one after the other finishes their connections. A few hundred socket connects can easily take up to 5 seconds, and then finally one of them times out. Maybe you can repro using that, maybe your server was far away or something? My PR should fix this, now the condition variable is only around pool management, not the socket connection code. |
|
@kristjanvalur Thank you for your responsiveness! Feeling a little embarrassed and I'll understand if you keep my question unanswered as you're not obliged to and it's clearaly not a SO or some education course, and even this issue has nothing to do with my problem.
we're using redis-py 5.0.0 and problem came with the update of a perfomance requirements. It's a web service which has to hit redis to process requests. Recently we got our requirements updated - now we have to handle 2k RPS on average. I have a class which encapsulates redis handling: class RedisCodeClient:
def __init__(self, pool: ConnectionPool):
self._conn_pool = pool
self._active_db_number = 0
async def select_active_db(self):
async with Redis(connection_pool=self._conn_pool, db=0) as redis:
active_db_number = await redis.get('active_db')
if not active_db_number:
return
self._active_db_number = active_db_number
async def get_code_data(self, code: str) -> list[dict[str, str]]:
async with Redis(connection_pool=self._conn_pool, db=self._active_db_number) as redis:
async with redis.pipeline(transaction=False) as pipe:
for number in [code[:i] for i in range(len(code), 0, -1)]:
pipe.hgetall(number)
return await pipe.execute()This is a class, instance of which are created on the first request when we ask for it in our handlers: @functools.lru_cache()
def get_code_client():
settings = get_settings()
pool = ConnectionPool(
host=settings.CODE_REDIS_HOST,
port=settings.CODE_REDIS_PORT,
db=settings.CODE_REDIS_DB,
decode_responses=True,
)
return RedisCodeClient(pool=pool)
...
async def get_code_data(
client: RedisCodeClient, code: str
) -> list[dict[str, str]]:
await client.select_active_db()
return await client.get_code_data(code)
...
async def example(code: str):
redis = get_code_client()
code_data = await get_code_data(redis, code)When we're testing our service at high RPS (~1.5k) - we hit the redis EDIT: If somebody stumbles upon my code and for any reason decide to use it for their own projects be careful there is nasty bug with selecting logical database in this code. |
Hi there. Superficially, your code looks okay. The only leakage here is that I see is the global ConnectionPool being created but The fact that changing to BlockingConnectionPool with max_params causes you to block, indicates that somehow, connections are not being returned to the pool... I'll investigate this a bit with the code you supplied. |
|
@kristjanvalur Based on my tests, it looks like your PR indeed fixed the issue with the ConnectionError raised ! Thanks again for taking care of this, this quickly. |
|
@iamdbychkov I am unable to reproduce your issue with the code provided. Despite that, I think I can improve on the safety of the library. I'll prepare a PR which should improve things and produce relevant warnings when connections are lost. |
|
@kristjanvalur My bad! I'm sorry, should've mentioned. Yes, every request is separate asyncio task. My guess is that a reason for everything not working is just a fact that incoming rate of requests is much greater than our redis instance is able to handle - new connections keep opening, as others are busy and we hit the redis I've tweaked redis to handle more than 5k connections and we've scheduled our tests on monday. Thank you for your responsiveness, you're being a real role model! |
Can you introduce a MAJOR release for the backward incompatible changes?
|
|
Hello @Krukov . As I already explained before, I am not a maintainer of this repository, just a regular contributor.
Cheers! |
Hi. I was wondering, in addition to Tasks, maybe your web stack is also using threads? I'm just guessing here, it could be an avenue to explore, since your global ConnectionPool would then need to be thread safe. |
Yes, we do spawn threads for some jobs, but not for request handling. Threads we spawn do not interfere with asyncio/redis part. At least to my knowledge. We're using FastAPI as our web-framework. I'll look it up, thanks for the tip! |
|
@kristjanvalur Sorry, my bad, I thought that
client = Redis(...)
client.auto_close_connection_pool = True |
No, it will not be switched to false. redis = Redis(auto_close_connection_pool=False)
pool = redis.connection_pool.
await redis.aclose()
#pool is still open and I can do what I want with it. but I need to close it if I don't want my connections to leak.This was the only reason for this argument, to deal with cases where someone wanted to use the ConnectionPool that was created automatically as part of the In this case here: pool = ConnectionPool()
redis = Redis(connection_pool=pool, auto_close_connection_pool=True)the Confusing? I think so. This is why we are deprecating it.
The correct way, if you create your Pool yourself, is to do pool = ConnectionPool()
redis = Redis(connection_pool = pool)
...
await redis.aclose()
await pool.close()or use the new function, from_pool, which is like modifying the pool = ConnectionPool()
redis = Redis.from_pool(pool)
...
await redis.aclose()
# pool is now also closedThe reason you are seeing the "No connection available" now, is that garbage collection has changed. The latest version, the library will not automatically close connections that you forget about. This is because it is hard to do so reliably, there are lots of problems with doing that. And it is the responsibility of the user to make sure they keep track of and close their connections. That said, I have a PR in this repo which restores some of the safety of the garbage collection, but will also output a
Well, this is actually what the |
|
I have a question, is it really necessary to set 'auto_close_connection_pool' to true in sentinel working mode? In redis-cluster and single redis connection, it is a single connection. Every time a request comes in, a new connection is created and the request is closed. It is normal, but in the sentinel working mode, redis does not directly create the connection pool. Instead, it creates the redis connection itself after getting all the connections from sentinel. When 'auto_close_connection_pool' is true, 'master_for' and 'slave_for' ' All the connections created will be closed, causing each request to reacquire the master and slave of the sentinel connection, and then create a redis connection. Normally, it should not be the case. Only a new redis connection is initialized during the request process. When this Refresh the sentinel connection only when the connection fails and reacquire the master-slave to update to the new master-slave redis connection. Also, the 'connection' created in redis-sentinel has no effect in the entire fastapi request. It is executed every time. A new connection is taken from the sentinel connection pool to execute the command, but the same one is not reused. This may be a problem with my writing method. redis: Sentinel connections will be reused before 5.0. After 5.1, Don't quite understand how it works |
|
Every time you call Your problem is a different one. You don't want the "async with". You are keeping the Instead, just do the following: async def redis_master(db: int = REDIS_DB_0) -> Redis:
global redis_db_master_dict
return redis_db_master_dict[db]the async with sentinel.slave_for(...) as client:
await client.do_something()
await client.do_something_different()
# now , 'client' is closed, and all its connections too.Cheers! |
I‘ll give it a try. thank you |
I found that in the connection of Wouldn’t such transformations Or should I execute it after I tried it and I found that when the Do I need to change the program like this? Last attempt result This works normally, but |
|
(Actually, a single connection client is created when you call In normal mode, this is what happens:
redis = Sentinel.master_from()
async with redis:
for request in requests:
await redis.get(key) # just get and return a connection automatically
# now all the pool is closed.The purpose of
|
Thank you for your solution. The second solution is a method that I feel is more understandable, and it indeed reduces the creation process. Thank you very much for your support |
Version: 5.0.1
Platform:
Python 3.11.5(3.11.5 (main, Sep 4 2023, 15:30:52) [GCC 10.2.1 20210110])
Description:
Since 5.0.1, we noticed a significant increase in
No connection available.errors from redis. We are using cashews (6.3.0) + redis-py for request caching for our fastapi application. We don't experience this with 5.0.0.Stack trace:
The text was updated successfully, but these errors were encountered: