This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch support/1.15
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 0331eeeb0895167680e3d3f876bf05b457fec5aa
Author: Kris10 <kod...@vmware.com>
AuthorDate: Thu Jan 27 12:40:53 2022 -0800
GEODE-9830: SINTERSTORE Command Support (#7302)
(cherry picked from commit 32e32af84a2727b3589d5940546ccc655a40e69d)
---
geode-for-redis/README.md | 1 +
.../set/SInterStoreNativeRedisAcceptanceTest.java} | 20 ++-
.../server/AbstractHitsMissesIntegrationTest.java | 12 +-
.../set/AbstractSInterIntegrationTest.java | 140 +----------------
...ava => AbstractSInterStoreIntegrationTest.java} | 127 +++++++--------
.../set/AbstractSUnionStoreIntegrationTest.java | 2 +-
.../executor/set/SInterStoreIntegrationTest.java} | 18 +--
.../redis/internal/commands/RedisCommandType.java | 4 +-
.../commands/executor/set/SDiffExecutor.java | 17 +-
.../commands/executor/set/SDiffStoreExecutor.java | 14 +-
.../commands/executor/set/SInterExecutor.java | 16 +-
.../commands/executor/set/SInterStoreExecutor.java | 13 +-
.../commands/executor/set/SUnionExecutor.java | 16 +-
.../commands/executor/set/SUnionStoreExecutor.java | 13 +-
.../commands/executor/set/SetOpExecutor.java | 173 +++------------------
.../apache/geode/redis/internal/data/RedisSet.java | 51 +++---
16 files changed, 196 insertions(+), 441 deletions(-)
diff --git a/geode-for-redis/README.md b/geode-for-redis/README.md
index 592b129..183d148 100644
--- a/geode-for-redis/README.md
+++ b/geode-for-redis/README.md
@@ -203,6 +203,7 @@ Geode for Redis implements a subset of the full Redis
command set.
- SDIFF
- SDIFFSTORE
- SINTER
+- SINTERSTORE
- SISMEMBER
- SET
- SETEX
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreNativeRedisAcceptanceTest.java
similarity index 69%
copy from
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
copy to
geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreNativeRedisAcceptanceTest.java
index 1021b86..b19232a 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
+++
b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreNativeRedisAcceptanceTest.java
@@ -14,18 +14,24 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-import java.util.Set;
-public class SDiffExecutor extends SetOpExecutor {
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.NativeRedisClusterTestRule;
+
+public class SInterStoreNativeRedisAcceptanceTest extends
AbstractSInterStoreIntegrationTest {
+
+ @ClassRule
+ public static NativeRedisClusterTestRule redis = new
NativeRedisClusterTestRule();
@Override
- protected boolean doSetOp(Set<byte[]> resultSet, Set<byte[]> nextSet) {
- resultSet.removeAll(nextSet);
- return resultSet.isEmpty();
+ public int getPort() {
+ return redis.getExposedPorts().get(0);
}
@Override
- protected boolean isStorage() {
- return false;
+ public void flushAll() {
+ redis.flushAll();
}
+
}
diff --git
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractHitsMissesIntegrationTest.java
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractHitsMissesIntegrationTest.java
index 23735d9..0933ed6 100644
---
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractHitsMissesIntegrationTest.java
+++
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/server/AbstractHitsMissesIntegrationTest.java
@@ -395,6 +395,12 @@ public abstract class AbstractHitsMissesIntegrationTest
implements RedisIntegrat
}
@Test
+ public void testSinterstore() {
+ runMultiKeyCommandAndAssertNoStatUpdates(SET_KEY,
+ (k1, k2) -> jedis.sinterstore(HASHTAG + "dest", k1, k2));
+ }
+
+ @Test
public void testSismember() {
runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.sismember(k,
"member"));
}
@@ -565,12 +571,6 @@ public abstract class AbstractHitsMissesIntegrationTest
implements RedisIntegrat
runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.sscan(k, "0"));
}
- @Test
- public void testSinterstore() {
- runMultiKeyCommandAndAssertNoStatUpdates(SET_KEY,
- (k1, k2) -> jedis.sinterstore(HASHTAG + "dest", k1, k2));
- }
-
/************* Helper Methods *************/
private void runCommandAndAssertHitsAndMisses(String key, Consumer<String>
command) {
Map<String, String> info = RedisTestHelper.getInfo(jedis);
diff --git
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterIntegrationTest.java
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterIntegrationTest.java
index 115317d..34b927d 100755
---
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterIntegrationTest.java
+++
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterIntegrationTest.java
@@ -17,13 +17,12 @@ package
org.apache.geode.redis.internal.commands.executor.set;
import static
org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_TYPE;
+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 org.assertj.core.api.Assertions.assertThatThrownBy;
import static redis.clients.jedis.Protocol.Command.SINTER;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
@@ -32,23 +31,19 @@ import org.junit.Before;
import org.junit.Test;
import redis.clients.jedis.HostAndPort;
import redis.clients.jedis.JedisCluster;
-import redis.clients.jedis.Protocol;
import org.apache.geode.redis.ConcurrentLoopingThreads;
import org.apache.geode.redis.RedisIntegrationTest;
-import org.apache.geode.test.awaitility.GeodeAwaitility;
public abstract class AbstractSInterIntegrationTest implements
RedisIntegrationTest {
private JedisCluster jedis;
- private static final int REDIS_CLIENT_TIMEOUT =
- Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
private static final String SET1 = "{tag1}set1";
private static final String SET2 = "{tag1}set2";
private static final String SET3 = "{tag1}set3";
@Before
public void setUp() {
- jedis = new JedisCluster(new HostAndPort("localhost", getPort()),
REDIS_CLIENT_TIMEOUT);
+ jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()),
REDIS_CLIENT_TIMEOUT);
}
@After
@@ -63,11 +58,6 @@ public abstract class AbstractSInterIntegrationTest
implements RedisIntegrationT
}
@Test
- public void sinterstoreErrors_givenTooFewArguments() {
- assertAtLeastNArgs(jedis, Protocol.Command.SINTERSTORE, 2);
- }
-
- @Test
public void sinter_withSetsFromDifferentSlots_returnsCrossSlotError() {
String setKeyDifferentSlot = "{tag2}set2";
jedis.sadd(SET1, "member1");
@@ -201,128 +191,4 @@ public abstract class AbstractSInterIntegrationTest
implements RedisIntegrationT
jedis.srem(SET3, newValues);
});
}
-
- @Test
- public void testSInterStore() {
- String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
- String[] secondSet = new String[] {"apple", "microsoft", "linux", "peach"};
- String[] thirdSet = new String[] {"luigi", "bowser", "peach", "mario"};
- jedis.sadd("{tag1}set1", firstSet);
- jedis.sadd("{tag1}set2", secondSet);
- jedis.sadd("{tag1}set3", thirdSet);
-
- Long resultSize =
- jedis.sinterstore("{tag1}result", "{tag1}set1", "{tag1}set2",
"{tag1}set3");
- Set<String> resultSet = jedis.smembers("{tag1}result");
-
- String[] expected = new String[] {"peach"};
- assertThat(resultSize).isEqualTo(expected.length);
- assertThat(resultSet).containsExactlyInAnyOrder(expected);
-
- Long otherResultSize = jedis.sinterstore("{tag1}set1", "{tag1}set1",
"{tag1}set2");
- Set<String> otherResultSet = jedis.smembers("{tag1}set1");
- String[] otherExpected = new String[] {"apple", "peach"};
- assertThat(otherResultSize).isEqualTo(otherExpected.length);
- assertThat(otherResultSet).containsExactlyInAnyOrder(otherExpected);
-
- Long emptySetSize =
- jedis.sinterstore("{tag1}newEmpty", "{tag1}nonexistent", "{tag1}set2",
"{tag1}set3");
- Set<String> emptyResultSet = jedis.smembers("{tag1}newEmpty");
- assertThat(emptySetSize).isEqualTo(0L);
- assertThat(emptyResultSet).isEmpty();
-
- emptySetSize =
- jedis.sinterstore("{tag1}set1", "{tag1}nonexistent", "{tag1}set2",
"{tag1}set3");
- emptyResultSet = jedis.smembers("{tag1}set1");
- assertThat(emptySetSize).isEqualTo(0L);
- assertThat(emptyResultSet).isEmpty();
-
- Long copySetSize = jedis.sinterstore("{tag1}copySet", "{tag1}set2",
"{tag1}newEmpty");
- Set<String> copyResultSet = jedis.smembers("{tag1}copySet");
- assertThat(copySetSize).isEqualTo(0);
- assertThat(copyResultSet).isEmpty();
- }
-
- @Test
- public void testSInterStore_withNonExistentKeys() {
- String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
- jedis.sadd("{tag1}set1", firstSet);
-
- Long resultSize =
- jedis.sinterstore("{tag1}set1", "{tag1}nonExistent1",
"{tag1}nonExistent2");
- assertThat(resultSize).isEqualTo(0);
- assertThat(jedis.exists("{tag1}set1")).isFalse();
- }
-
- @Test
- public void testSInterStore_withNonExistentKeys_andNonSetTarget() {
- jedis.set("string1", "stringValue");
-
- Long resultSize =
- jedis.sinterstore("{tag1}string1", "{tag1}nonExistent1",
"{tag1}nonExistent2");
- assertThat(resultSize).isEqualTo(0);
- assertThat(jedis.exists("{tag1}set1")).isFalse();
- }
-
- @Test
- public void testSInterStore_withNonSetKey() {
- String[] firstSet = new String[] {"pear", "apple", "plum", "orange",
"peach"};
- jedis.sadd("{tag1}set1", firstSet);
- jedis.set("{tag1}string1", "value1");
-
- assertThatThrownBy(() -> jedis.sinterstore("{tag1}set1", "{tag1}string1"))
- .hasMessage("WRONGTYPE Operation against a key holding the wrong kind
of value");
- assertThat(jedis.exists("{tag1}set1")).isTrue();
- }
-
- @Test
- public void testConcurrentSInterStore() throws InterruptedException {
- int ENTRIES = 100;
- int SUBSET_SIZE = 100;
-
- Set<String> masterSet = new HashSet<>();
- for (int i = 0; i < ENTRIES; i++) {
- masterSet.add("master-" + i);
- }
-
- List<Set<String>> otherSets = new ArrayList<>();
- for (int i = 0; i < ENTRIES; i++) {
- Set<String> oneSet = new HashSet<>();
- for (int j = 0; j < SUBSET_SIZE; j++) {
- oneSet.add("set-" + i + "-" + j);
- }
- otherSets.add(oneSet);
- }
-
- jedis.sadd("master", masterSet.toArray(new String[] {}));
-
- for (int i = 0; i < ENTRIES; i++) {
- jedis.sadd("set-" + i, otherSets.get(i).toArray(new String[] {}));
- jedis.sadd("set-" + i, masterSet.toArray(new String[] {}));
- }
-
- Runnable runnable1 = () -> {
- for (int i = 0; i < ENTRIES; i++) {
- jedis.sinterstore("master", "master", "set-" + i);
- Thread.yield();
- }
- };
-
- Runnable runnable2 = () -> {
- for (int i = 0; i < ENTRIES; i++) {
- jedis.sinterstore("master", "master", "set-" + i);
- Thread.yield();
- }
- };
-
- Thread thread1 = new Thread(runnable1);
- Thread thread2 = new Thread(runnable2);
-
- thread1.start();
- thread2.start();
- thread1.join();
- thread2.join();
-
-
assertThat(jedis.smembers("master").toArray()).containsExactlyInAnyOrder(masterSet.toArray());
- }
}
diff --git
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterStoreIntegrationTest.java
similarity index 57%
copy from
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
copy to
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterStoreIntegrationTest.java
index b6526c1..305a063 100644
---
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
+++
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterStoreIntegrationTest.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
+
import static
org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertAtLeastNArgs;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_TYPE;
@@ -21,7 +22,7 @@ import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.BIND_ADD
import static
org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CLIENT_TIMEOUT;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import static redis.clients.jedis.Protocol.Command.SUNIONSTORE;
+import static redis.clients.jedis.Protocol.Command.SINTERSTORE;
import java.util.concurrent.atomic.AtomicLong;
@@ -35,7 +36,7 @@ import redis.clients.jedis.Protocol;
import org.apache.geode.redis.ConcurrentLoopingThreads;
import org.apache.geode.redis.RedisIntegrationTest;
-public abstract class AbstractSUnionStoreIntegrationTest implements
RedisIntegrationTest {
+public abstract class AbstractSInterStoreIntegrationTest implements
RedisIntegrationTest {
private JedisCluster jedis;
private static final String DESTINATION_KEY = "{tag1}destinationKey";
private static final String[] DESTINATION_MEMBERS =
@@ -57,159 +58,155 @@ public abstract class AbstractSUnionStoreIntegrationTest
implements RedisIntegra
}
@Test
- public void sunionstore_givenTooFewArguments_returnsError() {
- assertAtLeastNArgs(jedis, Protocol.Command.SUNION, 1);
+ public void sinterstore_givenTooFewArguments_returnsError() {
+ assertAtLeastNArgs(jedis, Protocol.Command.SINTERSTORE, 2);
}
@Test
- public void sunionstore_withExistentSet_returnsUnionSize_storesUnion() {
+ public void sinterstore_withExistentSet_returnsInterSize_storesInter() {
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
- assertThat(jedis.sunionstore(DESTINATION_KEY,
SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length);
+ assertThat(jedis.sinterstore(DESTINATION_KEY,
SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length);
assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1);
}
@Test
- public void sunionstore_withNonExistentSet_returnsZero_destKeyDoesNotExist()
{
- assertThat(jedis.sunionstore(DESTINATION_KEY,
NON_EXISTENT_SET)).isEqualTo(0);
+ public void
sinterstore_withNonExistentSet_returnsZero_doesNotCreateSetAtDestination() {
+ assertThat(jedis.sinterstore(DESTINATION_KEY,
NON_EXISTENT_SET)).isEqualTo(0);
assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
}
@Test
- public void
sunionstore_withOneExistentAndOneNonExistentSet_returnsUnionSize_storesUnion() {
+ public void
sinterstore_withExistentFirstSetAndNonExistentSecondSet_returnsZero_doesNotCreateSetAtDestination()
{
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
- assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, NON_EXISTENT_SET))
- .isEqualTo(SET_MEMBERS_1.length);
-
assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1);
+ assertThat(jedis.sinterstore(DESTINATION_KEY, SET_KEY_1, NON_EXISTENT_SET))
+ .isEqualTo(0);
+ assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
}
@Test
- public void
sunionstore_withOneNonExistentAndOneExistentSet_returnsUnionSize_storesUnion() {
+ public void
sinterstore_withNonExistentFirstSetAndExistentSecondSetSet_returnsZero_doesNotCreateSetAtDestination()
{
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
- assertThat(jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET, SET_KEY_1))
- .isEqualTo(SET_MEMBERS_1.length);
-
assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1);
+ assertThat(jedis.sinterstore(DESTINATION_KEY, NON_EXISTENT_SET, SET_KEY_1))
+ .isEqualTo(0);
+ assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
}
@Test
- public void
sunionstore_withNonOverlappingSets_returnsUnionSize_storesUnion() {
+ public void
sinterstore_withNoIntersectingMembers_returnsZero_doesNotCreateSetAtDestination()
{
String[] secondSetMembers = new String[] {"apple", "microsoft", "linux",
"peach"};
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(SET_KEY_2, secondSetMembers);
- String[] result =
- {"one", "two", "three", "four", "five", "apple", "microsoft", "linux",
"peach"};
- assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1,
SET_KEY_2)).isEqualTo(result.length);
-
assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(result);
-
+ assertThat(jedis.sinterstore(DESTINATION_KEY, SET_KEY_1,
SET_KEY_2)).isEqualTo(0);
+ assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
}
@Test
- public void sunionstore_withSomeSharedValues_returnsUnionSize_storesUnion() {
+ public void
sinterstore_withSomeIntersectingMembers_returnsInterSize_storesInter() {
String[] secondSetMembers = new String[] {"one", "two", "linux", "peach"};
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(SET_KEY_2, secondSetMembers);
- String[] result = {"one", "two", "three", "four", "five", "linux",
"peach"};
- assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1,
SET_KEY_2)).isEqualTo(result.length);
+ String[] result = {"one", "two"};
+ assertThat(jedis.sinterstore(DESTINATION_KEY, SET_KEY_1,
SET_KEY_2)).isEqualTo(result.length);
assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(result);
}
@Test
- public void sunionstore_withAllSharedValues_returnsUnionSize_storesUnion() {
+ public void
sinterstore_withAllIntersectingMembers_returnsInterSize_storesInter() {
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(SET_KEY_2, SET_MEMBERS_1);
- assertThat(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2))
+ assertThat(jedis.sinterstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2))
.isEqualTo(SET_MEMBERS_1.length);
assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1);
}
@Test
- public void
sunionstore_withMultipleNonExistentSets_returnsZero_destKeyDoesNotExist() {
- assertThat(jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET,
"{tag1}nonExistentSet2"))
+ public void
sinterstore_withMultipleNonExistentSets_returnsZero_doesNotCreateSetAtDestination()
{
+ assertThat(jedis.sinterstore(DESTINATION_KEY, NON_EXISTENT_SET,
"{tag1}nonExistentSet2"))
.isEqualTo(0);
assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
}
@Test
- public void
sunionstore_withExistentSet_returnsUnionSize_destKeyOverwrittenWithUnion() {
+ public void
sinterstore_withExistentSet_returnsInterSize_destKeyOverwrittenWithInter() {
jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
- assertThat(jedis.sunionstore(DESTINATION_KEY,
SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length);
+ assertThat(jedis.sinterstore(DESTINATION_KEY,
SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length);
assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1);
}
@Test
- public void sunionstore_withNonExistentSet_returnsZero_destKeyIsDeleted() {
+ public void sinterstore_withNonExistentSet_returnsZero_destKeyIsDeleted() {
jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
- assertThat(jedis.sunionstore(DESTINATION_KEY,
NON_EXISTENT_SET)).isEqualTo(0);
+ assertThat(jedis.sinterstore(DESTINATION_KEY,
NON_EXISTENT_SET)).isEqualTo(0);
assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
}
@Test
- public void
sunionstore_withNonSetDestKey_withExistentSet_returnsUnionSize_destKeyOverwrittenWithUnion()
{
+ public void
sinterstore_withNonSetDestKey_withExistentSet_returnsInterSize_destKeyOverwrittenWithInter()
{
String stringKey = "{tag1}ding";
jedis.set(stringKey, "dong");
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
- assertThat(jedis.sunionstore(stringKey,
SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length);
+ assertThat(jedis.sinterstore(stringKey,
SET_KEY_1)).isEqualTo(SET_MEMBERS_1.length);
assertThat(jedis.smembers(stringKey)).containsExactlyInAnyOrder(SET_MEMBERS_1);
}
@Test
- public void
sunionstore_withNonSetDestKey_withNonExistentSet_returnsZero_destKeyIsDeleted()
{
+ public void
sinterstore_withNonSetDestKey_withNonExistentSet_returnsZero_destKeyIsDeleted()
{
String stringKey = "{tag1}ding";
jedis.set(stringKey, "dong");
- assertThat(jedis.sunionstore(stringKey, NON_EXISTENT_SET)).isEqualTo(0);
+ assertThat(jedis.sinterstore(stringKey, NON_EXISTENT_SET)).isEqualTo(0);
assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
}
@Test
- public void
sunionstore_withNonExistentDest_withNonSetKeyAsFirstKey_returnsWrongTypeError()
{
+ public void sinterstore_withNonSetKeyAsFirstKey_returnsWrongTypeError() {
String stringKey = "{tag1}ding";
jedis.set(stringKey, "dong");
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(SET_KEY_2, "doorbell");
- assertThatThrownBy(() -> jedis.sunionstore(DESTINATION_KEY, stringKey,
SET_KEY_1, SET_KEY_2))
+ assertThatThrownBy(() -> jedis.sinterstore(DESTINATION_KEY, stringKey,
SET_KEY_1, SET_KEY_2))
.hasMessageContaining(ERROR_WRONG_TYPE);
}
@Test
- public void
sunionstore_withExistentDest_withNonSetKeyAsThirdKey_returnsWrongTypeError() {
+ public void sinterstore_withNonSetKeyAsThirdKey_returnsWrongTypeError() {
String stringKey = "{tag1}ding";
jedis.set(stringKey, "dong");
jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(SET_KEY_2, "doorbell");
- assertThatThrownBy(() -> jedis.sunionstore(DESTINATION_KEY, SET_KEY_1,
SET_KEY_2, stringKey))
+ assertThatThrownBy(() -> jedis.sinterstore(DESTINATION_KEY, SET_KEY_1,
SET_KEY_2, stringKey))
.hasMessageContaining(ERROR_WRONG_TYPE);
}
@Test
- public void
sunionstore_withNonExistentDest_withNonSetKeyAsThirdKeyAndNonExistentSetAsFirstKey_returnsWrongTypeError()
{
+ public void
sinterstore_withNonSetKeyAsThirdKeyAndNonExistentSetAsFirstKey_returnsWrongTypeError()
{
String stringKey = "{tag1}ding";
jedis.set(stringKey, "dong");
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(SET_KEY_2, "doorbell");
assertThatThrownBy(
- () -> jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET, SET_KEY_1,
stringKey))
+ () -> jedis.sinterstore(DESTINATION_KEY, NON_EXISTENT_SET, SET_KEY_1,
stringKey))
.hasMessageContaining(ERROR_WRONG_TYPE);
}
@Test
- public void sunionstore_withSetsFromDifferentSlots_returnsCrossSlotError() {
+ public void sinterstore_withSetsFromDifferentSlots_returnsCrossSlotError() {
String setKeyDifferentSlot = "{tag2}setKey2";
String[] secondSetMembers = new String[] {"one", "two", "linux", "peach"};
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(setKeyDifferentSlot, secondSetMembers);
- assertThatThrownBy(() -> jedis.sendCommand(DESTINATION_KEY, SUNIONSTORE,
DESTINATION_KEY,
- SET_KEY_1, setKeyDifferentSlot))
- .hasMessage("CROSSSLOT " + ERROR_WRONG_SLOT);
+ assertThatThrownBy(() -> jedis.sendCommand(DESTINATION_KEY, SINTERSTORE,
DESTINATION_KEY,
+ SET_KEY_1, setKeyDifferentSlot)).hasMessage("CROSSSLOT " +
ERROR_WRONG_SLOT);
}
@Test
@@ -218,27 +215,23 @@ public abstract class AbstractSUnionStoreIntegrationTest
implements RedisIntegra
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(SET_KEY_2, secondSetMembers);
- String[] unionMembers = {"one", "two", "three", "four", "five", "linux",
"peach"};
- final AtomicLong sunionstoreResultReference = new AtomicLong(0);
+ String[] result = {"one", "two"};
+ final AtomicLong sinterSizeReference = new AtomicLong(0);
new ConcurrentLoopingThreads(1000,
i -> jedis.srem(SET_KEY_2, secondSetMembers),
- i -> sunionstoreResultReference
- .set(jedis.sunionstore(DESTINATION_KEY, SET_KEY_1, SET_KEY_2)))
- .runWithAction(() -> {
- // Check sunionstore return size of union
- assertThat(sunionstoreResultReference).satisfiesAnyOf(
- sunionstoreSize -> assertThat(sunionstoreSize.get())
- .isEqualTo(SET_MEMBERS_1.length),
- sunionstoreSize -> assertThat(sunionstoreSize.get())
- .isEqualTo(unionMembers.length));
- // Checks if values were stored in destination key
- assertThat(DESTINATION_KEY).satisfiesAnyOf(
- key -> assertThat(jedis.smembers(DESTINATION_KEY))
- .containsExactlyInAnyOrder(SET_MEMBERS_1),
- key -> assertThat(jedis.smembers(DESTINATION_KEY))
- .containsExactlyInAnyOrder(unionMembers));
- jedis.sadd(SET_KEY_2, secondSetMembers);
- jedis.del(DESTINATION_KEY);
- });
+ i -> sinterSizeReference.set(jedis.sinterstore(DESTINATION_KEY,
SET_KEY_1, SET_KEY_2)))
+ .runWithAction(() -> {
+ // Checks sinterstore return size of inter
+ assertThat(sinterSizeReference).satisfiesAnyOf(
+ sinterSize -> assertThat(sinterSize.get()).isEqualTo(0),
+ sinterSize ->
assertThat(sinterSize.get()).isEqualTo(result.length));
+ // Checks if values were stored in destination key
+ assertThat(DESTINATION_KEY).satisfiesAnyOf(
+ key -> assertThat(jedis.exists(key)).isFalse(),
+ key -> assertThat(jedis.smembers(DESTINATION_KEY))
+ .containsExactlyInAnyOrder(result));
+ jedis.sadd(SET_KEY_2, secondSetMembers);
+ jedis.del(DESTINATION_KEY);
+ });
}
}
diff --git
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
index b6526c1..5a71f4c 100644
---
a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
+++
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
@@ -58,7 +58,7 @@ public abstract class AbstractSUnionStoreIntegrationTest
implements RedisIntegra
@Test
public void sunionstore_givenTooFewArguments_returnsError() {
- assertAtLeastNArgs(jedis, Protocol.Command.SUNION, 1);
+ assertAtLeastNArgs(jedis, Protocol.Command.SUNIONSTORE, 2);
}
@Test
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreIntegrationTest.java
similarity index 73%
copy from
geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
copy to
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreIntegrationTest.java
index 1021b86..dd78b7d 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
+++
b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreIntegrationTest.java
@@ -14,18 +14,18 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-import java.util.Set;
+import org.junit.ClassRule;
-public class SDiffExecutor extends SetOpExecutor {
+import org.apache.geode.redis.GeodeRedisServerRule;
- @Override
- protected boolean doSetOp(Set<byte[]> resultSet, Set<byte[]> nextSet) {
- resultSet.removeAll(nextSet);
- return resultSet.isEmpty();
- }
+public class SInterStoreIntegrationTest extends
AbstractSInterStoreIntegrationTest {
+
+ @ClassRule
+ public static GeodeRedisServerRule server = new GeodeRedisServerRule();
@Override
- protected boolean isStorage() {
- return false;
+ public int getPort() {
+ return server.getPort();
}
+
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
index 7887281..c11155b 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandType.java
@@ -246,6 +246,8 @@ public enum RedisCommandType {
new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)),
SINTER(new SInterExecutor(), SUPPORTED,
new Parameter().min(2).lastKey(-1).flags(READONLY, SORT_FOR_SCRIPT)),
+ SINTERSTORE(new SInterStoreExecutor(), SUPPORTED,
+ new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)),
SISMEMBER(new SIsMemberExecutor(), SUPPORTED, new
Parameter().exact(3).flags(READONLY, FAST)),
SMEMBERS(new SMembersExecutor(), SUPPORTED,
new Parameter().exact(2).flags(READONLY, SORT_FOR_SCRIPT)),
@@ -353,8 +355,6 @@ public enum RedisCommandType {
/**************** Sets *****************/
- SINTERSTORE(new SInterStoreExecutor(), UNSUPPORTED,
- new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)),
SPOP(new SPopExecutor(), UNSUPPORTED,
new Parameter().min(2).max(3, ERROR_SYNTAX).flags(WRITE, RANDOM, FAST)),
SSCAN(new SScanExecutor(), UNSUPPORTED, new
Parameter().min(3).flags(READONLY, RANDOM),
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
index 1021b86..e8ba9ef 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffExecutor.java
@@ -14,18 +14,19 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
+import static org.apache.geode.redis.internal.data.RedisSet.sdiff;
+
+import java.util.List;
import java.util.Set;
-public class SDiffExecutor extends SetOpExecutor {
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.services.RegionProvider;
- @Override
- protected boolean doSetOp(Set<byte[]> resultSet, Set<byte[]> nextSet) {
- resultSet.removeAll(nextSet);
- return resultSet.isEmpty();
- }
+public class SDiffExecutor extends SetOpArrayResult {
@Override
- protected boolean isStorage() {
- return false;
+ protected Set<byte[]> getResult(RegionProvider regionProvider,
List<RedisKey> setKeys) {
+ return sdiff(regionProvider, setKeys, true);
}
+
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffStoreExecutor.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffStoreExecutor.java
index 644a802..fff9088 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffStoreExecutor.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SDiffStoreExecutor.java
@@ -14,10 +14,18 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-public class SDiffStoreExecutor extends SDiffExecutor {
+import static org.apache.geode.redis.internal.data.RedisSet.sdiffstore;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.services.RegionProvider;
+
+public class SDiffStoreExecutor extends SetOpIntegerResult {
@Override
- protected boolean isStorage() {
- return true;
+ protected int getResult(RegionProvider regionProvider, List<RedisKey>
setKeys, RedisKey destKey) {
+ return sdiffstore(regionProvider, setKeys, destKey);
}
+
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterExecutor.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterExecutor.java
index 1f83344..d40cb6a 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterExecutor.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterExecutor.java
@@ -14,18 +14,18 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
+import static org.apache.geode.redis.internal.data.RedisSet.sinter;
+
+import java.util.List;
import java.util.Set;
-public class SInterExecutor extends SetOpExecutor {
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.services.RegionProvider;
- @Override
- protected boolean isStorage() {
- return false;
- }
+public class SInterExecutor extends SetOpArrayResult {
@Override
- protected boolean doSetOp(Set<byte[]> resultSet, Set<byte[]> nextSet) {
- resultSet.retainAll(nextSet);
- return resultSet.isEmpty();
+ protected Set<byte[]> getResult(RegionProvider regionProvider,
List<RedisKey> setKeys) {
+ return sinter(regionProvider, setKeys, true);
}
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreExecutor.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreExecutor.java
index de1fabb..bc6dc1a 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreExecutor.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SInterStoreExecutor.java
@@ -15,11 +15,18 @@
package org.apache.geode.redis.internal.commands.executor.set;
+import static org.apache.geode.redis.internal.data.RedisSet.sinterstore;
-public class SInterStoreExecutor extends SInterExecutor {
+import java.util.List;
+
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.services.RegionProvider;
+
+public class SInterStoreExecutor extends SetOpIntegerResult {
@Override
- protected boolean isStorage() {
- return true;
+ protected int getResult(RegionProvider regionProvider, List<RedisKey>
setKeys, RedisKey destKey) {
+ return sinterstore(regionProvider, setKeys, destKey);
}
+
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionExecutor.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionExecutor.java
index 5089417..ed88ed9 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionExecutor.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionExecutor.java
@@ -14,18 +14,18 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
+import static org.apache.geode.redis.internal.data.RedisSet.sunion;
+
+import java.util.List;
import java.util.Set;
-public class SUnionExecutor extends SetOpExecutor {
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.services.RegionProvider;
- @Override
- protected boolean isStorage() {
- return false;
- }
+public class SUnionExecutor extends SetOpArrayResult {
@Override
- protected boolean doSetOp(Set<byte[]> resultSet, Set<byte[]> nextSet) {
- resultSet.addAll(nextSet);
- return false;
+ protected Set<byte[]> getResult(RegionProvider regionProvider,
List<RedisKey> setKeys) {
+ return sunion(regionProvider, setKeys, true);
}
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreExecutor.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreExecutor.java
index 9a956fe..2d5860b 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreExecutor.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreExecutor.java
@@ -14,10 +14,17 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-public class SUnionStoreExecutor extends SUnionExecutor {
+import static org.apache.geode.redis.internal.data.RedisSet.sunionstore;
+
+import java.util.List;
+
+import org.apache.geode.redis.internal.data.RedisKey;
+import org.apache.geode.redis.internal.services.RegionProvider;
+
+public class SUnionStoreExecutor extends SetOpIntegerResult {
@Override
- protected boolean isStorage() {
- return true;
+ protected int getResult(RegionProvider regionProvider, List<RedisKey>
setKeys, RedisKey destKey) {
+ return sunionstore(regionProvider, setKeys, destKey);
}
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java
index 9f6fce0..a769de8 100755
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetOpExecutor.java
@@ -14,24 +14,15 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-import static java.util.Collections.emptySet;
-import static org.apache.geode.redis.internal.data.RedisSet.sdiff;
-import static org.apache.geode.redis.internal.data.RedisSet.sdiffstore;
-import static org.apache.geode.redis.internal.data.RedisSet.sinter;
-import static org.apache.geode.redis.internal.data.RedisSet.sunion;
-import static org.apache.geode.redis.internal.data.RedisSet.sunionstore;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import org.apache.geode.redis.internal.commands.Command;
-import org.apache.geode.redis.internal.commands.RedisCommandType;
import org.apache.geode.redis.internal.commands.executor.CommandExecutor;
import org.apache.geode.redis.internal.commands.executor.RedisResponse;
import org.apache.geode.redis.internal.data.RedisKey;
-import org.apache.geode.redis.internal.data.RedisSet;
-import org.apache.geode.redis.internal.data.RedisSet.MemberSet;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
import org.apache.geode.redis.internal.services.RegionProvider;
@@ -39,160 +30,36 @@ public abstract class SetOpExecutor implements
CommandExecutor {
@Override
public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
- int setsStartIndex = 1;
-
- if (isStorage()) {
- setsStartIndex++;
- }
-
- List<RedisKey> commandElements = command.getProcessedCommandKeys();
- List<RedisKey> setKeys = commandElements.subList(setsStartIndex,
commandElements.size());
RegionProvider regionProvider = context.getRegionProvider();
+ List<RedisKey> commandElements = command.getProcessedCommandKeys();
+ List<RedisKey> setKeys = commandElements.subList(1,
commandElements.size());
- /*
- * SINTERSTORE currently use the else part of the code for their
implementation.
- * TODO: Once the above commands have been implemented remove the if else
- * Refactor so the implementation is in the executor. After delete
doActualSetOperation,
- * doStoreSetOp, doStoreSetOpWhileLocked, computeStoreSetOp, fetchSets
- */
- List<RedisKey> keysToLock = new ArrayList<>(setKeys);
- if (isStorage()) {
- keysToLock.add(command.getKey());
- }
- if (command.isOfType(RedisCommandType.SDIFF) ||
command.isOfType(RedisCommandType.SDIFFSTORE)) {
- if (isStorage()) {
- RedisKey destinationKey = command.getKey();
- int resultSize = context.lockedExecute(destinationKey, keysToLock,
- () -> sdiffstore(regionProvider, destinationKey, setKeys));
- return RedisResponse.integer(resultSize);
- }
-
- Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), keysToLock,
- () -> sdiff(regionProvider, setKeys));
- return RedisResponse.array(resultSet, true);
-
- } else if (command.isOfType(RedisCommandType.SINTER)) {
- Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), keysToLock,
- () -> sinter(regionProvider, setKeys));
- return RedisResponse.array(resultSet, true);
-
- } else if (command.isOfType(RedisCommandType.SUNION)
- || command.isOfType(RedisCommandType.SUNIONSTORE)) {
- if (isStorage()) {
- RedisKey destinationKey = command.getKey();
- int resultSize = context.lockedExecute(destinationKey, keysToLock,
- () -> sunionstore(regionProvider, destinationKey, setKeys));
- return RedisResponse.integer(resultSize);
- }
-
- Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), keysToLock,
- () -> sunion(regionProvider, setKeys));
- return RedisResponse.array(resultSet, true);
- }
-
- return doActualSetOperation(command, context, setKeys);
+ return context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys),
+ () -> performCommand(regionProvider, setKeys));
}
- private RedisResponse doActualSetOperation(Command command,
ExecutionHandlerContext context,
- List<RedisKey> setKeys) {
- if (isStorage()) {
- RedisKey destination = command.getKey();
- int storeCount = doStoreSetOp(command.getCommandType(), context,
destination, setKeys);
- return RedisResponse.integer(storeCount);
- }
-
- Set<byte[]> resultSet = null;
- for (RedisKey key : setKeys) {
- Set<byte[]> keySet = context.setLockedExecute(key, true,
- set -> new MemberSet(set.smembers()));
- if (resultSet == null) {
- resultSet = keySet;
- } else if (doSetOp(resultSet, keySet)) {
- break;
- }
- }
-
- return RedisResponse.array(resultSet, true);
- }
+ protected abstract RedisResponse performCommand(RegionProvider
regionProvider,
+ List<RedisKey> setKeys);
+}
- protected int doStoreSetOp(RedisCommandType setOp, ExecutionHandlerContext
context,
- RedisKey destination,
- List<RedisKey> setKeys) {
- List<MemberSet> nonDestinationSets = fetchSets(context, setKeys,
destination);
- return context.lockedExecute(destination,
- () -> doStoreSetOpWhileLocked(setOp, context, destination,
nonDestinationSets));
+// Used for set operations without store that return an array
+abstract class SetOpArrayResult extends SetOpExecutor {
+ protected RedisResponse performCommand(RegionProvider regionProvider,
List<RedisKey> setKeys) {
+ return RedisResponse.array(getResult(regionProvider, setKeys), true);
}
- private int doStoreSetOpWhileLocked(RedisCommandType setOp,
ExecutionHandlerContext context,
- RedisKey destination,
- List<MemberSet> nonDestinationSets) {
- Set<byte[]> result =
- computeStoreSetOp(setOp, nonDestinationSets, context, destination);
- if (result.isEmpty()) {
- context.getRegion().remove(destination);
- return 0;
- } else {
- context.getRegion().put(destination, new RedisSet(result));
- return result.size();
- }
- }
+ protected abstract Set<byte[]> getResult(RegionProvider regionProvider,
List<RedisKey> setKeys);
+}
- private Set<byte[]> computeStoreSetOp(RedisCommandType setOp,
List<MemberSet> nonDestinationSets,
- ExecutionHandlerContext context, RedisKey destination) {
- MemberSet result = null;
- if (nonDestinationSets.isEmpty()) {
- return emptySet();
- }
- for (MemberSet set : nonDestinationSets) {
- if (set == null) {
- RedisSet redisSet = context.getRedisSet(destination, false);
- set = new MemberSet(redisSet.smembers());
- }
- if (result == null) {
- result = set;
- } else {
- switch (setOp) {
- case SUNIONSTORE:
- result.addAll(set);
- break;
- case SINTERSTORE:
- result.retainAll(set);
- break;
- default:
- throw new IllegalStateException(
- "expected a set store command but found: " + setOp);
- }
- }
- }
- return result;
- }
- /**
- * Gets the set data for the given keys, excluding the destination if it was
in setKeys.
- * The result will have an element for each corresponding key and a null
element if
- * the corresponding key is the destination.
- * This is all done outside the striped executor to prevent a deadlock.
- */
- private List<MemberSet> fetchSets(ExecutionHandlerContext context,
List<RedisKey> setKeys,
- RedisKey destination) {
- List<MemberSet> result = new ArrayList<>(setKeys.size());
- for (RedisKey key : setKeys) {
- if (key.equals(destination)) {
- result.add(null);
- } else {
- result.add(context.setLockedExecute(key, false,
- set -> new MemberSet(set.smembers())));
- }
- }
- return result;
+// Used for set operations with store that return an integer
+abstract class SetOpIntegerResult extends SetOpExecutor {
+ protected RedisResponse performCommand(RegionProvider regionProvider,
List<RedisKey> setKeys) {
+ RedisKey destKey = setKeys.remove(0);
+ return RedisResponse.integer(getResult(regionProvider, setKeys, destKey));
}
- /**
- * @return true if no further calls of doSetOp are needed
- */
- protected abstract boolean doSetOp(Set<byte[]> resultSet, Set<byte[]>
nextSet);
-
- protected abstract boolean isStorage();
-
+ protected abstract int getResult(RegionProvider regionProvider,
List<RedisKey> setKeys,
+ RedisKey destKey);
}
diff --git
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
index f9a2e1a..04654da 100644
---
a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
+++
b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
@@ -92,17 +92,7 @@ public class RedisSet extends AbstractRedisData {
return 1;
}
- public static Set<byte[]> sunion(RegionProvider regionProvider,
List<RedisKey> keys) {
- return calculateUnion(regionProvider, keys, true);
- }
-
- public static int sunionstore(RegionProvider regionProvider, RedisKey
destinationKey,
- List<RedisKey> keys) {
- MemberSet union = calculateUnion(regionProvider, keys, false);
- return setOpStoreResult(regionProvider, destinationKey, union);
- }
-
- private static MemberSet calculateUnion(RegionProvider regionProvider,
List<RedisKey> keys,
+ public static MemberSet sunion(RegionProvider regionProvider, List<RedisKey>
keys,
boolean updateStats) {
MemberSet union = new MemberSet();
for (RedisKey key : keys) {
@@ -112,17 +102,13 @@ public class RedisSet extends AbstractRedisData {
return union;
}
- public static Set<byte[]> sdiff(RegionProvider regionProvider,
List<RedisKey> keys) {
- return calculateDiff(regionProvider, keys, true);
- }
-
- public static int sdiffstore(RegionProvider regionProvider, RedisKey
destinationKey,
- List<RedisKey> keys) {
- MemberSet diff = calculateDiff(regionProvider, keys, false);
- return setOpStoreResult(regionProvider, destinationKey, diff);
+ public static int sunionstore(RegionProvider regionProvider, List<RedisKey>
keys,
+ RedisKey destinationKey) {
+ MemberSet union = sunion(regionProvider, keys, false);
+ return setOpStoreResult(regionProvider, destinationKey, union);
}
- private static MemberSet calculateDiff(RegionProvider regionProvider,
List<RedisKey> keys,
+ public static MemberSet sdiff(RegionProvider regionProvider, List<RedisKey>
keys,
boolean updateStats) {
RedisSet firstSet = regionProvider.getTypedRedisData(REDIS_SET,
keys.get(0), updateStats);
MemberSet diff = new MemberSet(firstSet.members);
@@ -136,14 +122,21 @@ public class RedisSet extends AbstractRedisData {
return diff;
}
- public static Set<byte[]> sinter(RegionProvider regionProvider,
List<RedisKey> keys) {
- List<RedisSet> sets = createRedisSetList(keys, regionProvider);
+ public static int sdiffstore(RegionProvider regionProvider,
+ List<RedisKey> keys, RedisKey destinationKey) {
+ MemberSet diff = sdiff(regionProvider, keys, false);
+ return setOpStoreResult(regionProvider, destinationKey, diff);
+ }
+
+ public static MemberSet sinter(RegionProvider regionProvider, List<RedisKey>
keys,
+ boolean updateStats) {
+ List<RedisSet> sets = createRedisSetList(keys, regionProvider,
updateStats);
final RedisSet smallestSet = findSmallest(sets);
+ MemberSet result = new MemberSet(smallestSet.scard());
if (smallestSet.scard() == 0) {
- return Collections.emptySet();
+ return result;
}
- MemberSet result = new MemberSet(smallestSet.scard());
for (byte[] member : smallestSet.members) {
boolean addToSet = true;
for (RedisSet otherSet : sets) {
@@ -175,15 +168,21 @@ public class RedisSet extends AbstractRedisData {
}
private static List<RedisSet> createRedisSetList(List<RedisKey> keys,
- RegionProvider regionProvider) {
+ RegionProvider regionProvider, boolean updateStats) {
List<RedisSet> sets = new ArrayList<>(keys.size());
for (RedisKey key : keys) {
- RedisSet redisSet = regionProvider.getTypedRedisData(REDIS_SET, key,
true);
+ RedisSet redisSet = regionProvider.getTypedRedisData(REDIS_SET, key,
updateStats);
sets.add(redisSet);
}
return sets;
}
+ public static int sinterstore(RegionProvider regionProvider,
+ List<RedisKey> keys, RedisKey destinationKey) {
+ MemberSet inter = sinter(regionProvider, keys, false);
+ return setOpStoreResult(regionProvider, destinationKey, inter);
+ }
+
@VisibleForTesting
static int setOpStoreResult(RegionProvider regionProvider, RedisKey
destinationKey,
MemberSet result) {
