Redisson SentinelConnectionManager is too aggresive on startup

Redis version

7.4.9

Redisson version

3.45.0 But code still appears present

Redisson configuration

Kubernetes 10 Second connection timeout 3 retries Pointed at k8s service address which resolves into one of the sentinels Sentinel Discovery enabled Read mode: slave 3 sentinels, 3 redises

What is the Expected behavior?

On startup I would expect redisson to connect in a similar manor to how checkSlavesChange and checkSentinelsChange behave (In 3.45 this behavior is also wrong, but it has since been fixed), it gets a list of replicas/sentinels, checks if they are marked as up or s_down, then if they are up resolves the IP and finally tries to connect Correct replica behavior:

                if (isDown(flags, masterLinkStatus)) {
                    RedisURI uri = serviceManager.toURI(scheme, host, port);
                    slaveDown(uri);
                    continue;
                }

                String masterHost = map.get("master-host");
                String masterPort = map.get("master-port");

                CompletableFuture<InetSocketAddress> slaveAddrFuture = resolveIP(host, port);

Correct sentinel behavior

                if (!m.isEmpty() && !isDown(flags, masterLinkStatus)) {
                    return true;
                }
                return false;
            }).map(m -> {
                String ip = m.get("ip");
                String port = m.get("port");
                CompletionStage<InetSocketAddress> f = resolveIP(ip, port);

What is the Actual behavior?

On startup, the initial connection to slaves and sentinels takes place in the wrong order:

Sentinels

Sentinels do not check for up/down status before connecting, because sentinels never forget a redis/sentinel naturally, an IP change/kubernetes pod reschedule will result in a sentinel tracking an old entry, this is expected behavior but redisson will always try to connect to this on startup, which adds (in our case) a guaranteed ~10 seconds to the startup duration that is not really needed Incorrect behavior

                    if (cfg.isSentinelsDiscovery()) {
                        List<Map<String, String>> sentinelSentinels = connection.sync(StringCodec.INSTANCE, RedisCommands.SENTINEL_SENTINELS, cfg.getMasterName());
                        for (Map<String, String> map : sentinelSentinels) {
                            if (map.isEmpty()) {
                                continue;
                            }

                            String ip = map.get("ip");
                            String port = map.get("port");
                           <- This should check for s_down :(

                            InetSocketAddress sentinelAddr = resolveIP(ip, port).join();
                            CompletionStage<Void> future = registerSentinel(sentinelAddr);
                            connectionFutures.add(future.toCompletableFuture());
                        }
                    }

Replicas

This issue was partially fixed in https://github.com/redisson/redisson/issues/7004, which gave the correct behavior for the periodic checkSlavesChange check, however the same issue is also present in the startup code and was not fixed This causes exceptions and retries as it fails to resolve the a nonexistent hostname, over the course of its retries it can sucessfully start up most of the time, but only if the initial connection step is provided multiple initial sentinel hostnames, or if the one given resolves to multiple allowing it to get the 2 required to start (Another possible fix here would be to check for other sentinels first, that way it would be more able to pass the checkSentinelsList check)

Incorrect behavior:

                        InetSocketAddress slaveAddr = resolveIP(host, port).join(); <- resolve here
                        RedisURI uri = toURI(slaveAddr);
                        if (isHostname(host)) {
                            uri2hostname.put(uri, host);
                        }

                        log.debug("slave {} state: {}", slaveAddr, map);

                        if (isSlaveDown(flags, masterLinkStatus)) { <- check down here :(

Additional information

These issues are largely dependent on if you are using hostnames or not for redis, the first issue (dead sentinels) is far more likely when using IPs (though can still happen with hostnames) and the second (failing to resolve replica hostname) is obviously more common when hostnames are used but become invalid (for example, scaling down the number of replicas in k8s)

None of these a hugely impactful, but it would be nice if the startup code has the same fixes as the periodic checks did There may be some reason for why these things are not done (perhaps to make sure that if an isolated sentinel is the first one seen we try to connect to the others? I'm not sure though