Skip to content

chore: remove deprecated default timeout from AbstractRedisClient (#3328) #3344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/user-guide/connecting-redis.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ the error message. `RedisException` is a `RuntimeException`.
### Examples

``` java
RedisClient client = RedisClient.create(RedisURI.create("localhost", 6379));
client.setDefaultTimeout(20, TimeUnit.SECONDS);
RedisURI uri = new RedisURI("localhost", 6379, Duration.ofSeconds(20));
RedisClient client = RedisClient.create(uri);

// …

Expand Down
43 changes: 0 additions & 43 deletions src/main/java/io/lettuce/core/AbstractRedisClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ public abstract class AbstractRedisClient implements AutoCloseable {

private volatile ClientOptions clientOptions = ClientOptions.create();

private volatile Duration defaultTimeout = RedisURI.DEFAULT_TIMEOUT_DURATION;

/**
* Create a new instance with client resources.
*
Expand All @@ -134,47 +132,6 @@ protected int getChannelCount() {
return channels.size();
}

/**
* Returns the default {@link Duration timeout} for commands.
*
* @return the default {@link Duration timeout} for commands.
* @deprecated since 6.2, use {@link RedisURI#getTimeout()} to control timeouts.
*/
@Deprecated
public Duration getDefaultTimeout() {
return defaultTimeout;
}

/**
* Set the default timeout for connections created by this client. The timeout applies to connection attempts and
* non-blocking commands.
*
* @param timeout default connection timeout, must not be {@code null}.
* @since 5.0
* @deprecated since 6.2, use {@link RedisURI#getTimeout()} to control timeouts.
*/
@Deprecated
public void setDefaultTimeout(Duration timeout) {

LettuceAssert.notNull(timeout, "Timeout duration must not be null");
LettuceAssert.isTrue(!timeout.isNegative(), "Timeout duration must be greater or equal to zero");

this.defaultTimeout = timeout;
}

/**
* Set the default timeout for connections created by this client. The timeout applies to connection attempts and
* non-blocking commands.
*
* @param timeout Default connection timeout.
* @param unit Unit of time for the timeout.
* @deprecated since 6.2, use {@link RedisURI#getTimeout()} to control timeouts.
*/
@Deprecated
public void setDefaultTimeout(long timeout, TimeUnit unit) {
setDefaultTimeout(Duration.ofNanos(unit.toNanos(timeout)));
}

/**
* Returns the {@link ClientOptions} which are valid for that client. Connections inherit the current options at the moment
* the connection is created. Changes to options will not affect existing connections.
Expand Down
9 changes: 4 additions & 5 deletions src/main/java/io/lettuce/core/RedisClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ protected RedisClient(ClientResources clientResources, RedisURI redisURI) {
assertNotNull(redisURI);

this.redisURI = redisURI;
setDefaultTimeout(redisURI.getTimeout());
}

/**
Expand Down Expand Up @@ -217,7 +216,7 @@ public <K, V> StatefulRedisConnection<K, V> connect(RedisCodec<K, V> codec) {

checkForRedisURI();

return getConnection(connectStandaloneAsync(codec, this.redisURI, getDefaultTimeout()));
return getConnection(connectStandaloneAsync(codec, this.redisURI, this.redisURI.getTimeout()));
}

/**
Expand Down Expand Up @@ -339,7 +338,7 @@ private <K, V, S> ConnectionFuture<S> connectStatefulAsync(StatefulRedisConnecti
* @return A new stateful pub/sub connection
*/
public StatefulRedisPubSubConnection<String, String> connectPubSub() {
return getConnection(connectPubSubAsync(newStringStringCodec(), this.redisURI, getDefaultTimeout()));
return getConnection(connectPubSubAsync(newStringStringCodec(), this.redisURI, this.redisURI.getTimeout()));
}

/**
Expand All @@ -366,7 +365,7 @@ public StatefulRedisPubSubConnection<String, String> connectPubSub(RedisURI redi
*/
public <K, V> StatefulRedisPubSubConnection<K, V> connectPubSub(RedisCodec<K, V> codec) {
checkForRedisURI();
return getConnection(connectPubSubAsync(codec, this.redisURI, getDefaultTimeout()));
return getConnection(connectPubSubAsync(codec, this.redisURI, this.redisURI.getTimeout()));
}

/**
Expand Down Expand Up @@ -453,7 +452,7 @@ public StatefulRedisSentinelConnection<String, String> connectSentinel() {
*/
public <K, V> StatefulRedisSentinelConnection<K, V> connectSentinel(RedisCodec<K, V> codec) {
checkForRedisURI();
return getConnection(connectSentinelAsync(codec, this.redisURI, getDefaultTimeout()));
return getConnection(connectSentinelAsync(codec, this.redisURI, this.redisURI.getTimeout()));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ protected RedisClusterClient(ClientResources clientResources, Iterable<RedisURI>
this.initialUris = Collections.unmodifiableList(LettuceLists.newList(redisURIs));
this.refresh = createTopologyRefresh();

setDefaultTimeout(getFirstUri().getTimeout());
setOptions(ClusterClientOptions.create());
}

Expand Down
4 changes: 2 additions & 2 deletions src/main/javadoc/overview.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
</p>

<p>
All connections inherit a default timeout from their {@link io.lettuce.core.RedisClient}
All connections inherit a default timeout from their {@link io.lettuce.core.RedisURI}
and will throw a {@link io.lettuce.core.RedisException} when non-blocking commands fail
to return a result before the timeout expires. The timeout defaults to 60 seconds and
may be changed via {@link io.lettuce.core.RedisClient#setDefaultTimeout} or for
may be changed via {@link io.lettuce.core.RedisURI#setTimeout} or for
each individual connection.
</p>
</body>
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import javax.inject.Inject;

import io.lettuce.core.protocol.ProtocolVersion;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
Expand All @@ -48,40 +47,34 @@
/**
* @author Mark Paluch
* @author Jongyeol Choi
* @author Hari Mani
*/
@ExtendWith(LettuceExtension.class)
@Tag(INTEGRATION_TEST)
class RedisClientConnectIntegrationTests extends TestSupport {

private static final Duration EXPECTED_TIMEOUT = Duration.ofMillis(500);

private final RedisClient client;

@Inject
RedisClientConnectIntegrationTests(RedisClient client) {
this.client = client;
}

@BeforeEach
void before() {
client.setDefaultTimeout(EXPECTED_TIMEOUT);
}

/*
* Standalone/Stateful
*/
@Test
void connectClientUri() {

StatefulRedisConnection<String, String> connection = client.connect();
assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
connection.close();
}

@Test
void connectCodecClientUri() {
StatefulRedisConnection<String, String> connection = client.connect(UTF8);
assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
connection.close();
}

Expand Down Expand Up @@ -133,29 +126,29 @@ void connectcodecSentinelMissingHostAndSocketUri() {
@Test
@Disabled("Non-deterministic behavior. Can cause a deadlock")
void shutdownSyncInRedisFutureTest() {

RedisClient redisClient = RedisClient.create();
StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build());

CompletableFuture<String> f = connection.async().get("key1").whenComplete((result, e) -> {
connection.close();
redisClient.shutdown(0, 0, SECONDS); // deadlock expected.
}).toCompletableFuture();

assertThatThrownBy(() -> TestFutures.awaitOrTimeout(f)).isInstanceOf(TimeoutException.class);
try (final RedisClient redisClient = RedisClient.create();
final StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build())) {
CompletableFuture<String> f = connection.async().get("key1").whenComplete((result, e) -> {
connection.close();
redisClient.shutdown(0, 0, SECONDS); // deadlock expected.
}).toCompletableFuture();

assertThatThrownBy(() -> TestFutures.awaitOrTimeout(f)).isInstanceOf(TimeoutException.class);
}
}

@Test
void shutdownAsyncInRedisFutureTest() {

RedisClient redisClient = RedisClient.create();
StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build());
CompletableFuture<Void> f = connection.async().get("key1").thenCompose(result -> {
connection.close();
return redisClient.shutdownAsync(0, 0, SECONDS);
}).toCompletableFuture();
try (final RedisClient redisClient = RedisClient.create();
final StatefulRedisConnection<String, String> connection = redisClient.connect(redis(host, port).build())) {
CompletableFuture<Void> f = connection.async().get("key1").thenCompose(result -> {
connection.close();
return redisClient.shutdownAsync(0, 0, SECONDS);
}).toCompletableFuture();

TestFutures.awaitOrTimeout(f);
TestFutures.awaitOrTimeout(f);
}
}

/*
Expand All @@ -164,14 +157,14 @@ void shutdownAsyncInRedisFutureTest() {
@Test
void connectPubSubClientUri() {
StatefulRedisPubSubConnection<String, String> connection = client.connectPubSub();
assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
connection.close();
}

@Test
void connectPubSubCodecClientUri() {
StatefulRedisPubSubConnection<String, String> connection = client.connectPubSub(UTF8);
assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
connection.close();
}

Expand Down Expand Up @@ -243,14 +236,14 @@ void connectPubSubAsyncReauthNotSupportedWithRESP2() {
@Test
void connectSentinelClientUri() {
StatefulRedisSentinelConnection<String, String> connection = client.connectSentinel();
assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
connection.close();
}

@Test
void connectSentinelCodecClientUri() {
StatefulRedisSentinelConnection<String, String> connection = client.connectSentinel(UTF8);
assertThat(connection.getTimeout()).isEqualTo(EXPECTED_TIMEOUT);
assertThat(connection.getTimeout()).isEqualTo(RedisURI.DEFAULT_TIMEOUT_DURATION);
connection.close();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;

import io.lettuce.core.RedisClient;
import io.lettuce.core.RedisURI;
import io.lettuce.core.TimeoutOptions;
import io.lettuce.core.protocol.RedisCommand;
import io.lettuce.test.resource.FastShutdown;
import io.lettuce.test.settings.TestSettings;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

import io.lettuce.core.AbstractRedisClientTest;
import io.lettuce.core.ClientOptions;
import io.lettuce.core.RedisChannelWriter;
import io.lettuce.core.RedisCommandTimeoutException;
Expand All @@ -42,26 +46,37 @@

/**
* @author Mark Paluch
* @author Hari Mani
*/
@Tag(INTEGRATION_TEST)
class AtLeastOnceIntegrationTests extends AbstractRedisClientTest {
class AtLeastOnceIntegrationTests {

private String key = "key";
private static final String key = "key";

@BeforeEach
void before() {
client.setOptions(ClientOptions.builder().autoReconnect(true)
.timeoutOptions(TimeoutOptions.builder().timeoutCommands(false).build()).build());
private final RedisClient client;

public AtLeastOnceIntegrationTests() {
// needs to be increased on slow systems...perhaps...
client.setDefaultTimeout(3, TimeUnit.SECONDS);
final RedisURI uri = RedisURI.Builder.redis(TestSettings.host(), TestSettings.port()).withTimeout(Duration.ofSeconds(3))
.build();
this.client = RedisClient.create(uri);
this.client.setOptions(ClientOptions.builder().autoReconnect(true)
.timeoutOptions(TimeoutOptions.builder().timeoutCommands(false).build()).build());
}

@BeforeEach
void before() {
RedisCommands<String, String> connection = client.connect().sync();
connection.flushall();
connection.flushdb();
connection.getStatefulConnection().close();
}

@AfterEach
void tearDown() {
FastShutdown.shutdown(client);
}

@Test
void connectionIsConnectedAfterConnect() {

Expand Down Expand Up @@ -382,9 +397,6 @@ void retryAfterConnectionIsDisconnectedButFiltered() throws Exception {
client.setOptions(ClientOptions.builder().autoReconnect(true).replayFilter(filter)
.timeoutOptions(TimeoutOptions.builder().timeoutCommands(false).build()).build());

// needs to be increased on slow systems...perhaps...
client.setDefaultTimeout(3, TimeUnit.SECONDS);

StatefulRedisConnection<String, String> connection = client.connect();
RedisCommands<String, String> verificationConnection = client.connect().sync();

Expand Down
Loading