From 3cfde51a8c6df1bd6ea7e74e9b217d59c45449cd Mon Sep 17 00:00:00 2001 From: bono007 Date: Sun, 26 Jul 2020 17:00:13 -0500 Subject: [PATCH 1/2] Add option to disable Redis Cluster dynamic sources refresh This commit adds an option to enable/disable the DynamicRefreshSources setting on the Lettuce cluster toplogy refresh options. See gh-22571 --- .../redis/LettuceConnectionConfiguration.java | 3 ++ .../data/redis/RedisProperties.java | 15 +++++++++ .../redis/RedisAutoConfigurationTests.java | 31 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java index 6cc58a468d..536dca4d7b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java @@ -138,6 +138,9 @@ class LettuceConnectionConfiguration extends RedisConnectionConfiguration { if (refreshProperties.isAdaptive()) { refreshBuilder.enableAllAdaptiveRefreshTriggers(); } + if (refreshProperties.isDynamicSources() != null) { + refreshBuilder.dynamicRefreshSources(refreshProperties.isDynamicSources()); + } return builder.topologyRefreshOptions(refreshBuilder.build()); } return ClientOptions.builder(); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java index 803f60065d..45ab80e000 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java @@ -410,6 +410,13 @@ public class RedisProperties { */ private boolean adaptive; + /** + * Whether discovered nodes should be used as the source for the cluster + * topology. When set to false, only the initial seed nodes + * will be used as sources for topology discovery. + */ + private Boolean dynamicSources; + public Duration getPeriod() { return this.period; } @@ -426,6 +433,14 @@ public class RedisProperties { this.adaptive = adaptive; } + public Boolean isDynamicSources() { + return this.dynamicSources; + } + + public void setDynamicSources(Boolean dynamicSources) { + this.dynamicSources = dynamicSources; + } + } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java index f6f0c827b1..443c213ab6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java @@ -25,6 +25,7 @@ import java.util.stream.Collectors; import io.lettuce.core.ClientOptions; import io.lettuce.core.cluster.ClusterClientOptions; +import io.lettuce.core.cluster.ClusterTopologyRefreshOptions; import io.lettuce.core.cluster.ClusterTopologyRefreshOptions.RefreshTrigger; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; import org.junit.jupiter.api.Test; @@ -312,6 +313,36 @@ class RedisAutoConfigurationTests { ClientOptions.class, (options) -> assertThat(options.getClass()).isEqualTo(ClientOptions.class))); } + @Test + void testRedisConfigurationWithClusterDynamicSourcesEnabled() { + this.contextRunner + .withPropertyValues("spring.redis.cluster.nodes=127.0.0.1:27379,127.0.0.1:27380", + "spring.redis.lettuce.cluster.refresh.dynamic-sources=true") + .run(assertClientOptions(ClusterClientOptions.class, + (options) -> assertThat(options.getTopologyRefreshOptions().useDynamicRefreshSources()) + .isTrue())); + } + + @Test + void testRedisConfigurationWithClusterDynamicSourcesDisabled() { + this.contextRunner + .withPropertyValues("spring.redis.cluster.nodes=127.0.0.1:27379,127.0.0.1:27380", + "spring.redis.lettuce.cluster.refresh.dynamic-sources=false") + .run(assertClientOptions(ClusterClientOptions.class, + (options) -> assertThat(options.getTopologyRefreshOptions().useDynamicRefreshSources()) + .isFalse())); + } + + @Test + void testRedisConfigurationWithClusterDynamicSourcesUnspecifiedUsesDefault() { + this.contextRunner + .withPropertyValues("spring.redis.cluster.nodes=127.0.0.1:27379,127.0.0.1:27380", + "spring.redis.lettuce.cluster.refresh.dynamic-sources=") + .run(assertClientOptions(ClusterClientOptions.class, + (options) -> assertThat(options.getTopologyRefreshOptions().useDynamicRefreshSources()) + .isEqualTo(ClusterTopologyRefreshOptions.DEFAULT_DYNAMIC_REFRESH_SOURCES))); + } + private ContextConsumer assertClientOptions( Class expectedType, Consumer options) { return (context) -> { From 903dc93887cc797a25cddd388a04edd87214dc77 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 4 Aug 2020 16:33:15 +0200 Subject: [PATCH 2/2] Polish "Add option to disable Redis Cluster dynamic sources refresh" See gh-22571 --- .../redis/LettuceConnectionConfiguration.java | 6 +-- .../data/redis/RedisProperties.java | 28 ++++++------- .../redis/RedisAutoConfigurationTests.java | 8 ++-- .../data/redis/RedisPropertiesTests.java | 42 +++++++++++++++++++ 4 files changed, 62 insertions(+), 22 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisPropertiesTests.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java index 536dca4d7b..dca260f473 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/LettuceConnectionConfiguration.java @@ -131,16 +131,14 @@ class LettuceConnectionConfiguration extends RedisConnectionConfiguration { if (getProperties().getCluster() != null) { ClusterClientOptions.Builder builder = ClusterClientOptions.builder(); Refresh refreshProperties = getProperties().getLettuce().getCluster().getRefresh(); - Builder refreshBuilder = ClusterTopologyRefreshOptions.builder(); + Builder refreshBuilder = ClusterTopologyRefreshOptions.builder() + .dynamicRefreshSources(refreshProperties.isDynamicRefreshSources()); if (refreshProperties.getPeriod() != null) { refreshBuilder.enablePeriodicRefresh(refreshProperties.getPeriod()); } if (refreshProperties.isAdaptive()) { refreshBuilder.enableAllAdaptiveRefreshTriggers(); } - if (refreshProperties.isDynamicSources() != null) { - refreshBuilder.dynamicRefreshSources(refreshProperties.isDynamicSources()); - } return builder.topologyRefreshOptions(refreshBuilder.build()); } return ClientOptions.builder(); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java index 45ab80e000..c762541a1d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/RedisProperties.java @@ -399,6 +399,13 @@ public class RedisProperties { public static class Refresh { + /** + * Whether to discover and query all cluster nodes for obtaining the + * cluster topology. When set to false, only the initial seed nodes are + * used as sources for topology discovery. + */ + private boolean dynamicRefreshSources = true; + /** * Cluster topology refresh period. */ @@ -410,12 +417,13 @@ public class RedisProperties { */ private boolean adaptive; - /** - * Whether discovered nodes should be used as the source for the cluster - * topology. When set to false, only the initial seed nodes - * will be used as sources for topology discovery. - */ - private Boolean dynamicSources; + public boolean isDynamicRefreshSources() { + return this.dynamicRefreshSources; + } + + public void setDynamicRefreshSources(boolean dynamicRefreshSources) { + this.dynamicRefreshSources = dynamicRefreshSources; + } public Duration getPeriod() { return this.period; @@ -433,14 +441,6 @@ public class RedisProperties { this.adaptive = adaptive; } - public Boolean isDynamicSources() { - return this.dynamicSources; - } - - public void setDynamicSources(Boolean dynamicSources) { - this.dynamicSources = dynamicSources; - } - } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java index 443c213ab6..5a1424d21d 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisAutoConfigurationTests.java @@ -314,20 +314,20 @@ class RedisAutoConfigurationTests { } @Test - void testRedisConfigurationWithClusterDynamicSourcesEnabled() { + void testRedisConfigurationWithClusterDynamicRefreshSourcesEnabled() { this.contextRunner .withPropertyValues("spring.redis.cluster.nodes=127.0.0.1:27379,127.0.0.1:27380", - "spring.redis.lettuce.cluster.refresh.dynamic-sources=true") + "spring.redis.lettuce.cluster.refresh.dynamic-refresh-sources=true") .run(assertClientOptions(ClusterClientOptions.class, (options) -> assertThat(options.getTopologyRefreshOptions().useDynamicRefreshSources()) .isTrue())); } @Test - void testRedisConfigurationWithClusterDynamicSourcesDisabled() { + void testRedisConfigurationWithClusterDynamicRefreshSourcesDisabled() { this.contextRunner .withPropertyValues("spring.redis.cluster.nodes=127.0.0.1:27379,127.0.0.1:27380", - "spring.redis.lettuce.cluster.refresh.dynamic-sources=false") + "spring.redis.lettuce.cluster.refresh.dynamic-refresh-sources=false") .run(assertClientOptions(ClusterClientOptions.class, (options) -> assertThat(options.getTopologyRefreshOptions().useDynamicRefreshSources()) .isFalse())); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisPropertiesTests.java new file mode 100644 index 0000000000..0a42e51a73 --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/data/redis/RedisPropertiesTests.java @@ -0,0 +1,42 @@ +/* + * Copyright 2012-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.data.redis; + +import io.lettuce.core.cluster.ClusterTopologyRefreshOptions; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.autoconfigure.data.redis.RedisProperties.Lettuce; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link RedisProperties}. + * + * @author Stephane Nicoll + */ +class RedisPropertiesTests { + + @Test + void lettuceDefaultsAreConsistent() { + Lettuce lettuce = new RedisProperties().getLettuce(); + ClusterTopologyRefreshOptions defaultClusterTopologyRefreshOptions = ClusterTopologyRefreshOptions.builder() + .build(); + assertThat(lettuce.getCluster().getRefresh().isDynamicRefreshSources()) + .isEqualTo(defaultClusterTopologyRefreshOptions.useDynamicRefreshSources()); + } + +}