sabbey37 commented on a change in pull request #6444:
URL: https://github.com/apache/geode/pull/6444#discussion_r627713888
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractAppendIntegrationTest.java
##########
@@ -25,26 +25,31 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import redis.clients.jedis.Jedis;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.JedisCluster;
import org.apache.geode.redis.ConcurrentLoopingThreads;
import org.apache.geode.redis.RedisIntegrationTest;
+import org.apache.geode.test.awaitility.GeodeAwaitility;
public abstract class AbstractAppendIntegrationTest implements
RedisIntegrationTest {
- private Jedis jedis;
- private Jedis jedis2;
+ private JedisCluster jedis;
+ private JedisCluster jedis2;
+ private static final int REDIS_CLIENT_TIMEOUT =
+ Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
@Before
public void setUp() {
- jedis = new Jedis("localhost", getPort(), 10000000);
- jedis2 = new Jedis("localhost", getPort(), 10000000);
+ jedis = new JedisCluster(new HostAndPort("localhost", getPort()),
REDIS_CLIENT_TIMEOUT);
+ jedis2 = new JedisCluster(new HostAndPort("localhost", getPort()),
REDIS_CLIENT_TIMEOUT);
Review comment:
I don't think we need both `Jedis` instances any longer. Per
@jdeppe-pivotal (from a different PR):
```
JedisCluster is supposed to be thread-safe. It is an abstraction across all
nodes and
handles pools of connections for each node. Thus you should just need a
single instance
of it in the whole test and the same instance can be concurrently used by
different threads.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
