From 8d6d7d32c4215537f272881c04a6efc72ab21d46 Mon Sep 17 00:00:00 2001 From: Nikita Koksharov Date: Mon, 17 Jul 2023 09:47:17 +0300 Subject: [PATCH] Fixed - retryAttempt setting isn't applied during Redisson startup. #3256 --- .../cluster/ClusterConnectionManager.java | 2 +- .../org/redisson/config/ConfigSupport.java | 6 +++- .../connection/ConnectionManager.java | 2 +- .../MasterSlaveConnectionManager.java | 32 +++++++++++++++++-- .../ReplicatedConnectionManager.java | 4 +-- .../connection/SentinelConnectionManager.java | 4 +-- .../test/java/org/redisson/RedissonTest.java | 14 ++++---- 7 files changed, 49 insertions(+), 15 deletions(-) diff --git a/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java b/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java index da5cf1f6a..d6bb82ea8 100644 --- a/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java +++ b/redisson/src/main/java/org/redisson/cluster/ClusterConnectionManager.java @@ -80,7 +80,7 @@ public class ClusterConnectionManager extends MasterSlaveConnectionManager { } @Override - public void connect() { + public void doConnect() { if (cfg.getNodeAddresses().isEmpty()) { throw new IllegalArgumentException("At least one cluster node should be defined!"); } diff --git a/redisson/src/main/java/org/redisson/config/ConfigSupport.java b/redisson/src/main/java/org/redisson/config/ConfigSupport.java index f56657e33..5075e7888 100644 --- a/redisson/src/main/java/org/redisson/config/ConfigSupport.java +++ b/redisson/src/main/java/org/redisson/config/ConfigSupport.java @@ -216,7 +216,11 @@ public class ConfigSupport { throw new IllegalArgumentException("server(s) address(es) not defined!"); } if (!configCopy.isLazyInitialization()) { - cm.connect(); + try { + cm.connect(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } } return cm; } diff --git a/redisson/src/main/java/org/redisson/connection/ConnectionManager.java b/redisson/src/main/java/org/redisson/connection/ConnectionManager.java index 67d75fb3c..a3579b742 100644 --- a/redisson/src/main/java/org/redisson/connection/ConnectionManager.java +++ b/redisson/src/main/java/org/redisson/connection/ConnectionManager.java @@ -31,7 +31,7 @@ import java.util.concurrent.TimeUnit; */ public interface ConnectionManager { - void connect(); + void connect() throws InterruptedException; PublishSubscribeService getSubscribeService(); diff --git a/redisson/src/main/java/org/redisson/connection/MasterSlaveConnectionManager.java b/redisson/src/main/java/org/redisson/connection/MasterSlaveConnectionManager.java index d19c9a841..b282e1cf0 100644 --- a/redisson/src/main/java/org/redisson/connection/MasterSlaveConnectionManager.java +++ b/redisson/src/main/java/org/redisson/connection/MasterSlaveConnectionManager.java @@ -62,6 +62,8 @@ public class MasterSlaveConnectionManager implements ConnectionManager { protected final AtomicReference> lazyConnectLatch = new AtomicReference<>(); + private boolean lastAttempt; + public MasterSlaveConnectionManager(BaseMasterSlaveServersConfig cfg, ServiceManager serviceManager) { this.serviceManager = serviceManager; @@ -170,6 +172,8 @@ public class MasterSlaveConnectionManager implements ConnectionManager { try { connect(); f.complete(null); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); } catch (Exception e) { f.completeExceptionally(e); lazyConnectLatch.set(null); @@ -177,7 +181,31 @@ public class MasterSlaveConnectionManager implements ConnectionManager { } } - public void connect() { + @Override + public final void connect() throws InterruptedException { + for (int i = 0; i < config.getRetryAttempts(); i++) { + try { + if (i == config.getRetryAttempts() - 1) { + lastAttempt = true; + } + doConnect(); + return; + } catch (Exception e) { + try { + Thread.sleep(config.getRetryInterval()); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + return; + } + if (i == config.getRetryAttempts() - 1) { + lastAttempt = false; + throw e; + } + } + } + } + + protected void doConnect() { try { if (config.isSlaveNotUsed()) { masterSlaveEntry = new SingleEntry(this, serviceManager.getConnectionWatcher(), config); @@ -420,7 +448,7 @@ public class MasterSlaveConnectionManager implements ConnectionManager { } protected void internalShutdown() { - if (lazyConnectLatch.get() == null) { + if (lazyConnectLatch.get() == null && lastAttempt) { shutdown(); } } diff --git a/redisson/src/main/java/org/redisson/connection/ReplicatedConnectionManager.java b/redisson/src/main/java/org/redisson/connection/ReplicatedConnectionManager.java index abe27beff..c3aee8133 100644 --- a/redisson/src/main/java/org/redisson/connection/ReplicatedConnectionManager.java +++ b/redisson/src/main/java/org/redisson/connection/ReplicatedConnectionManager.java @@ -72,7 +72,7 @@ public class ReplicatedConnectionManager extends MasterSlaveConnectionManager { } @Override - public void connect() { + public void doConnect() { for (String address : cfg.getNodeAddresses()) { RedisURI addr = new RedisURI(address); CompletionStage connectionFuture = connectToNode(cfg, addr, addr.getHost()); @@ -105,7 +105,7 @@ public class ReplicatedConnectionManager extends MasterSlaveConnectionManager { log.warn("ReadMode = {}, but slave nodes are not found! Please specify all nodes in replicated mode.", this.config.getReadMode()); } - super.connect(); + super.doConnect(); scheduleMasterChangeCheck(cfg); } diff --git a/redisson/src/main/java/org/redisson/connection/SentinelConnectionManager.java b/redisson/src/main/java/org/redisson/connection/SentinelConnectionManager.java index c1cfd44a3..9b7e047ca 100755 --- a/redisson/src/main/java/org/redisson/connection/SentinelConnectionManager.java +++ b/redisson/src/main/java/org/redisson/connection/SentinelConnectionManager.java @@ -84,7 +84,7 @@ public class SentinelConnectionManager extends MasterSlaveConnectionManager { } @Override - public void connect() { + public void doConnect() { checkAuth(cfg); if ("redis".equals(scheme)) { @@ -202,7 +202,7 @@ public class SentinelConnectionManager extends MasterSlaveConnectionManager { log.warn("ReadMode = {}, but slave nodes are not found!", this.config.getReadMode()); } - super.connect(); + super.doConnect(); scheduleChangeCheck(cfg, null); } diff --git a/redisson/src/test/java/org/redisson/RedissonTest.java b/redisson/src/test/java/org/redisson/RedissonTest.java index 71ed6911a..6fd0f369b 100644 --- a/redisson/src/test/java/org/redisson/RedissonTest.java +++ b/redisson/src/test/java/org/redisson/RedissonTest.java @@ -4,6 +4,7 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; import io.netty.util.CharsetUtil; import net.bytebuddy.utility.RandomString; +import org.awaitility.Awaitility; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; @@ -31,6 +32,7 @@ import org.redisson.connection.balancer.RandomLoadBalancer; import java.io.IOException; import java.net.InetSocketAddress; +import java.time.Duration; import java.util.*; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicBoolean; @@ -1237,12 +1239,12 @@ public class RedissonTest extends BaseTest { @Test public void testClusterConnectionFail() { - Assertions.assertThrows(RedisConnectionException.class, () -> { - Config config = new Config(); - config.useClusterServers().addNodeAddress("redis://127.99.0.1:1111"); - Redisson.create(config); - - Thread.sleep(1500); + Awaitility.await().atLeast(Duration.ofSeconds(3)).atMost(Duration.ofSeconds(5)).untilAsserted(() -> { + Assertions.assertThrows(RedisConnectionException.class, () -> { + Config config = new Config(); + config.useClusterServers().addNodeAddress("redis://127.99.0.1:1111"); + Redisson.create(config); + }); }); }