From fd31e433c090c1256c47f07ea19337287ec5336c Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 11 Sep 2024 11:59:30 +0200 Subject: [PATCH] Polishing. Move maxTime calculation for topology refresh to and delegate shouldUseCachedValue() to newly introduced overload. Original Pull Request: #2989 --- .../jedis/JedisClusterConnection.java | 29 ++++++++++++++----- .../jedis/JedisClusterConnectionTests.java | 4 +-- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java index 625c08572..b4b2d8422 100644 --- a/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java +++ b/src/main/java/org/springframework/data/redis/connection/jedis/JedisClusterConnection.java @@ -854,9 +854,8 @@ public class JedisClusterConnection implements RedisClusterConnection { try (Connection connection = entry.getValue().getResource()) { - Set nodes = Converters.toSetOfRedisClusterNodes(new Jedis(connection).clusterNodes()); - topology = cached = new JedisClusterTopology(nodes, System.currentTimeMillis()); + topology = cached = new JedisClusterTopology(nodes, System.currentTimeMillis(), cacheTimeMs); return topology; } catch (Exception ex) { @@ -884,9 +883,9 @@ public class JedisClusterConnection implements RedisClusterConnection { * @since 2.2 * @deprecated since 3.3.4, use {@link #shouldUseCachedValue(JedisClusterTopology)} instead. */ - @Deprecated(since = "3.3.4") + @Deprecated(since = "3.3.4", forRemoval = true) protected boolean shouldUseCachedValue() { - return false; + return shouldUseCachedValue(cached); } /** @@ -899,22 +898,38 @@ public class JedisClusterConnection implements RedisClusterConnection { * @since 3.3.4 */ protected boolean shouldUseCachedValue(@Nullable JedisClusterTopology topology) { - return topology != null && topology.getTime() + cacheTimeMs > System.currentTimeMillis(); + return topology != null && topology.getMaxTime() > System.currentTimeMillis(); } } protected static class JedisClusterTopology extends ClusterTopology { private final long time; + private final long timeoutMs; - public JedisClusterTopology(Set nodes, long time) { + JedisClusterTopology(Set nodes, long creationTimeMs, long timeoutMs) { super(nodes); - this.time = time; + this.time = creationTimeMs; + this.timeoutMs = timeoutMs; } + /** + * Get the time in ms when the {@link ClusterTopology} was captured. + * + * @return ClusterTopology time. + */ public long getTime() { return time; } + + /** + * Get the maximum time in ms the {@link ClusterTopology} should be used before a refresh is required. + * + * @return ClusterTopology maximum age. + */ + long getMaxTime() { + return time + timeoutMs; + } } protected JedisCluster getCluster() { diff --git a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java index c56332f5f..8ec56d8b3 100644 --- a/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java +++ b/src/test/java/org/springframework/data/redis/connection/jedis/JedisClusterConnectionTests.java @@ -2965,8 +2965,8 @@ public class JedisClusterConnectionTests implements ClusterConnectionTests { assertThat(topology).isInstanceOf(JedisClusterConnection.JedisClusterTopology.class); assertThat(provider.shouldUseCachedValue(null)).isFalse(); - assertThat(provider.shouldUseCachedValue(new JedisClusterConnection.JedisClusterTopology(Set.of(), 0))).isFalse(); + assertThat(provider.shouldUseCachedValue(new JedisClusterConnection.JedisClusterTopology(Set.of(), System.currentTimeMillis() - 101, 100))).isFalse(); assertThat(provider.shouldUseCachedValue( - new JedisClusterConnection.JedisClusterTopology(Set.of(), System.currentTimeMillis() + 100))).isTrue(); + new JedisClusterConnection.JedisClusterTopology(Set.of(), System.currentTimeMillis() + 100, 100))).isTrue(); } }