aredis icon indicating copy to clipboard operation
aredis copied to clipboard

Questionable implementation of disconnect on idle

Open olii opened this issue 5 years ago • 3 comments

Checklist

  • Python version 3.8
  • hiredis
  • asyncio event loop
  • Does issue exists against the master branch of aredis? Yes

I have a question about the implementation of this method: https://github.com/NoneGG/aredis/blob/bea279b7d436242b221f7e9aabb13f1484ec261c/aredis/pool.py#L183-L191

When I need to terminate the application and empty the event loop I need to wait until all the connections get disconnected because of the idle timeout. Is there a way to force the connections to exit early so I don't have to wait for the max_idle_time seconds until the connections are released and the spawned tasks named disconnect_on_idle_time_exceeded() are finished?

olii avatar Feb 26 '20 10:02 olii

@olii Is it max_idle_time stil needed in your scene? Actually this is used for cleaning too many useless connections. You can just set max_idle_time to zero when initialize a redis client if you want the connection to get disconnected quickly.

Could you describe your scene more detailedly? According to your description, it seems like there is a service, which will create a lot of connections(which may be idle later) and it will manually clean these connections/loop task?

NoneGG avatar Feb 29 '20 14:02 NoneGG

You are right. It is a service that spawns tasks to do some jobs with Redis. As a service, it should be running for a long time. When there are N tasks created at once there is a high probability that N connections will be created. This behavior was not clear from the docs. In my case, there might be a peak when 1500 tasks/second are created but on average it is about 800/s. For a long period of time, some of them are only idle and consume resources on the Redis server.

What I wanted to achieve is to have a limited number of connections (let's say 10) and use them to do all the work. When the has no more available connections the operation will have wait until one is available. The current strategy is to raise an exception.

To overcome my issue I just wrapped some methods and deployed asyncio.Semaphore to limit the concurrency.

However, to get to the original issue, I was able to remove the max_idle_time completely using semaphore but I still think that it would be good to have an explicit way of closing all the connections without waiting for max_idle_time when the application is about to exit. What do you think?

olii avatar Feb 29 '20 18:02 olii

@olii Your requirement is reasonable. Actually the point is to clean useless coroutine (disconnect_on_idle_time_exceeded), these coroutine will wait until max_idle_time is reached. I will check if there is a better choice to do this.

If you don't need to clean all coroutines, you can just disconnect connections by ConnectionPool.disconnect

NoneGG avatar Mar 01 '20 02:03 NoneGG