From b6dc8bf87e401429008a9f30531171e87cb4ebb3 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 7 Jan 2019 11:58:54 +0100 Subject: [PATCH] DATAREDIS-918 - Enable Lettuce's global timeouts by default. LettuceClientConfiguration now enables Lettuce's global timeouts by default to enable timeouts using the reactive API. This change prevents hanging Redis commands due to a blocked connection or when Redis is down. Timeouts are enabled through defaulting so setting ClientOptions or ClusterClientOptions overrides this behavior. Original Pull Request: #381 --- .../connection/lettuce/LettuceClientConfiguration.java | 7 ++++--- .../lettuce/LettucePoolingClientConfiguration.java | 2 +- .../lettuce/LettuceClientConfigurationUnitTests.java | 9 +++++++-- .../LettucePoolingClientConfigurationUnitTests.java | 9 +++++++-- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfiguration.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfiguration.java index 82bd67e14..ea25094d6 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfiguration.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfiguration.java @@ -18,6 +18,7 @@ package org.springframework.data.redis.connection.lettuce; import io.lettuce.core.ClientOptions; import io.lettuce.core.ReadFrom; import io.lettuce.core.RedisURI; +import io.lettuce.core.TimeoutOptions; import io.lettuce.core.resource.ClientResources; import java.time.Duration; @@ -37,7 +38,7 @@ import org.springframework.util.Assert; *
  • Whether to verify peers using SSL
    • *
  • Whether to use StartTLS
    • *
  • Optional {@link ClientResources}
    • - *
  • Optional {@link ClientOptions}
    • + *
  • Optional {@link ClientOptions}, defaults to {@link ClientOptions} with enabled {@link TimeoutOptions}.
    • *
  • Optional client name
    • *
  • Optional {@link ReadFrom}. Enables Master/Replica operations if configured.
    • *
  • Client {@link Duration timeout}
    • @@ -123,7 +124,7 @@ public interface LettuceClientConfiguration { *
    Start TLS
    *
    no
    *
    Client Options
    - *
    none
    + *
    {@link ClientOptions} with enabled {@link io.lettuce.core.TimeoutOptions}
    *
    Client Resources
    *
    none
    *
    Client name
    @@ -152,7 +153,7 @@ public interface LettuceClientConfiguration { boolean verifyPeer = true; boolean startTls; @Nullable ClientResources clientResources; - @Nullable ClientOptions clientOptions; + ClientOptions clientOptions = ClientOptions.builder().timeoutOptions(TimeoutOptions.enabled()).build(); @Nullable String clientName; @Nullable ReadFrom readFrom; Duration timeout = Duration.ofSeconds(RedisURI.DEFAULT_TIMEOUT); diff --git a/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java b/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java index 24628cb60..39b895975 100644 --- a/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java +++ b/src/main/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfiguration.java @@ -58,7 +58,7 @@ public interface LettucePoolingClientConfiguration extends LettuceClientConfigur *
    Start TLS
    *
    no
    *
    Client Options
    - *
    none
    + *
    {@link ClientOptions} with enabled {@link io.lettuce.core.TimeoutOptions}
    *
    Client Resources
    *
    none
    *
    Connect Timeout
    diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfigurationUnitTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfigurationUnitTests.java index 1570f2ed7..1cfb234da 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfigurationUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettuceClientConfigurationUnitTests.java @@ -18,6 +18,7 @@ package org.springframework.data.redis.connection.lettuce; import static org.assertj.core.api.Assertions.*; import io.lettuce.core.ClientOptions; +import io.lettuce.core.TimeoutOptions; import io.lettuce.core.resource.ClientResources; import java.time.Duration; @@ -32,7 +33,7 @@ import org.junit.Test; */ public class LettuceClientConfigurationUnitTests { - @Test // DATAREDIS-574, DATAREDIS-576, DATAREDIS-667 + @Test // DATAREDIS-574, DATAREDIS-576, DATAREDIS-667, DATAREDIS-918 public void shouldCreateEmptyConfiguration() { LettuceClientConfiguration configuration = LettuceClientConfiguration.defaultConfiguration(); @@ -40,7 +41,11 @@ public class LettuceClientConfigurationUnitTests { assertThat(configuration.isUseSsl()).isFalse(); assertThat(configuration.isVerifyPeer()).isTrue(); assertThat(configuration.isStartTls()).isFalse(); - assertThat(configuration.getClientOptions()).isEmpty(); + assertThat(configuration.getClientOptions()).hasValueSatisfying(actual -> { + + TimeoutOptions timeoutOptions = actual.getTimeoutOptions(); + assertThat(timeoutOptions.isTimeoutCommands()).isTrue(); + }); assertThat(configuration.getClientResources()).isEmpty(); assertThat(configuration.getClientName()).isEmpty(); assertThat(configuration.getCommandTimeout()).isEqualTo(Duration.ofSeconds(60)); diff --git a/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfigurationUnitTests.java b/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfigurationUnitTests.java index f46f54e5c..c2ef087f5 100644 --- a/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfigurationUnitTests.java +++ b/src/test/java/org/springframework/data/redis/connection/lettuce/LettucePoolingClientConfigurationUnitTests.java @@ -18,6 +18,7 @@ package org.springframework.data.redis.connection.lettuce; import static org.assertj.core.api.Assertions.*; import io.lettuce.core.ClientOptions; +import io.lettuce.core.TimeoutOptions; import io.lettuce.core.resource.ClientResources; import java.time.Duration; @@ -33,7 +34,7 @@ import org.junit.Test; */ public class LettucePoolingClientConfigurationUnitTests { - @Test // DATAREDIS-667 + @Test // DATAREDIS-667, DATAREDIS-918 public void shouldCreateEmptyConfiguration() { LettucePoolingClientConfiguration configuration = LettucePoolingClientConfiguration.defaultConfiguration(); @@ -42,7 +43,11 @@ public class LettucePoolingClientConfigurationUnitTests { assertThat(configuration.isUseSsl()).isFalse(); assertThat(configuration.isVerifyPeer()).isTrue(); assertThat(configuration.isStartTls()).isFalse(); - assertThat(configuration.getClientOptions()).isEmpty(); + assertThat(configuration.getClientOptions()).hasValueSatisfying(actual -> { + + TimeoutOptions timeoutOptions = actual.getTimeoutOptions(); + assertThat(timeoutOptions.isTimeoutCommands()).isTrue(); + }); assertThat(configuration.getClientResources()).isEmpty(); assertThat(configuration.getCommandTimeout()).isEqualTo(Duration.ofSeconds(60)); assertThat(configuration.getShutdownTimeout()).isEqualTo(Duration.ofMillis(100));