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

@@ -49,7 +49,7 @@
         <dependency>
             <groupId>redis.clients</groupId>
             <artifactId>jedis</artifactId>
-            <version>2.9.0</version>
+            <version>5.0.2</version>
         </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
@@ -104,9 +104,9 @@
             <scope>test</scope>
         </dependency>
         <dependency>
-            <groupId>com.fiftyonred</groupId>
+            <groupId>com.github.fppt</groupId>
-            <artifactId>mock-jedis</artifactId>
+            <artifactId>jedis-mock</artifactId>
-            <version>0.4.0</version>
+            <version>1.0.11</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -126,6 +126,37 @@
                     <skip>true</skip>
                 </configuration>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-shade-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>redis-extension</id>
+                        <phase>package</phase>
+                        <goals>
+                            <goal>shade</goal>
+                        </goals>
+                        <configuration>
+                            <artifactSet>
+                                <includes>
+                                    <include>org.apache.commons:commons-pool2</include>
+                                    <include>redis.clients:jedis</include>
+                                </includes>
+                            </artifactSet>
+                            <relocations>
+                                <relocation>
+                                    <pattern>org.apache.commons.pool2</pattern>
+                                    <shadedPattern>org.apache.druid.redis.shaded.org.apache.commons.pool2</shadedPattern>
+                                </relocation>
+                                <relocation>
+                                    <pattern>redis.clients.jedis</pattern>
+                                    <shadedPattern>org.apache.druid.redis.shaded.redis.clients.jedis</shadedPattern>
+                                </relocation>
+                            </relocations>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
         </plugins>
     </build>
 </project>

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());

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) {
-    }
-  }
 }

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\","
+                                                 + "\"host\": \"" + server.getHost() + "\","
-                                                 + "\"port\": 6379"
+                                                 + "\"port\": " + server.getBindPort()
                                                  + "}", RedisCacheConfig.class);
 
     try (Cache cache = RedisCacheFactory.create(fromJson)) {
       Assert.assertTrue(cache instanceof RedisClusterCache);
     }
+    finally {
+      server.stop();
+    }
   }
 
   @Test

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.io.IOException;
-import java.util.Collections;
-import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.atomic.AtomicLong;
 
 public class RedisClusterCacheTest
 {
@@ -58,52 +56,25 @@ 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();
+    ServiceOptions options = ServiceOptions.defaultOptions().withClusterModeEnabled();
-    poolConfig.setMaxTotal(cacheConfig.getMaxTotalConnections());
+    server = RedisServer.newRedisServer().setOptions(options).start();
-    poolConfig.setMaxIdle(cacheConfig.getMaxIdleConnections());
+    HostAndPort hostAndPort = new HostAndPort(server.getHost(), server.getBindPort());
-    poolConfig.setMinIdle(cacheConfig.getMinIdleConnections());
+    JedisCluster cluster = new JedisCluster(hostAndPort);
-
+    cache = new RedisClusterCache(cluster, cacheConfig);
-    // 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<String, byte[]> 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
+  @After
-      public byte[] get(final byte[] key)
+  public void tearDown() throws IOException
   {
-        return cacheStorage.get(StringUtils.encodeBase64String(key));
+    server.stop();
   }
 
-      @Override
-      public List<byte[]> mget(final byte[]... keys)
-      {
-        mgetCount.incrementAndGet();
-        List<byte[]> ret = new ArrayList<>();
-        for (byte[] key : keys) {
-          String k = StringUtils.encodeBase64String(key);
-          byte[] value = cacheStorage.get(k);
-          ret.add(value);
-        }
-        return ret;
-      }
-    }, cacheConfig);
-  }
-
-
   @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<Cache.NamedKey, byte[]> 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(3, result.size());
-    Assert.assertEquals(mgetCount.get(), 4);
-    Assert.assertEquals(result.size(), 3);
     Assert.assertEquals(0x01020304, Ints.fromByteArray(result.get(key1)));
     Assert.assertEquals(0x02030405, Ints.fromByteArray(result.get(key2)));
     Assert.assertEquals(0x03040506, Ints.fromByteArray(result.get(key3)));

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.github.fppt.jedismock.RedisServer;
-import com.fiftyonred.mock_jedis.MockJedisPool;
 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();
+    server = RedisServer.newRedisServer().start();
-    poolConfig.setMaxTotal(cacheConfig.getMaxTotalConnections());
+    JedisPool pool = new JedisPool(server.getHost(), server.getBindPort());
-    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);
-      }
-    });
-
     cache = new RedisStandaloneCache(pool, cacheConfig);
   }
 
+  @After
+  public void tearDown() throws IOException
+  {
+    server.stop();
+  }
+
   @Test
   public void testBasicInjection() throws Exception
   {