Redis Sentinel / pool support

[mzealey]

Hi there, In 2.6.0 we are using redis connection pooling to handle ratelimiting. I see in 3.x this has been removed but the redis connection could easily go away in which case I think you cannot correctly handle retries?

Either way, in 2.6.0 there is also no obvious way to support Redis Sentinel for cluster management.

Perhaps a better redis interface would be for the caller to supply an (async)contextmanager reference which will return a redis connection to the redis rate limiter interface, and then it's down to the app to figure out retries/reconnects and whether they use connection pooling, clustering, sentinel etc?

[vutran1710]

Yeah, your point is totally correct! Since I'm about to have some free time in the next few days, I'll try looking into it.

[vutran1710]

To my knowledge, you can setup your redis bucket with either connection pool or redis-sentinel supported like this

Using redis sentinel

async def create_sentinel_redis_bucket(rates: List[Rate]):
    from redis.sentinel import Sentinel

    sentinel = Sentinel([('localhost', 26379)], socket_timeout=0.1)
    master = sentinel.master_for('mymaster', socket_timeout=0.1)
    bucket_key = f"test-bucket/{id_generator()}"
    assert master.ping()
    master.delete(bucket_key)
    bucket = RedisBucket.init(rates, master, bucket_key)
    assert bucket.count() == 0
    return bucket

Or with connection pool, either sync or async

async def create_redis_bucket(rates: List[Rate]):
    from redis import ConnectionPool
    from redis import Redis

    pool = ConnectionPool.from_url(getenv("REDIS", "redis://localhost:6379"))
    redis_db = Redis(connection_pool=pool)
    bucket_key = f"test-bucket/{id_generator()}"
    redis_db.delete(bucket_key)
    bucket = RedisBucket.init(rates, redis_db, bucket_key)
    assert bucket.count() == 0
    return bucket


async def create_async_redis_bucket(rates: List[Rate]):
    from redis.asyncio import ConnectionPool as AsyncConnectionPool
    from redis.asyncio import Redis as AsyncRedis

    pool = AsyncConnectionPool.from_url(getenv("REDIS", "redis://localhost:6379"))
    redis_db: AsyncRedis = AsyncRedis(connection_pool=pool)
    bucket_key = f"test-bucket/{id_generator()}"
    await redis_db.delete(bucket_key)
    bucket = await RedisBucket.init(rates, redis_db, bucket_key)
    assert await bucket.count() == 0
    return bucket

[mzealey]

Yes, however I believe above in your sentinel pool example it returns a connection to the master from the current sentinel configuration, however if that master goes down the connections will break and not automatically switch over to the new master.

Hence needing to call master_for() each time we go into a redis section to pull a (potentially new) master connection out of the pool

[vutran1710]

So if apparently redis' Sentinel class does not retry on master failover for us automatically, can we can create a supporting class for it and pass it to the RedisBucket class?

[vutran1710]

I think this might work

class SentinelRedis:
    from redis.sentinel import Sentinel

    """A class that wraps around redis sentinel and automatically resolves the master node"""
    def __init__(self, sentinel: Sentinel):
        self.sentinel = sentinel
        self.available_methods = [
            "evalsha",
            "zremrangebyscore",
            "delete",
            "zcard",
            "zrange"
        ]

    def __getattr__(self, name):
        if name not in self.available_methods:
            raise AttributeError

        master = None
        while master is None:
            try:
                found_master = self.sentinel.discover_master("mymaster")
                master = found_master
            except Exception as err:
                logger.error("Error while discovering master: %s", err)

        return getattr(master, name)

And then...

async def create_sentinel_redis_bucket(rates: List[Rate]) -> AbstractBucket:
    from redis.sentinel import Sentinel
    
    redis_db = SentinelRedis(Sentinel([("localhost", 26379)], socket_timeout=0.1))
    bucket = RedisBucket.init(rates, redis_db, "test-bucket")
    return bucket

[mzealey]

OK reading through the code in redis 5 it looks like I can do something like:

sentinel = Sentinel(...)
master_pool = SentinelConnectionPool("service", sentinel, is_master=True, ...)

and pass that pool in and hopefully it auto-reconnects to the master as required.

[vutran1710]

I got this working. Probably add the snippet to the documentation if you believe the setup is correct

from redis.sentinel import Sentinel
from redis.sentinel import SentinelConnectionPool
from redis import Redis

sentinel = Sentinel([('localhost', 26379)], socket_timeout=0.1)
pool = SentinelConnectionPool("mymaster", sentinel, ssl=False, check_connection=True, is_master=True, decode_responses=True)
redis_db = Redis(connection_pool=pool)
bucket_key = f"test-sentinel/{id_generator()}"
bucket = RedisBucket.init(rates, redis_db, bucket_key)

[mzealey]

OK We're still using limiter 2.5.0 (not 2.6.0 as I said in my initial ticket) so I think it's slightly different with what is required to go in. If you are doing the above then just .master_for() will work because it effectively does your pool/redis_db steps. But for 2.5.0 I believe the connection pool is passed in rather than the Redis object itself.

Either way I tihnk this is resolved now thank you :)