GEODE-9998: Upgrade Jedis client from 3.6.3 to 4.1.1 (apache#7340)

kamilla1201 · Feb 17, 2022 · 9305169 · 9305169
1 parent c51aea6
commit 9305169
Show file tree

Hide file tree

Showing 42 changed files with 254 additions and 257 deletions.
diff --git a/boms/geode-all-bom/src/test/resources/expected-pom.xml b/boms/geode-all-bom/src/test/resources/expected-pom.xml
@@ -460,7 +460,7 @@
       <dependency>
         <groupId>redis.clients</groupId>
         <artifactId>jedis</artifactId>
-        <version>3.6.3</version>
+        <version>4.1.1</version>
       </dependency>
       <dependency>
         <groupId>xerces</groupId>

diff --git a/buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy b/buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy
@@ -172,7 +172,7 @@ class DependencyConstraints implements Plugin<Project> {
         api(group: 'org.springframework.shell', name: 'spring-shell', version: get('springshell.version'))
         api(group: 'org.testcontainers', name: 'testcontainers', version: '1.15.3')
         api(group: 'pl.pragmatists', name: 'JUnitParams', version: '1.1.0')
-        api(group: 'redis.clients', name: 'jedis', version: '3.6.3')
+        api(group: 'redis.clients', name: 'jedis', version: '4.1.1')
         api(group: 'xerces', name: 'xercesImpl', version: '2.12.0')
         api(group: 'xml-apis', name: 'xml-apis', version: '1.4.01')
         api(group: 'org.junit-pioneer', name: 'junit-pioneer', version: '1.5.0')

diff --git a/geode-for-redis/src/commonTest/java/org/apache/geode/redis/mocks/MockSubscriber.java b/geode-for-redis/src/commonTest/java/org/apache/geode/redis/mocks/MockSubscriber.java
@@ -24,9 +24,7 @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
-import redis.clients.jedis.Client;
 import redis.clients.jedis.JedisPubSub;
-import redis.clients.jedis.exceptions.JedisConnectionException;
 
 
 public class MockSubscriber extends JedisPubSub {
@@ -45,8 +43,6 @@ public class MockSubscriber extends JedisPubSub {
       Collections.synchronizedList(new ArrayList<>());
   private CountDownLatch messageReceivedLatch = new CountDownLatch(0);
   private CountDownLatch pMessageReceivedLatch = new CountDownLatch(0);
-  private String localSocketAddress;
-  private Client client;
 
   public MockSubscriber() {
     this(new CountDownLatch(1));
@@ -65,21 +61,14 @@ public MockSubscriber(CountDownLatch subscriptionLatch, CountDownLatch unsubscri
     this.pUnsubscriptionLatch = pUnsubscriptionLatch;
   }
 
-  @Override
-  public void proceed(Client client, String... channels) {
-    localSocketAddress = client.getSocket().getLocalSocketAddress().toString();
-    this.client = client;
-    super.proceed(client, channels);
-  }
-
   private void switchThreadName(String suffix) {
     String threadName = Thread.currentThread().getName();
     int suffixIndex = threadName.indexOf(" -- ");
     if (suffixIndex >= 0) {
       threadName = threadName.substring(0, suffixIndex);
     }
 
-    threadName += " -- " + suffix + " [" + localSocketAddress + "]";
+    threadName += " -- " + suffix;
     Thread.currentThread().setName(threadName);
   }
 
@@ -133,16 +122,6 @@ public void onPong(String pattern) {
     receivedPings.add(pattern);
   }
 
-  // JedisPubSub ping with message is not currently possible, will submit a PR
-  // (https://github.com/xetorthio/jedis/issues/2049)
-  public void ping(String message) {
-    if (client == null) {
-      throw new JedisConnectionException("JedisPubSub is not subscribed to a Jedis instance.");
-    } else {
-      client.ping(message);
-    }
-  }
-
   private static final int AWAIT_TIMEOUT_MILLIS = 30000;
 
   public void awaitSubscribe(String channel) {

diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/UserExpirationDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/UserExpirationDUnitTest.java
@@ -26,7 +26,6 @@
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Test;
-import redis.clients.jedis.BinaryJedisCluster;
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.JedisCluster;
 
@@ -64,7 +63,7 @@ public static void setup() {
     jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, redisPort),
         REDIS_CLIENT_TIMEOUT,
         REDIS_CLIENT_TIMEOUT,
-        BinaryJedisCluster.DEFAULT_MAX_ATTEMPTS,
+        JedisCluster.DEFAULT_MAX_ATTEMPTS,
         USER, USER, "test-client",
         new GenericObjectPoolConfig<>());
   }

diff --git a/...pache/geode/redis/internal/commands/executor/key/AbstractRenameRedirectionsDUnitTest.java b/...pache/geode/redis/internal/commands/executor/key/AbstractRenameRedirectionsDUnitTest.java
@@ -69,7 +69,7 @@ public void renameReturnCrossslotsError_whenSlotsNotOnSameServer() {
 
   private String getKeyOnDifferentServerAs(String antiKey, String prefix) {
     ClusterNodes clusterNodes;
-    try (Jedis j = jedis.getConnectionFromSlot(0)) {
+    try (final Jedis j = new Jedis(jedis.getConnectionFromSlot(0))) {
       clusterNodes = ClusterNodes.parseClusterNodes(j.clusterNodes());
     }
     int antiSlot = JedisClusterCRC16.getCRC16(antiKey) % RegionProvider.REDIS_SLOTS;

diff --git a/...butedTest/java/org/apache/geode/redis/internal/commands/executor/key/RenameDUnitTest.java b/...butedTest/java/org/apache/geode/redis/internal/commands/executor/key/RenameDUnitTest.java
@@ -89,7 +89,7 @@ public static void setup() {
 
     int redisServerPort1 = clusterStartUp.getRedisPort(1);
     jedisCluster =
-        new JedisCluster(new HostAndPort(BIND_ADDRESS, redisServerPort1), REDIS_CLIENT_TIMEOUT);
+        new JedisCluster(new HostAndPort(BIND_ADDRESS, redisServerPort1), REDIS_CLIENT_TIMEOUT, 20);
   }
 
   @Before

diff --git a/.../org/apache/geode/redis/internal/commands/executor/sortedset/ZAddIncrOptionDUnitTest.java b/.../org/apache/geode/redis/internal/commands/executor/sortedset/ZAddIncrOptionDUnitTest.java
@@ -18,7 +18,7 @@
 import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
 import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
 import static org.assertj.core.api.Assertions.assertThat;
-import static redis.clients.jedis.BinaryJedisCluster.DEFAULT_MAX_ATTEMPTS;
+import static redis.clients.jedis.JedisCluster.DEFAULT_MAX_ATTEMPTS;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -33,7 +33,7 @@
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.JedisCluster;
 import redis.clients.jedis.Protocol;
-import redis.clients.jedis.exceptions.JedisClusterMaxAttemptsException;
+import redis.clients.jedis.exceptions.JedisClusterOperationException;
 
 import org.apache.geode.cache.Operation;
 import org.apache.geode.cache.Region;
@@ -156,7 +156,7 @@ private boolean zCardWithRetries(int retries, int maxRetries) {
     long memberSize;
     try {
       memberSize = jedis.zcard(sortedSetKey);
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       if (retries < maxRetries) {
         return false;
       }
@@ -188,7 +188,7 @@ private void doZAddIncrForAllMembersDuringCrash(AtomicBoolean hitJedisClusterIss
     for (int i = 0; i < setSize; i++) {
       try {
         doCrashZAddIncr(i);
-      } catch (JedisClusterMaxAttemptsException ignore) {
+      } catch (JedisClusterOperationException ignore) {
         hitJedisClusterIssue2347.set(true);
       }
     }

diff --git a/...dTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemDUnitTest.java b/...dTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZRemDUnitTest.java
@@ -31,7 +31,7 @@
 import org.junit.Test;
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.JedisCluster;
-import redis.clients.jedis.exceptions.JedisClusterMaxAttemptsException;
+import redis.clients.jedis.exceptions.JedisClusterOperationException;
 
 import org.apache.geode.cache.Operation;
 import org.apache.geode.cache.Region;
@@ -164,7 +164,7 @@ private boolean zRemWithRetries(Map<String, Double> map, int retries, int maxRet
     long removed;
     try {
       removed = jedis.zrem(sortedSetKey, map.keySet().toArray(new String[] {}));
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       if (retries < maxRetries) {
         return false;
       }
@@ -223,7 +223,7 @@ private boolean verifyDataDoesNotExist(Map<String, Double> memberScoreMap) {
         Double score = jedis.zscore(sortedSetKey, member);
         assertThat(score).isNull();
       }
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       return false;
     }
     return true;

diff --git a/.../org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByLexDUnitTest.java b/.../org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByLexDUnitTest.java
@@ -34,7 +34,7 @@
 import org.junit.Test;
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.JedisCluster;
-import redis.clients.jedis.exceptions.JedisClusterMaxAttemptsException;
+import redis.clients.jedis.exceptions.JedisClusterOperationException;
 
 import org.apache.geode.cache.Operation;
 import org.apache.geode.cache.Region;
@@ -225,7 +225,7 @@ private boolean zRemRangeByLexWithRetries(Map<String, Double> map, int retries,
     long removed;
     try {
       removed = jedis.zremrangeByLex(KEY, "[v", "[w");
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       if (retries < maxRetries) {
         return false;
       }
@@ -269,7 +269,7 @@ private boolean verifyDataDoesNotExist(Map<String, Double> memberScoreMap) {
         Double score = jedis.zscore(KEY, member);
         assertThat(score).isNull();
       }
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       return false;
     }
     return true;

diff --git a/...org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByRankDUnitTest.java b/...org/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByRankDUnitTest.java
@@ -14,6 +14,7 @@
  */
 package org.apache.geode.redis.internal.commands.executor.sortedset;
 
+import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT;
 import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
 import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
 import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
@@ -33,9 +34,10 @@
 import org.junit.Test;
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.JedisCluster;
-import redis.clients.jedis.exceptions.JedisClusterMaxAttemptsException;
+import redis.clients.jedis.exceptions.JedisClusterOperationException;
 
 import org.apache.geode.cache.Operation;
+import org.apache.geode.internal.AvailablePortHelper;
 import org.apache.geode.internal.cache.PartitionedRegion;
 import org.apache.geode.internal.cache.PartitionedRegionHelper;
 import org.apache.geode.redis.ConcurrentLoopingThreads;
@@ -64,9 +66,19 @@ public class ZRemRangeByRankDUnitTest {
   public void setup() {
     MemberVM locator = clusterStartUp.startLocatorVM(0);
     int locatorPort = locator.getPort();
-    MemberVM server1 = clusterStartUp.startRedisVM(1, locatorPort);
-    MemberVM server2 = clusterStartUp.startRedisVM(2, locatorPort);
-    MemberVM server3 = clusterStartUp.startRedisVM(3, locatorPort);
+
+    int[] serverPorts = AvailablePortHelper.getRandomAvailableTCPPorts(3);
+
+    MemberVM server1 = clusterStartUp.startRedisVM(1, x -> x
+        .withProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[0]))
+        .withConnectionToLocator(locatorPort));
+    MemberVM server2 = clusterStartUp.startRedisVM(2, x -> x
+        .withProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[1]))
+        .withConnectionToLocator(locatorPort));
+    MemberVM server3 = clusterStartUp.startRedisVM(3, x -> x
+        .withProperty(GEODE_FOR_REDIS_PORT, Integer.toString(serverPorts[2]))
+        .withConnectionToLocator(locatorPort));
+
     servers = new ArrayList<>();
     servers.add(server1);
     servers.add(server2);
@@ -162,7 +174,7 @@ private boolean verifyDataDoesNotExist(Map<String, Double> memberScoreMap) {
         Double score = jedis.zscore(KEY, member);
         assertThat(score).isNull();
       }
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       return false;
     }
     return true;
@@ -191,7 +203,7 @@ private boolean zRemRangeByRankWithRetries(Map<String, Double> map, int retries,
     long removed;
     try {
       removed = jedis.zremrangeByRank(KEY, 0, -1);
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       if (retries < maxRetries) {
         return false;
       }

diff --git a/...rg/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByScoreDUnitTest.java b/...rg/apache/geode/redis/internal/commands/executor/sortedset/ZRemRangeByScoreDUnitTest.java
@@ -36,7 +36,7 @@
 import org.junit.Test;
 import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.JedisCluster;
-import redis.clients.jedis.exceptions.JedisClusterMaxAttemptsException;
+import redis.clients.jedis.exceptions.JedisClusterOperationException;
 
 import org.apache.geode.cache.Operation;
 import org.apache.geode.internal.AvailablePortHelper;
@@ -197,7 +197,7 @@ private boolean verifyDataDoesNotExist(Map<String, Double> memberScoreMap) {
         Double score = jedis.zscore(KEY, member);
         assertThat(score).isNull();
       }
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       return false;
     }
     return true;
@@ -244,7 +244,7 @@ private boolean zRemRangeByScoreWithRetries(Map<String, Double> map, int retries
     long removed;
     try {
       removed = jedis.zremrangeByScore(KEY, "-inf", "+inf");
-    } catch (JedisClusterMaxAttemptsException e) {
+    } catch (JedisClusterOperationException e) {
       if (retries < maxRetries) {
         return false;
       }

diff --git a/.../java/org/apache/geode/redis/internal/executor/auth/AuthWhileServersRestartDUnitTest.java b/.../java/org/apache/geode/redis/internal/executor/auth/AuthWhileServersRestartDUnitTest.java
@@ -18,7 +18,7 @@
 import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT;
 import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADDRESS;
 import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
-import static redis.clients.jedis.BinaryJedisCluster.DEFAULT_MAX_ATTEMPTS;
+import static redis.clients.jedis.JedisCluster.DEFAULT_MAX_ATTEMPTS;
 
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -59,7 +59,7 @@ public class AuthWhileServersRestartDUnitTest {
   public static void classSetup() {
     MemberVM locator = clusterStartUp.startLocatorVM(0,
         x -> x.withSecurityManager(SimpleSecurityManager.class));
-    int locatorPort = locator.getPort();
+    final int locatorPort = locator.getPort();
 
     SerializableFunction<ServerStarterRule> serverOperator = s -> s
         .withCredential("cluster", "cluster")
@@ -72,7 +72,8 @@ public static void classSetup() {
     String finalRedisPort = Integer.toString(server3Port);
 
     operatorForVM3 = serverOperator.compose(o -> o
-        .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort));
+        .withProperty(GEODE_FOR_REDIS_PORT, finalRedisPort)
+        .withConnectionToLocator(locatorPort));
 
     clusterStartUp.startRedisVM(3, operatorForVM3);
 

diff --git a/.../org/apache/geode/redis/internal/executor/string/StringsKillMultipleServersDUnitTest.java b/.../org/apache/geode/redis/internal/executor/string/StringsKillMultipleServersDUnitTest.java
@@ -56,7 +56,7 @@ public static void classSetup() {
 
     int redisServerPort1 = cluster.getRedisPort(1);
     jedisCluster =
-        new JedisCluster(new HostAndPort(BIND_ADDRESS, redisServerPort1), 10_000);
+        new JedisCluster(new HostAndPort(BIND_ADDRESS, redisServerPort1), 10_000, 20);
 
     // This sequence ensures that servers 1, 2 and 3 are hosting all the buckets and server 4
     // has no buckets.
@@ -99,7 +99,12 @@ public void operationsShouldContinueAfterMultipleServerFailures() throws Excepti
   private Void doSetOps(AtomicBoolean running, AtomicInteger counter) {
     while (running.get()) {
       int i = counter.getAndIncrement();
-      jedisCluster.set("key-" + i, "value-" + i);
+      try {
+        jedisCluster.set("key-" + i, "value-" + i);
+      } catch (final Throwable t) {
+        t.printStackTrace();
+        throw t;
+      }
     }
     return null;
   }

diff --git a/...integrationTest/java/org/apache/geode/redis/AbstractCommandPipeliningIntegrationTest.java b/...integrationTest/java/org/apache/geode/redis/AbstractCommandPipeliningIntegrationTest.java
@@ -92,12 +92,16 @@ public void should_returnResultsOfPipelinedCommands_inCorrectOrder() {
     int numberOfPipeLineRequests = 1000;
 
     do {
-      Pipeline p = jedis.pipelined();
+      // use a unique key for each pipeline for incrementing
+      final String key = "P" + numberOfPipeLineRequests;
+      jedis.set(key, "-1");
+
+      final Pipeline p = jedis.pipelined();
       for (int i = 0; i < NUMBER_OF_COMMANDS_IN_PIPELINE; i++) {
-        p.echo(String.valueOf(i));
+        p.incr(key);
       }
 
-      List<Object> results = p.syncAndReturnAll();
+      final List<Object> results = p.syncAndReturnAll();
 
       verifyResultOrder(NUMBER_OF_COMMANDS_IN_PIPELINE, results);
       numberOfPipeLineRequests--;
@@ -109,8 +113,8 @@ public void should_returnResultsOfPipelinedCommands_inCorrectOrder() {
 
   private void verifyResultOrder(final int numberOfCommandInPipeline, List<Object> results) {
     for (int i = 0; i < numberOfCommandInPipeline; i++) {
-      String expected = String.valueOf(i);
-      String currentVal = (String) results.get(i);
+      final long expected = (long) i;
+      final long currentVal = (long) results.get(i);
 
       assertThat(currentVal).isEqualTo(expected);
     }

diff --git a/...apache/geode/redis/internal/commands/executor/cluster/AbstractClusterIntegrationTest.java b/...apache/geode/redis/internal/commands/executor/cluster/AbstractClusterIntegrationTest.java
@@ -49,7 +49,7 @@ public void teardown() {
 
   @Test
   public void testCluster_givenWrongNumberOfArguments() {
-    final Jedis connection = jedis.getConnectionFromSlot(0);
+    final Jedis connection = new Jedis(jedis.getConnectionFromSlot(0));
     assertThatThrownBy(() -> connection.sendCommand(Protocol.Command.CLUSTER))
         .hasMessage("ERR wrong number of arguments for 'cluster' command");
     assertThatThrownBy(
@@ -77,7 +77,7 @@ public void testCluster_givenWrongNumberOfArguments() {
 
   @Test
   public void keyslot_ReturnsCorrectSlot() {
-    final Jedis connection = jedis.getConnectionFromSlot(0);
+    final Jedis connection = new Jedis(jedis.getConnectionFromSlot(0));
     assertThat(connection.clusterKeySlot("nohash")).isEqualTo(9072);
     assertThat(connection.clusterKeySlot("with{hash}")).isEqualTo(238);
     assertThat(connection.clusterKeySlot("with{two}{hashes}")).isEqualTo(2127);

diff --git a/...ava/org/apache/geode/redis/internal/commands/executor/cluster/ClusterIntegrationTest.java b/...ava/org/apache/geode/redis/internal/commands/executor/cluster/ClusterIntegrationTest.java
@@ -36,7 +36,7 @@ public int getPort() {
 
   @Test
   public void errorMessageContainsListOfSupportedSubcommands() {
-    final Jedis connection = jedis.getConnectionFromSlot(0);
+    final Jedis connection = new Jedis(jedis.getConnectionFromSlot(0));
 
     String invalidSubcommand = "subcommand";
     assertThatThrownBy(() -> connection.sendCommand(Protocol.Command.CLUSTER, invalidSubcommand))