From b7d7f84bceff68da0609483de7b2e731b65085a9 Mon Sep 17 00:00:00 2001 From: Kengo Seki Date: Wed, 8 Nov 2023 23:52:41 +0900 Subject: [PATCH] Bump Jedis version to 5.0.2 (#15344) Currently, the redis-cache extension uses Jedis 2.9.0, which was released over seven years ago and is no longer listed in the official support matrix. This patch upgrades it to ensure the compatibility with the recent version of Redis and make future upgrades easier, including: Upgrade Jedis to v5.0.2, the latest version at this writing, and address the API changes and dependency version mismatch. Replace mock-jedis with jedis-mock, since the former has not been actively maintained any longer and not compatible with recent versions of Jedis. --- extensions-contrib/redis-cache/pom.xml | 39 +++++++++-- .../druid/client/cache/RedisCacheFactory.java | 3 +- .../druid/client/cache/RedisClusterCache.java | 9 +-- .../client/cache/RedisCacheConfigTest.java | 14 +++- .../client/cache/RedisClusterCacheTest.java | 69 +++++-------------- .../cache/RedisStandaloneCacheTest.java | 35 ++++------ 6 files changed, 82 insertions(+), 87 deletions(-) diff --git a/extensions-contrib/redis-cache/pom.xml b/extensions-contrib/redis-cache/pom.xml index 0cd80a1bc28..db7f33525e2 100644 --- a/extensions-contrib/redis-cache/pom.xml +++ b/extensions-contrib/redis-cache/pom.xml @@ -49,7 +49,7 @@ redis.clients jedis - 2.9.0 + 5.0.2 com.fasterxml.jackson.core @@ -104,9 +104,9 @@ test - com.fiftyonred - mock-jedis - 0.4.0 + com.github.fppt + jedis-mock + 1.0.11 test @@ -126,6 +126,37 @@ true + + org.apache.maven.plugins + maven-shade-plugin + + + redis-extension + package + + shade + + + + + org.apache.commons:commons-pool2 + redis.clients:jedis + + + + + org.apache.commons.pool2 + org.apache.druid.redis.shaded.org.apache.commons.pool2 + + + redis.clients.jedis + org.apache.druid.redis.shaded.redis.clients.jedis + + + + + + diff --git a/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisCacheFactory.java b/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisCacheFactory.java index b69bfc2c15a..3f15f115681 100644 --- a/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisCacheFactory.java +++ b/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisCacheFactory.java @@ -21,6 +21,7 @@ package org.apache.druid.client.cache; import org.apache.commons.lang.StringUtils; import org.apache.druid.java.util.common.IAE; +import redis.clients.jedis.ConnectionPoolConfig; import redis.clients.jedis.HostAndPort; import redis.clients.jedis.JedisCluster; import redis.clients.jedis.JedisPool; @@ -59,7 +60,7 @@ public class RedisCacheFactory return new HostAndPort(hostAndPort.substring(0, index), port); }).collect(Collectors.toSet()); - JedisPoolConfig poolConfig = new JedisPoolConfig(); + ConnectionPoolConfig poolConfig = new ConnectionPoolConfig(); poolConfig.setMaxTotal(config.getMaxTotalConnections()); poolConfig.setMaxIdle(config.getMaxIdleConnections()); poolConfig.setMinIdle(config.getMinIdleConnections()); diff --git a/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisClusterCache.java b/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisClusterCache.java index 35827babc76..a42f17b0db7 100644 --- a/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisClusterCache.java +++ b/extensions-contrib/redis-cache/src/main/java/org/apache/druid/client/cache/RedisClusterCache.java @@ -21,9 +21,8 @@ package org.apache.druid.client.cache; import org.apache.druid.java.util.common.Pair; import redis.clients.jedis.JedisCluster; -import redis.clients.util.JedisClusterCRC16; +import redis.clients.jedis.util.JedisClusterCRC16; -import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -110,10 +109,6 @@ public class RedisClusterCache extends AbstractRedisCache @Override protected void cleanup() { - try { - cluster.close(); - } - catch (IOException ignored) { - } + cluster.close(); } } diff --git a/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisCacheConfigTest.java b/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisCacheConfigTest.java index 941ff3376de..d1860636348 100644 --- a/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisCacheConfigTest.java +++ b/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisCacheConfigTest.java @@ -21,6 +21,8 @@ package org.apache.druid.client.cache; import com.fasterxml.jackson.databind.ObjectMapper; +import com.github.fppt.jedismock.RedisServer; +import com.github.fppt.jedismock.server.ServiceOptions; import org.apache.druid.java.util.common.IAE; import org.hamcrest.Description; import org.hamcrest.Matcher; @@ -114,18 +116,24 @@ public class RedisCacheConfigTest @Test public void testClusterPriority() throws IOException { + ServiceOptions options = ServiceOptions.defaultOptions().withClusterModeEnabled(); + RedisServer server = RedisServer.newRedisServer().setOptions(options).start(); + ObjectMapper mapper = new ObjectMapper(); RedisCacheConfig fromJson = mapper.readValue("{\"expiration\": 1000," + "\"cluster\": {" - + "\"nodes\": \"127.0.0.1:6379\"" + + "\"nodes\": \"" + server.getHost() + ":" + server.getBindPort() + "\"" + "}," - + "\"host\": \"127.0.0.1\"," - + "\"port\": 6379" + + "\"host\": \"" + server.getHost() + "\"," + + "\"port\": " + server.getBindPort() + "}", RedisCacheConfig.class); try (Cache cache = RedisCacheFactory.create(fromJson)) { Assert.assertTrue(cache instanceof RedisClusterCache); } + finally { + server.stop(); + } } @Test diff --git a/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisClusterCacheTest.java b/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisClusterCacheTest.java index c8a24b16f55..3a7f9bb04ab 100644 --- a/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisClusterCacheTest.java +++ b/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisClusterCacheTest.java @@ -21,22 +21,20 @@ package org.apache.druid.client.cache; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fiftyonred.mock_jedis.MockJedisCluster; +import com.github.fppt.jedismock.RedisServer; +import com.github.fppt.jedismock.server.ServiceOptions; import com.google.common.collect.Lists; import com.google.common.primitives.Ints; import org.apache.druid.java.util.common.StringUtils; +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import redis.clients.jedis.HostAndPort; -import redis.clients.jedis.JedisPoolConfig; +import redis.clients.jedis.JedisCluster; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; +import java.io.IOException; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicLong; public class RedisClusterCacheTest { @@ -58,51 +56,24 @@ public class RedisClusterCacheTest } }; + private RedisServer server; private RedisClusterCache cache; - private AtomicLong mgetCount = new AtomicLong(); @Before - public void setUp() + public void setUp() throws IOException { - JedisPoolConfig poolConfig = new JedisPoolConfig(); - poolConfig.setMaxTotal(cacheConfig.getMaxTotalConnections()); - poolConfig.setMaxIdle(cacheConfig.getMaxIdleConnections()); - poolConfig.setMinIdle(cacheConfig.getMinIdleConnections()); - - // orginal MockJedisCluster does not provide full support for all public get/set interfaces - // some methods must be overriden for test cases - cache = new RedisClusterCache(new MockJedisCluster(Collections.singleton(new HostAndPort("localhost", 6379))) - { - final ConcurrentHashMap cacheStorage = new ConcurrentHashMap<>(); - - @Override - public String setex(final byte[] key, final int seconds, final byte[] value) - { - cacheStorage.put(StringUtils.encodeBase64String(key), value); - return null; - } - - @Override - public byte[] get(final byte[] key) - { - return cacheStorage.get(StringUtils.encodeBase64String(key)); - } - - @Override - public List mget(final byte[]... keys) - { - mgetCount.incrementAndGet(); - List ret = new ArrayList<>(); - for (byte[] key : keys) { - String k = StringUtils.encodeBase64String(key); - byte[] value = cacheStorage.get(k); - ret.add(value); - } - return ret; - } - }, cacheConfig); + ServiceOptions options = ServiceOptions.defaultOptions().withClusterModeEnabled(); + server = RedisServer.newRedisServer().setOptions(options).start(); + HostAndPort hostAndPort = new HostAndPort(server.getHost(), server.getBindPort()); + JedisCluster cluster = new JedisCluster(hostAndPort); + cache = new RedisClusterCache(cluster, cacheConfig); } + @After + public void tearDown() throws IOException + { + server.stop(); + } @Test public void testConfig() throws JsonProcessingException @@ -135,8 +106,6 @@ public class RedisClusterCacheTest Assert.assertEquals(0x03040506, Ints.fromByteArray(cache.get(key3))); Assert.assertNull(cache.get(notExist)); - this.mgetCount.set(0); - //test multi get Map result = cache.getBulk( Lists.newArrayList( @@ -147,9 +116,7 @@ public class RedisClusterCacheTest ) ); - // these 4 keys are distributed among different nodes, so there should be 4 times call of MGET - Assert.assertEquals(mgetCount.get(), 4); - Assert.assertEquals(result.size(), 3); + Assert.assertEquals(3, result.size()); Assert.assertEquals(0x01020304, Ints.fromByteArray(result.get(key1))); Assert.assertEquals(0x02030405, Ints.fromByteArray(result.get(key2))); Assert.assertEquals(0x03040506, Ints.fromByteArray(result.get(key3))); diff --git a/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisStandaloneCacheTest.java b/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisStandaloneCacheTest.java index 7ff1ad32839..d980dfa5f6d 100644 --- a/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisStandaloneCacheTest.java +++ b/extensions-contrib/redis-cache/src/test/java/org/apache/druid/client/cache/RedisStandaloneCacheTest.java @@ -20,8 +20,7 @@ package org.apache.druid.client.cache; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fiftyonred.mock_jedis.MockJedis; -import com.fiftyonred.mock_jedis.MockJedisPool; +import com.github.fppt.jedismock.RedisServer; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.common.primitives.Ints; @@ -34,11 +33,13 @@ import org.apache.druid.guice.ManageLifecycle; import org.apache.druid.initialization.Initialization; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.lifecycle.Lifecycle; +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import redis.clients.jedis.JedisPoolConfig; +import redis.clients.jedis.JedisPool; +import java.io.IOException; import java.util.Map; import java.util.UUID; @@ -47,6 +48,7 @@ public class RedisStandaloneCacheTest private static final byte[] HI = StringUtils.toUtf8("hiiiiiiiiiiiiiiiiiii"); private static final byte[] HO = StringUtils.toUtf8("hooooooooooooooooooo"); + private RedisServer server; private RedisStandaloneCache cache; private final RedisCacheConfig cacheConfig = new RedisCacheConfig() { @@ -64,28 +66,19 @@ public class RedisStandaloneCacheTest }; @Before - public void setUp() + public void setUp() throws IOException { - JedisPoolConfig poolConfig = new JedisPoolConfig(); - poolConfig.setMaxTotal(cacheConfig.getMaxTotalConnections()); - poolConfig.setMaxIdle(cacheConfig.getMaxIdleConnections()); - poolConfig.setMinIdle(cacheConfig.getMinIdleConnections()); - - MockJedisPool pool = new MockJedisPool(poolConfig, "localhost"); - // orginal MockJedis do not support 'milliseconds' in long type, - // for test we override to support it - pool.setClient(new MockJedis("localhost") - { - @Override - public String psetex(byte[] key, long milliseconds, byte[] value) - { - return this.psetex(key, (int) milliseconds, value); - } - }); - + server = RedisServer.newRedisServer().start(); + JedisPool pool = new JedisPool(server.getHost(), server.getBindPort()); cache = new RedisStandaloneCache(pool, cacheConfig); } + @After + public void tearDown() throws IOException + { + server.stop(); + } + @Test public void testBasicInjection() throws Exception {