You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/05/04 13:45:12 UTC
[GitHub] [geode] jdeppe-pivotal commented on a change in pull request #6429: GEODE-9220: Switch set DUnit tests to use JedisCluster
jdeppe-pivotal commented on a change in pull request #6429:
URL: https://github.com/apache/geode/pull/6429#discussion_r625776087
##########
File path: geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/set/SremDUnitTest.java
##########
@@ -125,89 +121,143 @@ public void shouldDistributeDataAmongMultipleServers_givenMultipleClients_removi
allMembers.addAll(members1);
allMembers.addAll(members2);
- jedis3.sadd(key, allMembers.toArray(new String[] {}));
+ jedis.sadd(key, allMembers.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key, members1.get(i)),
- (i) -> jedis2.srem(key, members2.get(i))).run();
+ (i) -> jedis.srem(key, members1.get(i)),
+ (i) -> jedis.srem(key, members2.get(i))).run();
- Set<String> results = jedis3.smembers(key);
+ Set<String> results = jedis.smembers(key);
assertThat(results).isEmpty();
}
+
@Test
- public void shouldDistributeDataAmongMultipleServers_givenMultipleClients_removingSameDataFromSameSetConcurrently() {
+ public void shouldDistributeDataAmongCluster_thenRemoveSameDataFromSameSetConcurrently() {
String key = "key";
List<String> members = makeMemberList(SET_SIZE, "member-");
- jedis3.sadd(key, members.toArray(new String[] {}));
+ jedis.sadd(key, members.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key, members.get(i)),
- (i) -> jedis2.srem(key, members.get(i))).run();
+ (i) -> jedis.srem(key, members.get(i)),
+ (i) -> jedis.srem(key, members.get(i))).run();
- Set<String> results = jedis3.smembers(key);
+ Set<String> results = jedis.smembers(key);
assertThat(results).isEmpty();
}
+
@Test
- public void shouldDistributeDataAmongMultipleServers_givenMultipleClients_removingFromDifferentSetsConcurrently() {
+ public void shouldDistributeDataAmongCluster_thenRemoveFromDifferentSetsConcurrently() {
String key1 = "key1";
String key2 = "key2";
List<String> members1 = makeMemberList(SET_SIZE, "member1-");
List<String> members2 = makeMemberList(SET_SIZE, "member2-");
- jedis3.sadd(key1, members1.toArray(new String[] {}));
- jedis3.sadd(key2, members2.toArray(new String[] {}));
+ jedis.sadd(key1, members1.toArray(new String[] {}));
+ jedis.sadd(key2, members2.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key1, members1.get(i)),
- (i) -> jedis2.srem(key2, members2.get(i))).run();
+ (i) -> jedis.srem(key1, members1.get(i)),
+ (i) -> jedis.srem(key2, members2.get(i))).run();
- Set<String> results1 = jedis3.smembers(key1);
- Set<String> results2 = jedis3.smembers(key2);
+ Set<String> results1 = jedis.smembers(key1);
+ Set<String> results2 = jedis.smembers(key2);
assertThat(results1).isEmpty();
assertThat(results2).isEmpty();
}
+
+ @Test
+ public void shouldDistributeDataAmongCluster_givenMultipleClientsOnDifferentServer_removingSameDataFromSameSetConcurrently() {
+
+ int redisServerPort2 = clusterStartUp.getRedisPort(2);
+ JedisCluster jedis2 =
+ new JedisCluster(new HostAndPort(LOCAL_HOST, redisServerPort), redisServerPort2);
+
+ String key = "key";
+
+ List<String> members = makeMemberList(SET_SIZE, "member1-");
+ jedis.sadd(key, members.toArray(new String[] {}));
+
+ new ConcurrentLoopingThreads(SET_SIZE,
+ (i) -> jedis.srem(key, members.get(i)),
+ (i) -> jedis2.srem(key, members.get(i))).run();
+
+ Set<String> results = jedis.smembers(key);
+
+ assertThat(results).isEmpty();
+
+ jedis2.close();
+ }
Review comment:
This is the same test as `shouldDistributeDataAmongCluster_thenRemoveSameDataFromSameSetConcurrently`. There is no difference in using a different `JedisCluster` instance inside the concurrent loop. Even with the same `jedis` instance, each thread will create a separate connection.
##########
File path: geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/set/SaddDUnitTest.java
##########
@@ -208,17 +204,14 @@ public void shouldDistributeDataAmongMultipleServers_givenTwoSetsOfClients_Opera
allMembers.addAll(members2);
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.sadd(key, members1.get(i)),
- (i) -> jedis1B.sadd(key, members1.get(i)),
- (i) -> jedis2.sadd(key, members2.get(i)),
- (i) -> jedis2B.sadd(key, members2.get(i))).run();
+ (i) -> jedis.sadd(key, members1.get(i)),
+ (i) -> jedis2.sadd(key, members2.get(i))).runInLockstep();
Review comment:
You can just use the single `jedis` field here without creating `jedis2`
##########
File path: geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/set/SremDUnitTest.java
##########
@@ -218,20 +268,17 @@ public void shouldDistributeDataAmongMultipleServers_givenMultipleClientsOnSameS
allMembers.addAll(members1);
allMembers.addAll(members2);
- jedis3.sadd(key, allMembers.toArray(new String[] {}));
+ jedis.sadd(key, allMembers.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key, members1.get(i)),
- (i) -> jedis1B.srem(key, members1.get(i)),
- (i) -> jedis2.srem(key, members2.get(i)),
- (i) -> jedis2B.srem(key, members2.get(i))).run();
+ (i) -> jedis.srem(key, members1.get(i)),
+ (i) -> jedis2.srem(key, members2.get(i))).run();
Review comment:
This is the same test as `shouldDistributeDataAmongCluster_givenMultipleClientsOnDifferentServer_removingDifferentDataFromSameSetConcurrently`
##########
File path: geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/set/SremDUnitTest.java
##########
@@ -125,89 +121,143 @@ public void shouldDistributeDataAmongMultipleServers_givenMultipleClients_removi
allMembers.addAll(members1);
allMembers.addAll(members2);
- jedis3.sadd(key, allMembers.toArray(new String[] {}));
+ jedis.sadd(key, allMembers.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key, members1.get(i)),
- (i) -> jedis2.srem(key, members2.get(i))).run();
+ (i) -> jedis.srem(key, members1.get(i)),
+ (i) -> jedis.srem(key, members2.get(i))).run();
- Set<String> results = jedis3.smembers(key);
+ Set<String> results = jedis.smembers(key);
assertThat(results).isEmpty();
}
+
@Test
- public void shouldDistributeDataAmongMultipleServers_givenMultipleClients_removingSameDataFromSameSetConcurrently() {
+ public void shouldDistributeDataAmongCluster_thenRemoveSameDataFromSameSetConcurrently() {
String key = "key";
List<String> members = makeMemberList(SET_SIZE, "member-");
- jedis3.sadd(key, members.toArray(new String[] {}));
+ jedis.sadd(key, members.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key, members.get(i)),
- (i) -> jedis2.srem(key, members.get(i))).run();
+ (i) -> jedis.srem(key, members.get(i)),
+ (i) -> jedis.srem(key, members.get(i))).run();
- Set<String> results = jedis3.smembers(key);
+ Set<String> results = jedis.smembers(key);
assertThat(results).isEmpty();
}
+
@Test
- public void shouldDistributeDataAmongMultipleServers_givenMultipleClients_removingFromDifferentSetsConcurrently() {
+ public void shouldDistributeDataAmongCluster_thenRemoveFromDifferentSetsConcurrently() {
String key1 = "key1";
String key2 = "key2";
List<String> members1 = makeMemberList(SET_SIZE, "member1-");
List<String> members2 = makeMemberList(SET_SIZE, "member2-");
- jedis3.sadd(key1, members1.toArray(new String[] {}));
- jedis3.sadd(key2, members2.toArray(new String[] {}));
+ jedis.sadd(key1, members1.toArray(new String[] {}));
+ jedis.sadd(key2, members2.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key1, members1.get(i)),
- (i) -> jedis2.srem(key2, members2.get(i))).run();
+ (i) -> jedis.srem(key1, members1.get(i)),
+ (i) -> jedis.srem(key2, members2.get(i))).run();
- Set<String> results1 = jedis3.smembers(key1);
- Set<String> results2 = jedis3.smembers(key2);
+ Set<String> results1 = jedis.smembers(key1);
+ Set<String> results2 = jedis.smembers(key2);
assertThat(results1).isEmpty();
assertThat(results2).isEmpty();
}
+
+ @Test
+ public void shouldDistributeDataAmongCluster_givenMultipleClientsOnDifferentServer_removingSameDataFromSameSetConcurrently() {
+
+ int redisServerPort2 = clusterStartUp.getRedisPort(2);
+ JedisCluster jedis2 =
+ new JedisCluster(new HostAndPort(LOCAL_HOST, redisServerPort), redisServerPort2);
+
+ String key = "key";
+
+ List<String> members = makeMemberList(SET_SIZE, "member1-");
+ jedis.sadd(key, members.toArray(new String[] {}));
+
+ new ConcurrentLoopingThreads(SET_SIZE,
+ (i) -> jedis.srem(key, members.get(i)),
+ (i) -> jedis2.srem(key, members.get(i))).run();
+
+ Set<String> results = jedis.smembers(key);
+
+ assertThat(results).isEmpty();
+
+ jedis2.close();
+ }
+
+
@Test
- public void shouldDistributeDataAmongMultipleServers_givenMultipleClientsOnSameServer_removingSameDataFromSameSetConcurrently() {
+ public void shouldDistributeDataAmongCluster_givenMultipleClientsOnSameServer_removingSameDataFromSameSetConcurrently() {
- Jedis jedis1B = new Jedis(LOCAL_HOST, redisServerPort1);
- Jedis jedis2B = new Jedis(LOCAL_HOST, redisServerPort2);
+ JedisCluster jedis2 =
+ new JedisCluster(new HostAndPort(LOCAL_HOST, redisServerPort), redisServerPort);
String key = "key";
List<String> members = makeMemberList(SET_SIZE, "member1-");
- jedis3.sadd(key, members.toArray(new String[] {}));
+ jedis.sadd(key, members.toArray(new String[] {}));
new ConcurrentLoopingThreads(SET_SIZE,
- (i) -> jedis1.srem(key, members.get(i)),
- (i) -> jedis1B.srem(key, members.get(i)),
- (i) -> jedis2.srem(key, members.get(i)),
- (i) -> jedis2B.srem(key, members.get(i))).run();
+ (i) -> jedis.srem(key, members.get(i)),
+ (i) -> jedis2.srem(key, members.get(i))).run();
+
+ Set<String> results = jedis.smembers(key);
+
+ assertThat(results).isEmpty();
+
+ jedis2.close();
+ }
+
+ @Test
+ public void shouldDistributeDataAmongCluster_givenMultipleClientsOnDifferentServer_removingDifferentDataFromSameSetConcurrently() {
+
+ int redisServerPort2 = clusterStartUp.getRedisPort(2);
+ JedisCluster jedis2 =
+ new JedisCluster(new HostAndPort(LOCAL_HOST, redisServerPort), redisServerPort2);
+
+ String key = "key1";
+
+ List<String> members1 = makeMemberList(SET_SIZE, "member1-");
+ List<String> members2 = makeMemberList(SET_SIZE, "member2-");
+
+ List<String> allMembers = new ArrayList<>();
+ allMembers.addAll(members1);
+ allMembers.addAll(members2);
- Set<String> results = jedis3.smembers(key);
+ jedis.sadd(key, allMembers.toArray(new String[] {}));
+
+ new ConcurrentLoopingThreads(SET_SIZE,
+ (i) -> jedis.srem(key, members1.get(i)),
+ (i) -> jedis2.srem(key, members2.get(i))).run();
Review comment:
You can just use the same `jedis` instance here instead of creating a new `jedis2`.
--
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:
users@infra.apache.org