Rotating Slaves in Sentinel fails when hitting last slave in SentinelConnectionPool.

[imclvr]

Version: Redis 7.0.12 and redis-py 4.6.0

Platform: Not relevant, as issue seems to come from the codebase.

Description:

When attempting to instance a client to one slave, RedisSentinelConnectionPool performs some kind of round robin balancing, implemented here:

    async def rotate_slaves(self) -> AsyncIterator:
        """Round-robin slave balancer"""
        slaves = await self.sentinel_manager.discover_slaves(self.service_name)
        if slaves:
            if self.slave_rr_counter is None:
                self.slave_rr_counter = random.randint(0, len(slaves) - 1)
            for _ in range(len(slaves)):
                self.slave_rr_counter = (self.slave_rr_counter + 1) % len(slaves)
                slave = slaves[self.slave_rr_counter]
                yield slave
        # Fallback to the master connection
        try:
            yield await self.get_master_address()
        except MasterNotFoundError:
            pass
        raise SlaveNotFoundError(f"No slave found for {self.service_name!r}")

I noticed two issues here:

• Once that the round robin algorithm hits the last slave, I would expect that the counter is placed again in the first slave and the first slave is hence returned. However, a SlaveNotFoundError is thrown, which makes the usage of SentinelConnectionPool to fail eventually.

• When fetching a slave, I do not expect to get master in return. That may harm performance on applications, as read-only replicas are not guaranteed to be always returned.

So, it would be great that returning master would be optional (e.g. configured from a flag passed as parameter to slave_for).

Also, some logging saying that master is returned as fallback would be appreciated.

[ljluestc]

import random
import logging
from typing import AsyncIterator

class RedisSentinelConnectionPool:
    def __init__(self, sentinel_manager, service_name, fallback_to_master=True):
        self.sentinel_manager = sentinel_manager
        self.service_name = service_name
        self.fallback_to_master = fallback_to_master
        self.slave_rr_counter = None

    async def rotate_slaves(self) -> AsyncIterator:
        """Round-robin slave balancer"""
        slaves = await self.sentinel_manager.discover_slaves(self.service_name)
        if slaves:
            if self.slave_rr_counter is None:
                self.slave_rr_counter = random.randint(0, len(slaves) - 1)
            for _ in range(len(slaves)):
                self.slave_rr_counter = (self.slave_rr_counter + 1) % len(slaves)
                slave = slaves[self.slave_rr_counter]
                yield slave
        if self.fallback_to_master:
            # Fallback to the master connection
            try:
                master = await self.get_master_address()
                logging.info("Master returned as fallback for service: %s", self.service_name)
                yield master
            except MasterNotFoundError:
                logging.error("Master not found for service: %s", self.service_name)
                pass
        raise SlaveNotFoundError(f"No slave found for {self.service_name!r}")

    async def get_master_address(self):
        # Mock function to simulate getting the master address
        raise NotImplementedError

class MasterNotFoundError(Exception):
    pass

class SlaveNotFoundError(Exception):
    pass

# Example usage
async def main():
    sentinel_manager = None  # Replace with actual sentinel manager
    pool = RedisSentinelConnectionPool(sentinel_manager, "mymaster", fallback_to_master=True)
    try:
        async for slave in pool.rotate_slaves():
            print(f"Using slave: {slave}")
    except SlaveNotFoundError as e:
        print(str(e))

# In an async environment, run the main function
# await main()