From 7c20dd2f83cbdedb5fa4cb0cc0c89f98e5413bbe Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 18 Aug 2017 09:34:35 +0200 Subject: [PATCH] DATAREDIS-676 - Pass on configured command timeout to LettuceClusterConnection. We now not only configure the underlying lettuce connection with the command timeout set via the connection factory, but also make sure to pass the option on to the LettuceClusterConnection. Original pull request: #266. --- .../lettuce/LettuceClusterConnection.java | 39 ++++++++++++------- .../lettuce/LettuceConnectionFactory.java | 3 +- .../LettuceConnectionFactoryUnitTests.java | 20 ++++++++++ 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnection.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnection.java index 0f8e91f0c..ae5f63858 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClusterConnection.java @@ -16,6 +16,7 @@ package org.springframework.data.redis.connection.lettuce; import io.lettuce.core.RedisException; +import io.lettuce.core.RedisURI; import io.lettuce.core.api.StatefulConnection; import io.lettuce.core.api.sync.BaseRedisCommands; import io.lettuce.core.cluster.RedisClusterClient; @@ -26,6 +27,7 @@ import io.lettuce.core.codec.ByteArrayCodec; import io.lettuce.core.codec.RedisCodec; import io.lettuce.core.pubsub.StatefulRedisPubSubConnection; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; @@ -65,36 +67,47 @@ public class LettuceClusterConnection extends LettuceConnection implements Defau private final RedisClusterClient clusterClient; private ClusterCommandExecutor clusterCommandExecutor; private ClusterTopologyProvider topologyProvider; - private final boolean disposeClusterCommandExecutorOnClose; + private boolean disposeClusterCommandExecutorOnClose; /** - * Creates new {@link LettuceClusterConnection} using {@link RedisClusterClient}. + * Creates new {@link LettuceClusterConnection} using {@link RedisClusterClient} with default + * {@link RedisURI#DEFAULT_TIMEOUT_DURATION timeout} and a fresh {@link ClusterCommandExecutor} that gets destroyed on + * close. * * @param clusterClient must not be {@literal null}. */ public LettuceClusterConnection(RedisClusterClient clusterClient) { - super(null, 100, clusterClient, null, 0); + this(clusterClient, RedisURI.DEFAULT_TIMEOUT_DURATION, + new ClusterCommandExecutor(new LettuceClusterTopologyProvider(clusterClient), + new LettuceClusterNodeResourceProvider(clusterClient), exceptionConverter)); - Assert.notNull(clusterClient, "RedisClusterClient must not be null."); - - this.clusterClient = clusterClient; - topologyProvider = new LettuceClusterTopologyProvider(clusterClient); - clusterCommandExecutor = new ClusterCommandExecutor(topologyProvider, - new LettuceClusterNodeResourceProvider(clusterClient), exceptionConverter); - disposeClusterCommandExecutorOnClose = true; + this.disposeClusterCommandExecutorOnClose = true; } /** - * Creates new {@link LettuceClusterConnection} using {@link RedisClusterClient} running commands across the cluster - * via given {@link ClusterCommandExecutor}. + * Creates new {@link LettuceClusterConnection} with default {@link RedisURI#DEFAULT_TIMEOUT_DURATION timeout} using + * {@link RedisClusterClient} running commands across the cluster via given {@link ClusterCommandExecutor}. * * @param clusterClient must not be {@literal null}. * @param executor must not be {@literal null}. */ public LettuceClusterConnection(RedisClusterClient clusterClient, ClusterCommandExecutor executor) { + this(clusterClient, RedisURI.DEFAULT_TIMEOUT_DURATION, executor); + } - super(null, 100, clusterClient, null, 0); + /** + * Creates new {@link LettuceClusterConnection} with given command {@code timeout} using {@link RedisClusterClient} + * running commands across the cluster via given {@link ClusterCommandExecutor}. + * + * @param clusterClient must not be {@literal null}. + * @param timeout must not be {@literal null}. + * @param executor must not be {@literal null}. + * @since 2.0 + */ + public LettuceClusterConnection(RedisClusterClient clusterClient, Duration timeout, ClusterCommandExecutor executor) { + + super(null, timeout.toMillis(), clusterClient, null, 0); Assert.notNull(clusterClient, "RedisClusterClient must not be null."); Assert.notNull(executor, "ClusterCommandExecutor must not be null."); diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java index 6c0e4a3c3..2ac7fe954 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactory.java @@ -272,7 +272,8 @@ public class LettuceConnectionFactory throw new InvalidDataAccessApiUsageException("Cluster is not configured!"); } - return new LettuceClusterConnection((RedisClusterClient) client, clusterCommandExecutor); + return new LettuceClusterConnection((RedisClusterClient) client, Duration.ofMillis(getTimeout()), + clusterCommandExecutor); } /* diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactoryUnitTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactoryUnitTests.java index dda358492..ff038e159 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceConnectionFactoryUnitTests.java @@ -43,6 +43,7 @@ import org.springframework.data.redis.connection.RedisClusterConfiguration; import org.springframework.data.redis.connection.RedisPassword; import org.springframework.data.redis.connection.RedisSentinelConfiguration; import org.springframework.data.redis.connection.RedisStandaloneConfiguration; +import org.springframework.test.util.ReflectionTestUtils; /** * Unit tests for {@link LettuceConnectionFactory}. @@ -486,4 +487,23 @@ public class LettuceConnectionFactoryUnitTests { connectionFactory.setUseSsl(false); } + + @Test // DATAREDIS-676 + public void timeoutShouldBePassedOnToClusterConnection() { + + LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory(clusterConfig); + connectionFactory.setTimeout(2000); + connectionFactory.afterPropertiesSet(); + + assertThat(ReflectionTestUtils.getField(connectionFactory.getClusterConnection(), "timeout"), is(equalTo(2000L))); + } + + @Test // DATAREDIS-676 + public void timeoutSetOnClientConfigShouldBePassedOnToClusterConnection() { + + LettuceConnectionFactory connectionFactory = new LettuceConnectionFactory(clusterConfig, LettuceClientConfiguration.builder().commandTimeout(Duration.ofSeconds(2)).build()); + connectionFactory.afterPropertiesSet(); + + assertThat(ReflectionTestUtils.getField(connectionFactory.getClusterConnection(), "timeout"), is(equalTo(2000L))); + } }