From 76098c6adca5603fc166fa2a2af9f7cf31e52317 Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Tue, 3 Jun 2025 16:52:48 +0300 Subject: [PATCH 1/6] Enable adaptive refresh by default #3249 --- .../ClusterTopologyRefreshOptions.java | 54 +++++++++++++++++-- ...lusterTopologyRefreshOptionsUnitTests.java | 2 + ...sterTopologyRefreshSchedulerUnitTests.java | 17 +++++- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java index 882e4718f7..ac35e61e5e 100644 --- a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java +++ b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java @@ -33,14 +33,19 @@ * Options to control the Cluster topology refreshing of {@link RedisClusterClient}. * * @author Mark Paluch + * @author Tihomir Mateev * @since 4.2 */ public class ClusterTopologyRefreshOptions { - public static final Set DEFAULT_ADAPTIVE_REFRESH_TRIGGERS = Collections.emptySet(); + /** + * Since Lettuce 7.0 all adaptive triggers are enabled by default + */ + public static final Set DEFAULT_ADAPTIVE_REFRESH_TRIGGERS = EnumSet.allOf(RefreshTrigger.class); public static final long DEFAULT_ADAPTIVE_REFRESH_TIMEOUT = 30; + @Deprecated public static final TimeUnit DEFAULT_ADAPTIVE_REFRESH_TIMEOUT_UNIT = TimeUnit.SECONDS; public static final Duration DEFAULT_ADAPTIVE_REFRESH_TIMEOUT_DURATION = Duration @@ -54,6 +59,7 @@ public class ClusterTopologyRefreshOptions { public static final long DEFAULT_REFRESH_PERIOD = 60; + @Deprecated public static final TimeUnit DEFAULT_REFRESH_PERIOD_UNIT = TimeUnit.SECONDS; public static final Duration DEFAULT_REFRESH_PERIOD_DURATION = Duration.ofSeconds(DEFAULT_REFRESH_PERIOD); @@ -159,12 +165,16 @@ private Builder() { * Enables adaptive topology refreshing using one or more {@link RefreshTrigger triggers}. Adaptive refresh triggers * initiate topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an * immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on - * a large scale. Adaptive refresh triggers are disabled by default. See also + * a large scale. Adaptive refresh triggers are all enabled by default. See also * {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}. * * @param refreshTrigger one or more {@link RefreshTrigger} to enabled * @return {@code this} + * @deprecated Starting from 7.0, this method has no effect as all adaptive triggers are enabled by default. + * @see #disableAllAdaptiveRefreshTriggers() + * @see #disableAdaptiveRefreshTrigger(RefreshTrigger...) */ + @Deprecated public Builder enableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) { LettuceAssert.notNull(refreshTrigger, "RefreshTriggers must not be null"); @@ -179,16 +189,54 @@ public Builder enableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) { * Enables adaptive topology refreshing using all {@link RefreshTrigger triggers}. Adaptive refresh triggers initiate * topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an * immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on - * a large scale. Adaptive refresh triggers are disabled by default. See also + * a large scale. Adaptive refresh triggers are all enabled by default. See also * {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}. * * @return {@code this} + * @deprecated Starting from 7.0, this method has no effect as all adaptive triggers are enabled by default. + * @see #disableAllAdaptiveRefreshTriggers() + * @see #disableAdaptiveRefreshTrigger(RefreshTrigger...) */ + @Deprecated public Builder enableAllAdaptiveRefreshTriggers() { adaptiveRefreshTriggers.addAll(EnumSet.allOf(RefreshTrigger.class)); return this; } + /** + * Disables adaptive topology refreshing using one or more {@link RefreshTrigger triggers}. Adaptive refresh triggers + * initiate topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an + * immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on + * a large scale. Adaptive refresh triggers are all enabled by default. See also + * {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}. + * + * @param refreshTrigger one or more {@link RefreshTrigger} to enabled + * @return {@code this} + */ + public Builder disableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) { + + LettuceAssert.notNull(refreshTrigger, "RefreshTriggers must not be null"); + LettuceAssert.noNullElements(refreshTrigger, "RefreshTriggers must not contain null elements"); + LettuceAssert.notEmpty(refreshTrigger, "RefreshTriggers must contain at least one element"); + + Arrays.asList(refreshTrigger).forEach(adaptiveRefreshTriggers::remove); + return this; + } + + /** + * Disables adaptive topology refreshing using all {@link RefreshTrigger triggers}. Adaptive refresh triggers initiate + * topology view updates based on events happened during Redis Cluster operations. Adaptive triggers lead to an + * immediate topology refresh. Adaptive triggered refreshes are rate-limited using a timeout since events can happen on + * a large scale. Adaptive refresh triggers are all enabled by default. See also + * {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}. + * + * @return {@code this} + */ + public Builder disableAllAdaptiveRefreshTriggers() { + adaptiveRefreshTriggers.clear(); + return this; + } + /** * Set the timeout for adaptive topology updates. This timeout is to rate-limit topology updates initiated by refresh * triggers to one topology refresh per timeout. Defaults to {@literal 30 SECONDS}. See {@link #DEFAULT_REFRESH_PERIOD} diff --git a/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptionsUnitTests.java b/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptionsUnitTests.java index 2844fd7fa4..4d980fea0c 100644 --- a/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptionsUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptionsUnitTests.java @@ -25,6 +25,7 @@ void testBuilder() { ClusterTopologyRefreshOptions options = ClusterTopologyRefreshOptions.builder()// .enablePeriodicRefresh(true).refreshPeriod(10, TimeUnit.MINUTES)// .dynamicRefreshSources(false) // + .disableAllAdaptiveRefreshTriggers()// .enableAdaptiveRefreshTrigger(RefreshTrigger.MOVED_REDIRECT)// .adaptiveRefreshTriggersTimeout(15, TimeUnit.MILLISECONDS)// .closeStaleConnections(false)// @@ -46,6 +47,7 @@ void testCopy() { ClusterTopologyRefreshOptions master = ClusterTopologyRefreshOptions.builder()// .enablePeriodicRefresh(true).refreshPeriod(10, TimeUnit.MINUTES)// .dynamicRefreshSources(false) // + .disableAllAdaptiveRefreshTriggers()// .enableAdaptiveRefreshTrigger(RefreshTrigger.MOVED_REDIRECT)// .adaptiveRefreshTriggersTimeout(15, TimeUnit.MILLISECONDS)// .closeStaleConnections(false)// diff --git a/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshSchedulerUnitTests.java b/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshSchedulerUnitTests.java index 327f49c97d..070bdc3886 100644 --- a/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshSchedulerUnitTests.java +++ b/src/test/java/io/lettuce/core/cluster/ClusterTopologyRefreshSchedulerUnitTests.java @@ -132,7 +132,7 @@ void shouldTriggerRefreshOnAskRedirection() { } @Test - void shouldNotTriggerAdaptiveRefreshUsingDefaults() { + void shouldTriggerAdaptiveRefreshUsingDefaults() { ClusterTopologyRefreshOptions clusterTopologyRefreshOptions = ClusterTopologyRefreshOptions.create(); @@ -141,6 +141,21 @@ void shouldNotTriggerAdaptiveRefreshUsingDefaults() { when(clusterClient.getClusterClientOptions()).thenReturn(clusterClientOptions); + sut.onAskRedirection(); + verify(eventExecutors).submit(any(Runnable.class)); + } + + @Test + void shouldNotTriggerAdaptiveRefreshWhenDisabled() { + + ClusterTopologyRefreshOptions clusterTopologyRefreshOptions = ClusterTopologyRefreshOptions.builder() + .disableAllAdaptiveRefreshTriggers().build(); + + ClusterClientOptions clusterClientOptions = ClusterClientOptions.builder() + .topologyRefreshOptions(clusterTopologyRefreshOptions).build(); + + when(clusterClient.getClusterClientOptions()).thenReturn(clusterClientOptions); + sut.onAskRedirection(); verify(eventExecutors, never()).submit(any(Runnable.class)); } From 19673863f8d43e6e303f32a125d95eb465d99120 Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Tue, 3 Jun 2025 16:59:10 +0300 Subject: [PATCH 2/6] Added @since --- .../io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java index ac35e61e5e..0d8940fd78 100644 --- a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java +++ b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java @@ -212,6 +212,7 @@ public Builder enableAllAdaptiveRefreshTriggers() { * * @param refreshTrigger one or more {@link RefreshTrigger} to enabled * @return {@code this} + * @since 7.0 */ public Builder disableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) { @@ -231,6 +232,7 @@ public Builder disableAdaptiveRefreshTrigger(RefreshTrigger... refreshTrigger) { * {@link #adaptiveRefreshTriggersTimeout(long, TimeUnit)} and {@link RefreshTrigger}. * * @return {@code this} + * @since 7.0 */ public Builder disableAllAdaptiveRefreshTriggers() { adaptiveRefreshTriggers.clear(); From 30f0cb71305dcd13aaeccd8ffc38e45749e82331 Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Tue, 3 Jun 2025 20:53:47 +0300 Subject: [PATCH 3/6] Fix integration test [incomplete] --- .../core/cluster/RedisClusterSetupIntegrationTests.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java b/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java index 62ac7848a2..de854affd6 100644 --- a/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java +++ b/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java @@ -240,10 +240,6 @@ public void changeTopologyWhileOperations() throws Exception { ClusterSetup.setup2Masters(clusterHelper); - ClusterTopologyRefreshOptions clusterTopologyRefreshOptions = ClusterTopologyRefreshOptions.builder() - .enableAllAdaptiveRefreshTriggers().build(); - - clusterClient.setOptions(ClusterClientOptions.builder().topologyRefreshOptions(clusterTopologyRefreshOptions).build()); StatefulRedisClusterConnection connection = clusterClient.connect(); RedisAdvancedClusterCommands sync = connection.sync(); RedisAdvancedClusterAsyncCommands async = connection.async(); From 604cada9a3aeebd820f79883ba16cf54457bc2e8 Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Thu, 5 Jun 2025 17:50:48 +0300 Subject: [PATCH 4/6] Add also fix to issue 3250 --- .../io/lettuce/core/cluster/ClusterClientOptions.java | 11 ++++++++--- .../core/cluster/ClusterTopologyRefreshOptions.java | 4 +--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/lettuce/core/cluster/ClusterClientOptions.java b/src/main/java/io/lettuce/core/cluster/ClusterClientOptions.java index 832c310f54..05a50326b5 100644 --- a/src/main/java/io/lettuce/core/cluster/ClusterClientOptions.java +++ b/src/main/java/io/lettuce/core/cluster/ClusterClientOptions.java @@ -53,7 +53,8 @@ public class ClusterClientOptions extends ClientOptions { public static final Duration DEFAULT_REFRESH_PERIOD_DURATION = Duration.ofSeconds(DEFAULT_REFRESH_PERIOD); - public static final boolean DEFAULT_VALIDATE_CLUSTER_MEMBERSHIP = true; + /** Since Lettuce 7.0 validation is by default disabled. */ + public static final boolean DEFAULT_VALIDATE_CLUSTER_MEMBERSHIP = false; public static final Predicate DEFAULT_NODE_FILTER = node -> true; @@ -310,8 +311,12 @@ public Builder topologyRefreshOptions(ClusterTopologyRefreshOptions topologyRefr } /** - * Validate the cluster node membership before allowing connections to a cluster node. Defaults to {@code true}. See + * Validate the cluster node membership before allowing connections to a cluster node. Defaults to {@code false}. See * {@link ClusterClientOptions#DEFAULT_VALIDATE_CLUSTER_MEMBERSHIP}. + *

