redis-py icon indicating copy to clipboard operation
redis-py copied to clipboard

Connection pool throws error instead of blocking until connection is available: redis.exceptions.ConnectionError: Too many connections

Open Fogapod opened this issue 2 years ago • 3 comments

Version: 4.4.0 Platform: Linux Description: Connection pool works in unexpected way: when connection limit is reached it throws an error.

Code to reproduce:

import asyncio

from redis import asyncio as redis


async def main() -> None:
    async with redis.Redis(max_connections=2) as rd:
        await asyncio.gather(*[rd.set(str(i), i) for i in range(100)])


asyncio.run(main())

By default redis sets connection limit to ridiculously high number: https://github.com/redis/redis-py/issues/2220 This hides the issue with pool. When set so something realistic like 50 or 100 it starts throwing redis.exceptions.ConnectionError: Too many connections under load.

This bug was introduced in aioredis 2.0, here is relevant issue in aioredis repo: https://github.com/aio-libs/aioredis-py/issues/1173 This comment in particular explains the problem: https://github.com/aio-libs/aioredis-py/issues/1173#issuecomment-1247553011

aredis has the same design for some reason: https://github.com/NoneGG/aredis/issues/167 Other than that I am unfamiliar with async connection pool implementations that would throw errors instead of waiting so I think this is a bug.

I think sync pool works in the same way. Probably not a desired behaviour for sync apps as well.

Fogapod avatar Dec 16 '22 19:12 Fogapod

Looking at source I found BlockingConnectionPool class that works as expected. For some reason it isn't mentioned in documentation and honestly it should be the default pool.

BlockingConnectionPool can be used this way:

import asyncio

from redis import asyncio as redis

async def main() -> None:
    pool = redis.BlockingConnectionPool(max_connections=2)
    async with redis.Redis(connection_pool=pool) as rd:
        await asyncio.gather(*[rd.set(str(i), i) for i in range(100)])


asyncio.run(main())

It solves both problems: has sane number of connections by default and blocks instead of raising exception so that all operations complete successfully.

Another note is that redis client accepting pool object means it ignores quite a lot of configuration present in Redis.__init__, all these parameters are silently ignored and you would have to replicate proper pool setup yourself. A nore pythonic way would be accepting pool_cls and passing pool spicific **kwargs to implementation

Fogapod avatar Dec 17 '22 17:12 Fogapod

I created this PR: https://github.com/redis/redis-py/pull/2618 as a way to pass in the ConnectionPool's class. Otherwise I think all this logic and these defaults have to be copy and pasted outside where the ConnectionPool is then created: https://github.com/redis/redis-py/blob/6c708c2e0511364c2c3f21fa1259de05e590632d/redis/asyncio/client.py#L158-L169 https://github.com/redis/redis-py/blob/6c708c2e0511364c2c3f21fa1259de05e590632d/redis/asyncio/client.py#L217-L248

cancan101 avatar Mar 15 '23 05:03 cancan101

This issue is marked stale. It will be closed in 30 days if it is not updated.

github-actions[bot] avatar Mar 15 '24 00:03 github-actions[bot]