You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2022/01/28 18:54:06 UTC
[geode] 03/09: GEODE-9837: SUNIONSTORE Command Support (#7284)
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 0ade491c33f55176d7ea7fa9d07fa028e9857c90
Author: Kris10 <ko...@vmware.com>
AuthorDate: Thu Jan 20 14:11:41 2022 -0800
GEODE-9837: SUNIONSTORE Command Support (#7284)
(cherry picked from commit 3a36962edfcd30aa3afa3a50813c63bfc155f699)
---
.../tools_modules/geode_for_redis.html.md.erb | 1 +
geode-for-redis/README.md | 1 +
.../set/SUnionStoreNativeRedisAcceptanceTest.java | 36 +++
.../server/AbstractHitsMissesIntegrationTest.java | 12 +-
.../set/AbstractSUnionIntegrationTest.java | 141 ------------
.../set/AbstractSUnionStoreIntegrationTest.java | 242 +++++++++++++++++++++
.../executor/set/SUnionStoreIntegrationTest.java | 31 +++
.../redis/internal/commands/RedisCommandType.java | 4 +-
.../commands/executor/set/SetOpExecutor.java | 16 +-
.../apache/geode/redis/internal/data/RedisSet.java | 18 +-
10 files changed, 344 insertions(+), 158 deletions(-)
diff --git a/geode-docs/tools_modules/geode_for_redis.html.md.erb b/geode-docs/tools_modules/geode_for_redis.html.md.erb
index 9451dc7..748ddf8 100644
--- a/geode-docs/tools_modules/geode_for_redis.html.md.erb
+++ b/geode-docs/tools_modules/geode_for_redis.html.md.erb
@@ -125,6 +125,7 @@ If the server is functioning properly, you should see a response of `PONG`.
- STRLEN <br/>
- SUBSCRIBE <br/>
- SUNION <br/>
+ - SUNIONSTORE <br/>
- TTL <br/>
- TYPE <br/>
- UNSUBSCRIBE <br/>
diff --git a/geode-for-redis/README.md b/geode-for-redis/README.md
index 2162229..fa392d9 100644
--- a/geode-for-redis/README.md
+++ b/geode-for-redis/README.md
@@ -215,6 +215,7 @@ Geode for Redis implements a subset of the full Redis command set.
- STRLEN
- SUBSCRIBE
- SUNION
+- SUNIONSTORE
- TTL
- TYPE
- UNSUBSCRIBE
diff --git a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreNativeRedisAcceptanceTest.java b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreNativeRedisAcceptanceTest.java
new file mode 100644
index 0000000..8f63afa
--- /dev/null
+++ b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreNativeRedisAcceptanceTest.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.commands.executor.set;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.NativeRedisClusterTestRule;
+
+public class SUnionStoreNativeRedisAcceptanceTest extends AbstractSUnionStoreIntegrationTest {
+
+ @ClassRule
+ public static NativeRedisClusterTestRule redis = new NativeRedisClusterTestRule();
+
+ @Override
+ public int getPort() {
+ return redis.getExposedPorts().get(0);
+ }
+
+ @Override
+ 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 1c9a0da..5923e07 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
@@ -419,6 +419,12 @@ public abstract class AbstractHitsMissesIntegrationTest implements RedisIntegrat
runMultiKeyCommandAndAssertHitsAndMisses(SET_KEY, (k1, k2) -> jedis.sunion(k1, k2));
}
+ @Test
+ public void testSunionstore() {
+ runMultiKeyCommandAndAssertNoStatUpdates(SET_KEY,
+ (k1, k2) -> jedis.sunionstore(HASHTAG + "dest", k1, k2));
+ }
+
/************* Hash related commands *************/
@Test
public void testHset() {
@@ -565,12 +571,6 @@ public abstract class AbstractHitsMissesIntegrationTest implements RedisIntegrat
(k1, k2) -> jedis.sinterstore(HASHTAG + "dest", k1, k2));
}
- @Test
- public void testSunionstore() {
- runMultiKeyCommandAndAssertNoStatUpdates(SET_KEY,
- (k1, k2) -> jedis.sunionstore(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/AbstractSUnionIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionIntegrationTest.java
index aa64424..7e6ac15 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionIntegrationTest.java
@@ -22,9 +22,6 @@ import static org.apache.geode.test.dunit.rules.RedisClusterStartupRule.REDIS_CL
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
@@ -189,142 +186,4 @@ public abstract class AbstractSUnionIntegrationTest implements RedisIntegrationT
jedis.sadd(SET_KEY_2, unionMembers);
});
}
-
- @Test
- public void testSUnionStore() {
- 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.sunionstore("{tag1}result", "{tag1}set1", "{tag1}set2");
- assertThat(resultSize).isEqualTo(7);
-
- Set<String> resultSet = jedis.smembers("{tag1}result");
- String[] expected =
- new String[] {"pear", "apple", "plum", "orange", "peach", "microsoft", "linux"};
- assertThat(resultSet).containsExactlyInAnyOrder(expected);
-
- Long notEmptyResultSize =
- jedis.sunionstore("{tag1}notempty", "{tag1}nonexistent", "{tag1}set1");
- Set<String> notEmptyResultSet = jedis.smembers("{tag1}notempty");
- assertThat(notEmptyResultSize).isEqualTo(firstSet.length);
- assertThat(notEmptyResultSet).containsExactlyInAnyOrder(firstSet);
-
- jedis.sadd("{tag1}newEmpty", "born2die");
- jedis.srem("{tag1}newEmpty", "born2die");
- Long newNotEmptySize =
- jedis.sunionstore("{tag1}newNotEmpty", "{tag1}set2", "{tag1}newEmpty");
- Set<String> newNotEmptySet = jedis.smembers("{tag1}newNotEmpty");
- assertThat(newNotEmptySize).isEqualTo(secondSet.length);
- assertThat(newNotEmptySet).containsExactlyInAnyOrder(secondSet);
- }
-
- @Test
- public void testSUnionStore_withNonExistentKeys() {
- String[] firstSet = new String[] {"pear", "apple", "plum", "orange", "peach"};
- jedis.sadd("{tag1}set1", firstSet);
-
- Long resultSize =
- jedis.sunionstore("{tag1}set1", "{tag1}nonExistent1", "{tag1}nonExistent2");
- assertThat(resultSize).isEqualTo(0);
- assertThat(jedis.exists("{tag1}set1")).isFalse();
- }
-
- @Test
- public void testSUnionStore_withNonExistentKeys_andNonSetTarget() {
- jedis.set("{tag1}string1", "stringValue");
-
- Long resultSize =
- jedis.sunionstore("{tag1}string1", "{tag1}nonExistent1", "{tag1}nonExistent2");
- assertThat(resultSize).isEqualTo(0);
- assertThat(jedis.exists("{tag1}set1")).isFalse();
- }
-
- @Test
- public void testSUnionStore_withNonSetKey() {
- String[] firstSet = new String[] {"pear", "apple", "plum", "orange", "peach"};
- jedis.sadd("{tag1}set1", firstSet);
- jedis.set("{tag1}string1", "value1");
-
- assertThatThrownBy(() -> jedis.sunionstore("{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 testConcurrentSUnionStore() 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("{tag1}master", masterSet.toArray(new String[] {}));
-
- for (int i = 0; i < ENTRIES; i++) {
- jedis.sadd("{tag1}set-" + i, otherSets.get(i).toArray(new String[] {}));
- }
-
- Runnable runnable1 = () -> {
- for (int i = 0; i < ENTRIES; i++) {
- jedis.sunionstore("{tag1}master", "{tag1}master", "{tag1}set-" + i);
- Thread.yield();
- }
- };
-
- Runnable runnable2 = () -> {
- for (int i = 0; i < ENTRIES; i++) {
- jedis.sunionstore("{tag1}master", "{tag1}master", "{tag1}set-" + i);
- Thread.yield();
- }
- };
-
- Thread thread1 = new Thread(runnable1);
- Thread thread2 = new Thread(runnable2);
-
- thread1.start();
- thread2.start();
- thread1.join();
- thread2.join();
-
- otherSets.forEach(masterSet::addAll);
-
- assertThat(jedis.smembers("{tag1}master").toArray())
- .containsExactlyInAnyOrder(masterSet.toArray());
- }
-
- @Test
- public void doesNotThrowExceptions_whenConcurrentSaddAndSunionExecute() {
- final int ENTRIES = 1000;
- Set<String> set1 = new HashSet<>();
- Set<String> set2 = new HashSet<>();
- for (int i = 0; i < ENTRIES; i++) {
- set1.add("value-1-" + i);
- set2.add("value-2-" + i);
- }
-
- jedis.sadd("{player1}key1", set1.toArray(new String[] {}));
- jedis.sadd("{player1}key2", set2.toArray(new String[] {}));
-
- new ConcurrentLoopingThreads(ENTRIES,
- i -> jedis.sunion("{player1}key1", "{player1}key2"),
- i -> jedis.sadd("{player1}key1", "newValue-1-" + i),
- i -> jedis.sadd("{player1}key2", "newValue-2-" + i))
- .runInLockstep();
- }
-
}
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
new file mode 100644
index 0000000..7d0ee9b
--- /dev/null
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
@@ -0,0 +1,242 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+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_DIFFERENT_SLOTS;
+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 java.util.concurrent.atomic.AtomicLong;
+
+import org.junit.After;
+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;
+
+public abstract class AbstractSUnionStoreIntegrationTest implements RedisIntegrationTest {
+ private JedisCluster jedis;
+ private static final String DESTINATION_KEY = "{tag1}destinationKey";
+ private static final String[] DESTINATION_MEMBERS =
+ {"six", "seven", "eight", "nine", "ten", "one", "two"};
+ private static final String SET_KEY_1 = "{tag1}setKey1";
+ private static final String[] SET_MEMBERS_1 = {"one", "two", "three", "four", "five"};
+ private static final String NON_EXISTENT_SET = "{tag1}nonExistentSet";
+ private static final String SET_KEY_2 = "{tag1}setKey2";
+
+ @Before
+ public void setUp() {
+ jedis = new JedisCluster(new HostAndPort(BIND_ADDRESS, getPort()), REDIS_CLIENT_TIMEOUT);
+ }
+
+ @After
+ public void tearDown() {
+ flushAll();
+ jedis.close();
+ }
+
+ @Test
+ public void sunionstore_givenTooFewArguments_returnsError() {
+ assertAtLeastNArgs(jedis, Protocol.Command.SUNION, 1);
+ }
+
+ @Test
+ public void sunionstore_withExistentSet_returnsUnionSize_storesUnion() {
+ jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
+ assertThat(jedis.sunionstore(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);
+ assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
+ }
+
+ @Test
+ public void sunionstore_withOneExistentAndOneNonExistentSet_returnsUnionSize_storesUnion() {
+ 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);
+ }
+
+ @Test
+ public void sunionstore_withOneNonExistentAndOneExistentSet_returnsUnionSize_storesUnion() {
+ 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);
+ }
+
+ @Test
+ public void sunionstore_withNonOverlappingSets_returnsUnionSize_storesUnion() {
+ 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);
+
+ }
+
+ @Test
+ public void sunionstore_withSomeSharedValues_returnsUnionSize_storesUnion() {
+ 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);
+ assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(result);
+ }
+
+ @Test
+ public void sunionstore_withAllSharedValues_returnsUnionSize_storesUnion() {
+ 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))
+ .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"))
+ .isEqualTo(0);
+ assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
+ }
+
+ @Test
+ public void sunionstore_withExistentSet_returnsUnionSize_destKeyOverwrittenWithUnion() {
+ 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.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(SET_MEMBERS_1);
+ }
+
+ @Test
+ public void sunionstore_withNonExistentSet_returnsZero_destKeyIsDeleted() {
+ jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
+ assertThat(jedis.sunionstore(DESTINATION_KEY, NON_EXISTENT_SET)).isEqualTo(0);
+ assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
+ }
+
+ @Test
+ public void sunionstore_withNonSetDestKey_withExistentSet_returnsUnionSize_destKeyOverwrittenWithUnion() {
+ 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.smembers(stringKey)).containsExactlyInAnyOrder(SET_MEMBERS_1);
+ }
+
+ @Test
+ public void sunionstore_withNonSetDestKey_withNonExistentSet_returnsZero_destKeyIsDeleted() {
+ String stringKey = "{tag1}ding";
+ jedis.set(stringKey, "dong");
+
+ assertThat(jedis.sunionstore(stringKey, NON_EXISTENT_SET)).isEqualTo(0);
+ assertThat(jedis.exists(DESTINATION_KEY)).isFalse();
+ }
+
+ @Test
+ public void sunionstore_withNonExistentDest_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))
+ .hasMessageContaining(ERROR_WRONG_TYPE);
+ }
+
+ @Test
+ public void sunionstore_withExistentDest_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))
+ .hasMessageContaining(ERROR_WRONG_TYPE);
+ }
+
+ @Test
+ public void sunionstore_withNonExistentDest_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))
+ .hasMessageContaining(ERROR_WRONG_TYPE);
+ }
+
+ @Test
+ public void sunionstore_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.sunionstore(DESTINATION_KEY, SET_KEY_1, setKeyDifferentSlot))
+ .hasMessageContaining(ERROR_DIFFERENT_SLOTS);
+ }
+
+ @Test
+ public void ensureSetConsistency_whenRunningConcurrently() {
+ String[] secondSetMembers = new String[] {"one", "two", "linux", "peach"};
+ 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);
+ 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);
+ });
+ }
+}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreIntegrationTest.java
new file mode 100644
index 0000000..ff365ff
--- /dev/null
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SUnionStoreIntegrationTest.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.redis.internal.commands.executor.set;
+
+import org.junit.ClassRule;
+
+import org.apache.geode.redis.GeodeRedisServerRule;
+
+public class SUnionStoreIntegrationTest extends AbstractSUnionStoreIntegrationTest {
+
+ @ClassRule
+ public static GeodeRedisServerRule server = new GeodeRedisServerRule();
+
+ @Override
+ 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 3a461de..ef1d703 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
@@ -253,6 +253,8 @@ public enum RedisCommandType {
SREM(new SRemExecutor(), SUPPORTED, new Parameter().min(3).flags(WRITE, FAST)),
SUNION(new SUnionExecutor(), SUPPORTED,
new Parameter().min(2).lastKey(-1).flags(READONLY, SORT_FOR_SCRIPT)),
+ SUNIONSTORE(new SUnionStoreExecutor(), SUPPORTED,
+ new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)),
/************ Sorted Sets **************/
@@ -357,8 +359,6 @@ public enum RedisCommandType {
new Parameter().min(2).flags(READONLY, RANDOM)),
SSCAN(new SScanExecutor(), UNSUPPORTED, new Parameter().min(3).flags(READONLY, RANDOM),
new Parameter().odd(ERROR_SYNTAX).firstKey(0)),
- SUNIONSTORE(new SUnionStoreExecutor(), UNSUPPORTED,
- new Parameter().min(3).lastKey(-1).flags(WRITE, DENYOOM)),
/*************** Server ****************/
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 e57a0d0..b83c8e2 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
@@ -19,6 +19,7 @@ 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;
@@ -57,8 +58,7 @@ public abstract class SetOpExecutor implements CommandExecutor {
}
/*
- * SINTERSTORE, SUNIONSTORE currently use the else part of the code
- * for their implementation.
+ * 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
@@ -74,11 +74,21 @@ public abstract class SetOpExecutor implements CommandExecutor {
Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys),
() -> sdiff(regionProvider, setKeys));
return RedisResponse.array(resultSet, true);
+
} else if (command.isOfType(RedisCommandType.SINTER)) {
Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys),
() -> sinter(regionProvider, setKeys));
return RedisResponse.array(resultSet, true);
- } else if (command.isOfType(RedisCommandType.SUNION)) {
+
+ } else if (command.isOfType(RedisCommandType.SUNION)
+ || command.isOfType(RedisCommandType.SUNIONSTORE)) {
+ if (isStorage()) {
+ RedisKey destinationKey = command.getKey();
+ int resultSize = context.lockedExecute(destinationKey, new ArrayList<>(setKeys),
+ () -> sunionstore(regionProvider, destinationKey, setKeys));
+ return RedisResponse.integer(resultSize);
+ }
+
Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys),
() -> sunion(regionProvider, setKeys));
return RedisResponse.array(resultSet, true);
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 cf162ec..f17d909 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
@@ -96,6 +96,12 @@ public class RedisSet extends AbstractRedisData {
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,
boolean updateStats) {
MemberSet union = new MemberSet();
@@ -180,10 +186,10 @@ public class RedisSet extends AbstractRedisData {
@VisibleForTesting
static int setOpStoreResult(RegionProvider regionProvider, RedisKey destinationKey,
- MemberSet diff) {
+ MemberSet result) {
RedisSet destinationSet =
regionProvider.getTypedRedisDataElseRemove(REDIS_SET, destinationKey, false);
- if (diff.isEmpty()) {
+ if (result.isEmpty()) {
if (destinationSet != null) {
regionProvider.getDataRegion().remove(destinationKey);
}
@@ -192,14 +198,14 @@ public class RedisSet extends AbstractRedisData {
if (destinationSet != null) {
destinationSet.persistNoDelta();
- destinationSet.members = diff;
+ destinationSet.members = result;
destinationSet.storeChanges(regionProvider.getDataRegion(), destinationKey,
- new ReplaceByteArrays(diff));
+ new ReplaceByteArrays(result));
} else {
- regionProvider.getDataRegion().put(destinationKey, new RedisSet(diff));
+ regionProvider.getDataRegion().put(destinationKey, new RedisSet(result));
}
- return diff.size();
+ return result.size();
}
public Pair<BigInteger, List<Object>> sscan(GlobPattern matchPattern, int count,