+ * Since 7.0, validation is disabled by default, as it is causing problems in some upgrade scenarios. In scenarios where + * upgraded nodes are added to the cluster the ASK / MOVED replies usually come before the topology is refreshed and - + * respectively - this validation would fail. * * @param validateClusterNodeMembership {@code true} if validation is enabled. * @return {@code this} @@ -425,7 +430,7 @@ public ClusterTopologyRefreshOptions getTopologyRefreshOptions() { } /** - * Validate the cluster node membership before allowing connections to a cluster node. Defaults to {@code true}. + * Validate the cluster node membership before allowing connections to a cluster node. Defaults to {@code false}. * * @return {@code true} if validation is enabled. */ diff --git a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java index 0d8940fd78..16ec16e6be 100644 --- a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java +++ b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java @@ -38,9 +38,7 @@ */ public class ClusterTopologyRefreshOptions { - /** - * Since Lettuce 7.0 all adaptive triggers are enabled by default - */ + /** Since Lettuce 7.0 all adaptive triggers are enabled by default */ public static final Set DEFAULT_ADAPTIVE_REFRESH_TRIGGERS = EnumSet.allOf(RefreshTrigger.class); public static final long DEFAULT_ADAPTIVE_REFRESH_TIMEOUT = 30; From 5f9f633bc8b69a5751d388dedf48e29c72a051cd Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Mon, 7 Jul 2025 10:47:38 +0300 Subject: [PATCH 5/6] Change default to 5 seconds --- .../lettuce/core/cluster/ClusterTopologyRefreshOptions.java | 3 ++- .../core/cluster/RedisClusterSetupIntegrationTests.java | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java index 16ec16e6be..13bae0d998 100644 --- a/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java +++ b/src/main/java/io/lettuce/core/cluster/ClusterTopologyRefreshOptions.java @@ -41,7 +41,8 @@ public class ClusterTopologyRefreshOptions { /** Since Lettuce 7.0 all adaptive triggers are enabled by default */ public static final Set DEFAULT_ADAPTIVE_REFRESH_TRIGGERS = EnumSet.allOf(RefreshTrigger.class); - public static final long DEFAULT_ADAPTIVE_REFRESH_TIMEOUT = 30; + /** Since Lettuce 7.0 the default adaptive refresh timeout is 5 seconds */ + public static final long DEFAULT_ADAPTIVE_REFRESH_TIMEOUT = 5; @Deprecated public static final TimeUnit DEFAULT_ADAPTIVE_REFRESH_TIMEOUT_UNIT = TimeUnit.SECONDS; diff --git a/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java b/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java index de854affd6..fb3fc97b70 100644 --- a/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java +++ b/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java @@ -240,6 +240,11 @@ public void changeTopologyWhileOperations() throws Exception { ClusterSetup.setup2Masters(clusterHelper); + ClusterTopologyRefreshOptions clusterTopologyRefreshOptions = ClusterTopologyRefreshOptions.builder() + .enableAllAdaptiveRefreshTriggers().build(); + + clusterClient.setOptions(ClusterClientOptions.builder().topologyRefreshOptions(clusterTopologyRefreshOptions).build()); + StatefulRedisClusterConnection connection = clusterClient.connect(); RedisAdvancedClusterCommands sync = connection.sync(); RedisAdvancedClusterAsyncCommands async = connection.async(); From 717e9f45eb396690e9553ef01df6faacb6917dcc Mon Sep 17 00:00:00 2001 From: Tihomir Mateev Date: Mon, 7 Jul 2025 11:07:41 +0300 Subject: [PATCH 6/6] Remove unnecessary new line --- .../lettuce/core/cluster/RedisClusterSetupIntegrationTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java b/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java index fb3fc97b70..62ac7848a2 100644 --- a/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java +++ b/src/test/java/io/lettuce/core/cluster/RedisClusterSetupIntegrationTests.java @@ -244,7 +244,6 @@ public void changeTopologyWhileOperations() throws Exception { .enableAllAdaptiveRefreshTriggers().build(); clusterClient.setOptions(ClusterClientOptions.builder().topologyRefreshOptions(clusterTopologyRefreshOptions).build()); - StatefulRedisClusterConnection connection = clusterClient.connect(); RedisAdvancedClusterCommands sync = connection.sync(); RedisAdvancedClusterAsyncCommands async = connection.async();