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:03 UTC
[geode] branch support/1.15 updated (0271f6a -> 0331eee)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a change to branch support/1.15
in repository https://gitbox.apache.org/repos/asf/geode.git.
from 0271f6a GEODE-8616: Refactoring the test to remove deprecated APIs (#7301)
new 08d2006 Revert "GEODE-9883 Update Geode for Redis docs file (#7274)"
new d1d5e1e GEODE-9977: Remove defunct RedisCommandSupportLevel.INTERNAL type (#7283)
new 0ade491 GEODE-9837: SUNIONSTORE Command Support (#7284)
new d456338 GEODE-9834: SRANDMEMBER Command Support (#7228)
new 9166cb4 GEODE-9885: Handle duplicated appends in Redis StringsDUnitTest (#7290)
new 731fcd0 update redis svg to use new module name (#7288)
new ebd17de GEODE-9922: Move Redis cross-slot checking to RegionProvider (#7295)
new 9b8a768 GEODE-9883 Update Geode for Redis docs file (#7274)
new 0331eee GEODE-9830: SINTERSTORE Command Support (#7302)
The 9 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
geode-docs/images_svg/geode_for_redis.svg | 206 +++++++++++--
geode-for-redis/README.md | 3 +
...a => SInterStoreNativeRedisAcceptanceTest.java} | 2 +-
...a => SRandMemberNativeRedisAcceptanceTest.java} | 2 +-
...a => SUnionStoreNativeRedisAcceptanceTest.java} | 2 +-
.../resources/0001-configure-redis-tests.patch | 340 ++++++++++++++++++++-
.../commands/executor/string/StringsDUnitTest.java | 71 +++--
.../key/AbstractRenameIntegrationTest.java | 15 +-
.../key/AbstractRenameNXIntegrationTest.java | 12 +
.../server/AbstractHitsMissesIntegrationTest.java | 34 +--
.../executor/set/AbstractSDiffIntegrationTest.java | 15 +-
.../set/AbstractSDiffStoreIntegrationTest.java | 15 +-
.../set/AbstractSInterIntegrationTest.java | 148 +--------
.../set/AbstractSInterStoreIntegrationTest.java | 237 ++++++++++++++
.../executor/set/AbstractSMoveIntegrationTest.java | 165 +++++-----
.../set/AbstractSRandMemberIntegrationTest.java | 165 ++++++++++
.../set/AbstractSUnionIntegrationTest.java | 151 +--------
.../set/AbstractSUnionStoreIntegrationTest.java | 244 +++++++++++++++
.../executor/set/AbstractSetsIntegrationTest.java | 49 ---
...onTest.java => SInterStoreIntegrationTest.java} | 2 +-
...onTest.java => SRandMemberIntegrationTest.java} | 2 +-
...onTest.java => SUnionStoreIntegrationTest.java} | 2 +-
.../string/AbstractMSetIntegrationTest.java | 3 +-
.../string/AbstractMSetNXIntegrationTest.java | 3 +-
.../apache/geode/codeAnalysis/excludedClasses.txt | 1 +
.../geode/redis/internal/RedisConstants.java | 2 -
.../commands/RedisCommandSupportLevel.java | 3 +-
.../redis/internal/commands/RedisCommandType.java | 18 +-
.../executor/key/AbstractRenameExecutor.java | 5 -
.../executor/server/COMMANDCommandExecutor.java | 3 +-
.../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/SMoveExecutor.java | 9 -
.../commands/executor/set/SRandMemberExecutor.java | 50 +--
.../commands/executor/set/SUnionExecutor.java | 16 +-
.../commands/executor/set/SUnionStoreExecutor.java | 13 +-
.../commands/executor/set/SetOpExecutor.java | 167 ++--------
...dMemberExecutor.java => SetRandomExecutor.java} | 52 ++--
.../executor/sortedset/ZInterStoreExecutor.java | 2 +-
.../executor/sortedset/ZStoreExecutor.java | 14 +-
.../executor/sortedset/ZUnionStoreExecutor.java | 2 +-
.../executor/string/AbstractMSetExecutor.java | 8 +-
.../geode/redis/internal/data/NullRedisSet.java | 2 +-
.../geode/redis/internal/data/NullRedisString.java | 5 -
...Exception.java => RedisCrossSlotException.java} | 12 +-
.../apache/geode/redis/internal/data/RedisSet.java | 142 +++++----
.../geode/redis/internal/data/RedisString.java | 6 +-
.../SizeableObjectOpenCustomHashSet.java | 8 +
.../internal/netty/ExecutionHandlerContext.java | 3 +
.../redis/internal/services/RegionProvider.java | 20 +-
.../internal/services/RegionProviderTest.java | 74 +++++
53 files changed, 1710 insertions(+), 875 deletions(-)
copy geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/{SMoveNativeRedisAcceptanceTest.java => SInterStoreNativeRedisAcceptanceTest.java} (92%)
copy geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/{SCardNativeRedisAcceptanceTest.java => SRandMemberNativeRedisAcceptanceTest.java} (92%)
copy geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/{SIsMemberNativeRedisAcceptanceTest.java => SUnionStoreNativeRedisAcceptanceTest.java} (92%)
mode change 100755 => 100644
create mode 100644 geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterStoreIntegrationTest.java
create mode 100644 geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSRandMemberIntegrationTest.java
create mode 100644 geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSUnionStoreIntegrationTest.java
copy geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/{SIsMemberIntegrationTest.java => SInterStoreIntegrationTest.java} (92%)
copy geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/{SCardIntegrationTest.java => SRandMemberIntegrationTest.java} (92%)
copy geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/{SIsMemberIntegrationTest.java => SUnionStoreIntegrationTest.java} (92%)
mode change 100755 => 100644
copy geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/{SRandMemberExecutor.java => SetRandomExecutor.java} (59%)
mode change 100755 => 100644
copy geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/{RedisKeyExistsException.java => RedisCrossSlotException.java} (76%)
mode change 100755 => 100644
create mode 100644 geode-for-redis/src/test/java/org/apache/geode/redis/internal/services/RegionProviderTest.java
[geode] 09/09: GEODE-9830: SINTERSTORE Command Support (#7302)
Posted by je...@apache.org.
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 <ko...@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) {
[geode] 04/09: GEODE-9834: SRANDMEMBER Command Support (#7228)
Posted by je...@apache.org.
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 d4563383d0e8dbbd8f67f28d3367664c5c8f518b
Author: Kris10 <ko...@vmware.com>
AuthorDate: Fri Jan 21 10:14:07 2022 -0800
GEODE-9834: SRANDMEMBER Command Support (#7228)
(cherry picked from commit a53c6da8dad75c8953de7e7ceb4bbfa545e5f405)
---
.../tools_modules/geode_for_redis.html.md.erb | 1 +
geode-for-redis/README.md | 1 +
.../set/SRandMemberNativeRedisAcceptanceTest.java | 34 +++++
.../server/AbstractHitsMissesIntegrationTest.java | 10 +-
.../set/AbstractSRandMemberIntegrationTest.java | 165 +++++++++++++++++++++
.../executor/set/AbstractSetsIntegrationTest.java | 49 ------
.../executor/set/SRandMemberIntegrationTest.java | 29 ++++
.../redis/internal/commands/RedisCommandType.java | 4 +-
.../commands/executor/set/SRandMemberExecutor.java | 50 +------
...dMemberExecutor.java => SetRandomExecutor.java} | 52 ++++---
.../geode/redis/internal/data/NullRedisSet.java | 2 +-
.../apache/geode/redis/internal/data/RedisSet.java | 85 ++++++-----
.../SizeableObjectOpenCustomHashSet.java | 8 +
13 files changed, 330 insertions(+), 160 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 748ddf8..475fc7d 100644
--- a/geode-docs/tools_modules/geode_for_redis.html.md.erb
+++ b/geode-docs/tools_modules/geode_for_redis.html.md.erb
@@ -121,6 +121,7 @@ If the server is functioning properly, you should see a response of `PONG`.
- SLOWLOG **[3]** <br/>
- SMEMBERS <br/>
- SMOVE <br/>
+ - SRANDMEMBER <br/>
- SREM <br/>
- STRLEN <br/>
- SUBSCRIBE <br/>
diff --git a/geode-for-redis/README.md b/geode-for-redis/README.md
index fa392d9..592b129 100644
--- a/geode-for-redis/README.md
+++ b/geode-for-redis/README.md
@@ -211,6 +211,7 @@ Geode for Redis implements a subset of the full Redis command set.
- SLOWLOG <sup>3</sup>
- SMEMBERS
- SMOVE
+- SRANDMEMBER
- SREM
- STRLEN
- SUBSCRIBE
diff --git a/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberNativeRedisAcceptanceTest.java b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberNativeRedisAcceptanceTest.java
new file mode 100644
index 0000000..dcab8c0
--- /dev/null
+++ b/geode-for-redis/src/acceptanceTest/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberNativeRedisAcceptanceTest.java
@@ -0,0 +1,34 @@
+/*
+ * 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 SRandMemberNativeRedisAcceptanceTest extends AbstractSRandMemberIntegrationTest {
+ @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 5923e07..23735d9 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
@@ -410,6 +410,11 @@ public abstract class AbstractHitsMissesIntegrationTest implements RedisIntegrat
}
@Test
+ public void testSrandmember() {
+ runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.srandmember(k));
+ }
+
+ @Test
public void testSrem() {
runCommandAndAssertNoStatUpdates(SET_KEY, k -> jedis.srem(k, "member"));
}
@@ -556,11 +561,6 @@ public abstract class AbstractHitsMissesIntegrationTest implements RedisIntegrat
}
@Test
- public void testSrandmember() {
- runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.srandmember(k));
- }
-
- @Test
public void testSscan() {
runCommandAndAssertHitsAndMisses(SET_KEY, k -> jedis.sscan(k, "0"));
}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSRandMemberIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSRandMemberIntegrationTest.java
new file mode 100644
index 0000000..821e68d
--- /dev/null
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSRandMemberIntegrationTest.java
@@ -0,0 +1,165 @@
+/*
+ * 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_NOT_INTEGER;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
+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.Arrays;
+import java.util.List;
+
+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.RedisIntegrationTest;
+
+public abstract class AbstractSRandMemberIntegrationTest implements RedisIntegrationTest {
+ private JedisCluster jedis;
+ private static final String NON_EXISTENT_SET_KEY = "{tag1}nonExistentSet";
+ private static final String SET_KEY = "{tag1}setKey";
+ private static final String[] SET_MEMBERS =
+ {"one", "two", "three", "four", "five", "six", "seven", "eight"};
+
+ @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 srandmemberTooFewArgs_returnsError() {
+ assertAtLeastNArgs(jedis, Protocol.Command.SRANDMEMBER, 1);
+ }
+
+ @Test
+ public void srandmemberTooManyArgs_returnsError() {
+ assertThatThrownBy(
+ () -> jedis.sendCommand(SET_KEY, Protocol.Command.SRANDMEMBER, SET_KEY, "5", "5"))
+ .hasMessageContaining(ERROR_SYNTAX);
+ }
+
+ @Test
+ public void srandmember_withInvalidCount_returnsError() {
+ assertThatThrownBy(() -> jedis.sendCommand(SET_KEY, Protocol.Command.SRANDMEMBER, SET_KEY, "b"))
+ .hasMessageContaining(ERROR_NOT_INTEGER);
+ }
+
+ @Test
+ public void srandmember_withoutCount_withNonExistentSet_returnsNull() {
+ assertThat(jedis.srandmember(NON_EXISTENT_SET_KEY)).isNull();
+ assertThat(jedis.exists(NON_EXISTENT_SET_KEY)).isFalse();
+ }
+
+ @Test
+ public void srandmember_withoutCount_withExistentSet_returnsOneMember() {
+ jedis.sadd(SET_KEY, SET_MEMBERS);
+ String result = jedis.srandmember(SET_KEY);
+ assertThat(result).isIn(Arrays.asList(SET_MEMBERS));
+ }
+
+ @Test
+ public void srandmember_withCountAsZero_withExistentSet_returnsEmptyList() {
+ jedis.sadd(SET_KEY, SET_MEMBERS);
+ assertThat(jedis.srandmember(SET_KEY, 0)).isEmpty();
+ }
+
+ @Test
+ public void srandmember_withNegativeCount_withExistentSet_returnsSubsetOfSet() {
+ jedis.sadd(SET_KEY, SET_MEMBERS);
+ int count = -20;
+
+ List<String> result = jedis.srandmember(SET_KEY, count);
+ assertThat(result.size()).isEqualTo(-count);
+ assertThat(result).isSubsetOf(SET_MEMBERS);
+ }
+
+ @Test
+ public void srandmember_withSmallCount_withNonExistentSet_returnsEmptyList() {
+ assertThat(jedis.srandmember(NON_EXISTENT_SET_KEY, 1)).isEmpty();
+ assertThat(jedis.exists(NON_EXISTENT_SET_KEY)).isFalse();
+ }
+
+ @Test
+ public void srandmember_withSmallCount_withExistentSet_returnsCorrectNumberOfMembers() {
+ jedis.sadd(SET_KEY, SET_MEMBERS);
+ int count = 2; // 2*3 < 8 Calls srandomUniqueListWithSmallCount
+
+ List<String> result = jedis.srandmember(SET_KEY, count);
+ assertThat(result.size()).isEqualTo(2);
+ assertThat(result).isSubsetOf(SET_MEMBERS);
+ assertThat(result).doesNotHaveDuplicates();
+ }
+
+ @Test
+ public void srandmember_withLargeCount_withExistentSet_returnsCorrectNumberOfMembers() {
+ jedis.sadd(SET_KEY, SET_MEMBERS);
+ int count = 6; // 6*3 > 8 Calls srandomUniqueListWithLargeCount
+
+ List<String> result = jedis.srandmember(SET_KEY, count);
+ assertThat(result.size()).isEqualTo(count);
+ assertThat(result).isSubsetOf(SET_MEMBERS);
+ assertThat(result).doesNotHaveDuplicates();
+ }
+
+ @Test
+ public void srandmember_withCountAsSetSize_withExistentSet_returnsAllMembers() {
+ jedis.sadd(SET_KEY, SET_MEMBERS);
+ int count = SET_MEMBERS.length;
+
+ assertThat(jedis.srandmember(SET_KEY, count)).containsExactlyInAnyOrder(SET_MEMBERS);
+ }
+
+ @Test
+ public void srandmember_withCountGreaterThanSetSize_withExistentSet_returnsAllMembers() {
+ jedis.sadd(SET_KEY, SET_MEMBERS);
+ assertThat(jedis.srandmember(SET_KEY, 20)).containsExactlyInAnyOrder(SET_MEMBERS);
+ }
+
+ @Test
+ public void srandmember_withoutCount_withWrongKeyType_returnsWrongTypeError() {
+ String key = "ding";
+ jedis.set(key, "dong");
+ assertThatThrownBy(() -> jedis.srandmember(key)).hasMessageContaining(ERROR_WRONG_TYPE);
+ }
+
+ @Test
+ public void srandmember_withCount_withWrongKeyType_returnsWrongTypeError() {
+ String key = "ding";
+ jedis.set(key, "dong");
+ assertThatThrownBy(() -> jedis.srandmember(key, 5)).hasMessageContaining(ERROR_WRONG_TYPE);
+ }
+
+ @Test
+ public void srandmember_withCountAsZero_withWrongKeyType_returnsWrongTypeError() {
+ String key = "ding";
+ jedis.set(key, "dong");
+ assertThatThrownBy(() -> jedis.srandmember(key, 0)).hasMessageContaining(ERROR_WRONG_TYPE);
+ }
+}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSetsIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSetsIntegrationTest.java
index 99c05c3..e40b5cb 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSetsIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSetsIntegrationTest.java
@@ -19,7 +19,6 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.HashSet;
-import java.util.List;
import java.util.Set;
import org.junit.After;
@@ -99,54 +98,6 @@ public abstract class AbstractSetsIntegrationTest implements RedisIntegrationTes
}
@Test
- public void srandmember_withStringFails() {
- jedis.set("string", "value");
- assertThatThrownBy(() -> jedis.srandmember("string")).hasMessageContaining("WRONGTYPE");
- }
-
- @Test
- public void srandmember_withNonExistentKeyReturnsNull() {
- assertThat(jedis.srandmember("non existent")).isNull();
- }
-
- @Test
- public void srandmemberCount_withNonExistentKeyReturnsEmptyArray() {
- assertThat(jedis.srandmember("non existent", 3)).isEmpty();
- }
-
- @Test
- public void srandmember_returnsOneMember() {
- jedis.sadd("key", "m1", "m2");
- String result = jedis.srandmember("key");
- assertThat(result).isIn("m1", "m2");
- }
-
- @Test
- public void srandmemberCount_returnsTwoUniqueMembers() {
- jedis.sadd("key", "m1", "m2", "m3");
- List<String> results = jedis.srandmember("key", 2);
- assertThat(results).hasSize(2);
- assertThat(results).containsAnyOf("m1", "m2", "m3");
- assertThat(results.get(0)).isNotEqualTo(results.get(1));
- }
-
- @Test
- public void srandmemberNegativeCount_returnsThreeMembers() {
- jedis.sadd("key", "m1", "m2", "m3");
- List<String> results = jedis.srandmember("key", -3);
- assertThat(results).hasSize(3);
- assertThat(results).containsAnyOf("m1", "m2", "m3");
- }
-
- @Test
- public void srandmemberNegativeCount_givenSmallSet_returnsThreeMembers() {
- jedis.sadd("key", "m1");
- List<String> results = jedis.srandmember("key", -3);
- assertThat(results).hasSize(3);
- assertThat(results).containsAnyOf("m1");
- }
-
- @Test
public void smembers_givenKeyNotProvided_returnsWrongNumberOfArgumentsError() {
assertThatThrownBy(() -> jedis.sendCommand("key", Protocol.Command.SMEMBERS))
.hasMessageContaining("ERR wrong number of arguments for 'smembers' command");
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberIntegrationTest.java
new file mode 100644
index 0000000..26766b6
--- /dev/null
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberIntegrationTest.java
@@ -0,0 +1,29 @@
+/*
+ * 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 SRandMemberIntegrationTest extends AbstractSRandMemberIntegrationTest {
+ @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 ef1d703..7887281 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
@@ -250,6 +250,8 @@ public enum RedisCommandType {
SMEMBERS(new SMembersExecutor(), SUPPORTED,
new Parameter().exact(2).flags(READONLY, SORT_FOR_SCRIPT)),
SMOVE(new SMoveExecutor(), SUPPORTED, new Parameter().exact(4).lastKey(2).flags(WRITE, FAST)),
+ SRANDMEMBER(new SRandMemberExecutor(), SUPPORTED,
+ new Parameter().min(2).max(3, ERROR_SYNTAX).flags(READONLY, RANDOM)),
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)),
@@ -355,8 +357,6 @@ public enum RedisCommandType {
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)),
- SRANDMEMBER(new SRandMemberExecutor(), UNSUPPORTED,
- 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)),
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberExecutor.java
index 86f60cf..a60d1c5 100755
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberExecutor.java
@@ -14,55 +14,13 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-import static org.apache.geode.redis.internal.netty.Coder.bytesToLong;
-import static org.apache.geode.redis.internal.netty.Coder.narrowLongToInt;
-
-import java.util.Collection;
import java.util.List;
-import org.apache.geode.redis.internal.commands.Command;
-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.netty.ExecutionHandlerContext;
-
-public class SRandMemberExecutor implements CommandExecutor {
-
- private static final String ERROR_NOT_NUMERIC = "The count provided must be numeric";
+import org.apache.geode.redis.internal.data.RedisSet;
+public class SRandMemberExecutor extends SetRandomExecutor {
@Override
- public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
- List<byte[]> commandElems = command.getProcessedCommand();
-
- RedisKey key = command.getKey();
-
- boolean countSpecified = false;
- int count;
-
- if (commandElems.size() > 2) {
- try {
- count = narrowLongToInt(bytesToLong(commandElems.get(2)));
- countSpecified = true;
- } catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_NOT_NUMERIC);
- }
- } else {
- count = 1;
- }
-
- if (count == 0) {
- return RedisResponse.emptyArray();
- }
-
- Collection<byte[]> results = context.setLockedExecute(key, true,
- set -> set.srandmember(count));
-
- if (countSpecified) {
- return RedisResponse.array(results, true);
- } else if (results.isEmpty()) {
- return RedisResponse.nil();
- } else {
- return RedisResponse.bulkString(results.iterator().next());
- }
+ protected List<byte[]> performCommand(RedisSet set, int count) {
+ return set.srandmember(count);
}
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetRandomExecutor.java
old mode 100755
new mode 100644
similarity index 59%
copy from geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberExecutor.java
copy to geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetRandomExecutor.java
index 86f60cf..56e4254
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SRandMemberExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SetRandomExecutor.java
@@ -14,55 +14,63 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
+import static org.apache.geode.redis.internal.data.NullRedisDataStructures.NULL_REDIS_SET;
+import static org.apache.geode.redis.internal.data.RedisDataType.REDIS_SET;
import static org.apache.geode.redis.internal.netty.Coder.bytesToLong;
import static org.apache.geode.redis.internal.netty.Coder.narrowLongToInt;
-import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import org.apache.geode.redis.internal.commands.Command;
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.netty.ExecutionHandlerContext;
+import org.apache.geode.redis.internal.services.RegionProvider;
-public class SRandMemberExecutor implements CommandExecutor {
-
- private static final String ERROR_NOT_NUMERIC = "The count provided must be numeric";
-
+public abstract class SetRandomExecutor implements CommandExecutor {
@Override
public RedisResponse executeCommand(Command command, ExecutionHandlerContext context) {
List<byte[]> commandElems = command.getProcessedCommand();
-
RedisKey key = command.getKey();
+ boolean hasCount = commandElems.size() == 3;
- boolean countSpecified = false;
int count;
-
- if (commandElems.size() > 2) {
+ if (hasCount) {
try {
count = narrowLongToInt(bytesToLong(commandElems.get(2)));
- countSpecified = true;
} catch (NumberFormatException e) {
- return RedisResponse.error(ERROR_NOT_NUMERIC);
+ return RedisResponse.error(ERROR_NOT_INTEGER);
}
} else {
count = 1;
}
- if (count == 0) {
- return RedisResponse.emptyArray();
- }
-
- Collection<byte[]> results = context.setLockedExecute(key, true,
- set -> set.srandmember(count));
-
- if (countSpecified) {
+ List<byte[]> results =
+ context.lockedExecute(key, () -> getResult(count, context.getRegionProvider(), key));
+ if (hasCount) {
return RedisResponse.array(results, true);
- } else if (results.isEmpty()) {
- return RedisResponse.nil();
} else {
- return RedisResponse.bulkString(results.iterator().next());
+ if (results.isEmpty()) {
+ return RedisResponse.nil();
+ } else {
+ return RedisResponse.bulkString(results.get(0));
+ }
+ }
+ }
+
+ private List<byte[]> getResult(int count, RegionProvider regionProvider, RedisKey key) {
+ RedisSet set =
+ regionProvider.getTypedRedisData(REDIS_SET, key, true);
+ if (count == 0 || set == NULL_REDIS_SET) {
+ return Collections.emptyList();
}
+
+ return performCommand(set, count);
}
+
+ protected abstract List<byte[]> performCommand(RedisSet set, int count);
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java
index 1ef7d6f..13dbed8 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSet.java
@@ -43,7 +43,7 @@ class NullRedisSet extends RedisSet {
}
@Override
- public Collection<byte[]> srandmember(int count) {
+ public List<byte[]> srandmember(int count) {
return emptyList();
}
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 f17d909..f9a2e1a 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
@@ -28,6 +28,7 @@ import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.Random;
@@ -52,7 +53,6 @@ import org.apache.geode.redis.internal.services.RegionProvider;
public class RedisSet extends AbstractRedisData {
protected static final int REDIS_SET_OVERHEAD = memoryOverhead(RedisSet.class);
-
private MemberSet members;
public RedisSet(Collection<byte[]> members) {
@@ -275,49 +275,64 @@ public class RedisSet extends AbstractRedisData {
return popped;
}
- public Collection<byte[]> srandmember(int count) {
- int membersSize = members.size();
- boolean duplicatesAllowed = count < 0;
- if (duplicatesAllowed) {
- count = -count;
+ public List<byte[]> srandmember(int count) {
+ List<byte[]> result = new ArrayList<>();
+ int randMethodRatio = 3;
+ if (count < 0) {
+ srandomDuplicateList(-count, result);
+ } else if (count * randMethodRatio < members.size()) {
+ // Count is small enough to add random elements to result
+ srandomUniqueListWithSmallCount(count, result);
+ } else {
+ // Count either equal or greater to member size or close to the member size
+ srandomUniqueListWithLargeCount(count, result);
}
+ return result;
+ }
- if (!duplicatesAllowed && membersSize <= count && count != 1) {
- return new ArrayList<>(members);
+ private void srandomDuplicateList(int count, List<byte[]> result) {
+ Random rand = new Random();
+ while (result.size() != count) {
+ int randIndex = rand.nextInt(members.getBackingArrayLength());
+ byte[] member = members.getFromBackingArray(randIndex);
+ if (member != null) {
+ result.add(member);
+ }
}
+ }
+ private void srandomUniqueListWithSmallCount(int count, List<byte[]> result) {
Random rand = new Random();
+ Set<Integer> indexesUsed = new HashSet<>();
- // TODO: this could be optimized to take advantage of MemberSet
- // storing its data in an array. We probably don't need to copy it
- // into another array here.
- byte[][] entries = members.toArray(new byte[membersSize][]);
-
- if (count == 1) {
- byte[] randEntry = entries[rand.nextInt(entries.length)];
- // TODO: Now that the result is no longer serialized this could use singleton.
- // Note using ArrayList because Collections.singleton has serialization issues.
- List<byte[]> result = new ArrayList<>(1);
- result.add(randEntry);
- return result;
- }
- if (duplicatesAllowed) {
- List<byte[]> result = new ArrayList<>(count);
- while (count > 0) {
- result.add(entries[rand.nextInt(entries.length)]);
- count--;
+ while (result.size() != count) {
+ int randIndex = rand.nextInt(members.getBackingArrayLength());
+ byte[] member = members.getFromBackingArray(randIndex);
+ if (member != null && !indexesUsed.contains(randIndex)) {
+ result.add(member);
+ indexesUsed.add(randIndex);
}
- return result;
- } else {
- Set<byte[]> result = new MemberSet(count);
- // Note that rand.nextInt can return duplicates when "count" is high
- // so we need to use a Set to collect the results.
- while (result.size() < count) {
- byte[] s = entries[rand.nextInt(entries.length)];
- result.add(s);
+ }
+ }
+
+ private void srandomUniqueListWithLargeCount(int count, List<byte[]> result) {
+ if (count >= members.size()) {
+ result.addAll(members);
+ return;
+ }
+
+ Random rand = new Random();
+ MemberSet duplicateSet = new MemberSet(members);
+
+ while (duplicateSet.size() != count) {
+ int randIndex = rand.nextInt(duplicateSet.getBackingArrayLength());
+ byte[] member = duplicateSet.getFromBackingArray(randIndex);
+ if (member != null) {
+ duplicateSet.remove(member);
}
- return result;
}
+
+ result.addAll(duplicateSet);
}
public boolean sismember(byte[] member) {
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableObjectOpenCustomHashSet.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableObjectOpenCustomHashSet.java
index d441b0b..1d57d8f 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableObjectOpenCustomHashSet.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/collections/SizeableObjectOpenCustomHashSet.java
@@ -62,6 +62,14 @@ public abstract class SizeableObjectOpenCustomHashSet<K> extends ObjectOpenCusto
return removed;
}
+ public K getFromBackingArray(final int pos) {
+ return key[pos];
+ }
+
+ public int getBackingArrayLength() {
+ return key.length;
+ }
+
@Override
public int getSizeInBytes() {
// The object referenced by the "strategy" field is not sized
[geode] 02/09: GEODE-9977: Remove defunct RedisCommandSupportLevel.INTERNAL type (#7283)
Posted by je...@apache.org.
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 d1d5e1e7c09f89a81cdaed6d4478d2f8cd121a41
Author: Donal Evans <do...@vmware.com>
AuthorDate: Thu Jan 20 08:53:14 2022 -0800
GEODE-9977: Remove defunct RedisCommandSupportLevel.INTERNAL type (#7283)
Authored-by: Donal Evans <do...@vmware.com>
(cherry picked from commit b349cb8b33fb0c93c0a3ea6a2f6c32bd33b51319)
---
.../geode/redis/internal/commands/RedisCommandSupportLevel.java | 3 +--
.../org/apache/geode/redis/internal/commands/RedisCommandType.java | 6 ------
.../internal/commands/executor/server/COMMANDCommandExecutor.java | 3 +--
3 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandSupportLevel.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandSupportLevel.java
index a8f6796..15caf12 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandSupportLevel.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/RedisCommandSupportLevel.java
@@ -19,6 +19,5 @@ package org.apache.geode.redis.internal.commands;
public enum RedisCommandSupportLevel {
SUPPORTED,
UNSUPPORTED,
- UNKNOWN,
- INTERNAL
+ UNKNOWN
}
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 acf335f..3a461de 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
@@ -16,7 +16,6 @@
package org.apache.geode.redis.internal.commands;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
-import static org.apache.geode.redis.internal.commands.RedisCommandSupportLevel.INTERNAL;
import static org.apache.geode.redis.internal.commands.RedisCommandSupportLevel.SUPPORTED;
import static org.apache.geode.redis.internal.commands.RedisCommandSupportLevel.UNSUPPORTED;
import static org.apache.geode.redis.internal.commands.RedisCommandType.Flag.ADMIN;
@@ -447,11 +446,6 @@ public enum RedisCommandType {
return supportLevel == UNSUPPORTED;
}
-
- public boolean isInternal() {
- return supportLevel == INTERNAL;
- }
-
public boolean isUnknown() {
return supportLevel == RedisCommandSupportLevel.UNKNOWN;
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/COMMANDCommandExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/COMMANDCommandExecutor.java
index 1557d92..b5950a8 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/COMMANDCommandExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/server/COMMANDCommandExecutor.java
@@ -62,8 +62,7 @@ public class COMMANDCommandExecutor implements CommandExecutor {
List<Object> response = new ArrayList<>();
for (RedisCommandType type : RedisCommandType.values()) {
- if (type.isInternal()
- || type.isUnknown()
+ if (type.isUnknown()
|| (type.isUnsupported() && !context.allowUnsupportedCommands())
|| type == RedisCommandType.QUIT) {
continue;
[geode] 05/09: GEODE-9885: Handle duplicated appends in Redis StringsDUnitTest (#7290)
Posted by je...@apache.org.
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 9166cb44ec2854c8c5d501334476f0d6dfe9d258
Author: Donal Evans <do...@vmware.com>
AuthorDate: Fri Jan 21 15:28:10 2022 -0800
GEODE-9885: Handle duplicated appends in Redis StringsDUnitTest (#7290)
* GEODE-9885: Handle duplicated appends in Redis StringsDUnitTest
- The Jedis client automatically retries when a bucket is moved during
an APPEND operation, which can lead to the append being duplicated. Add
error handling to the test to check if unexpected append values are
duplicates of the previous append and if so, ignore the AssertionError
- Add missing synchronization around in-place array modification in
RedisString.applyReplaceByteArrayAtOffsetDelta() method
- Remove unnecessarily overridden method from NullRedisSet
Authored-by: Donal Evans <do...@vmware.com>
(cherry picked from commit b8dd86b846083a59ffe1aa56b489df60f4d75d39)
---
.../commands/executor/string/StringsDUnitTest.java | 71 +++++++++++++---------
.../geode/redis/internal/data/NullRedisString.java | 5 --
.../geode/redis/internal/data/RedisString.java | 6 +-
3 files changed, 47 insertions(+), 35 deletions(-)
diff --git a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/StringsDUnitTest.java b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/StringsDUnitTest.java
index 2a4479a..a8f8ff4 100644
--- a/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/StringsDUnitTest.java
+++ b/geode-for-redis/src/distributedTest/java/org/apache/geode/redis/internal/commands/executor/string/StringsDUnitTest.java
@@ -30,7 +30,6 @@ import java.util.function.Consumer;
import org.junit.After;
import org.junit.AfterClass;
-import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
@@ -57,14 +56,13 @@ public class StringsDUnitTest {
private static final int NUM_ITERATIONS = 1000;
private static JedisCluster jedisCluster;
- private static MemberVM locator;
-
@BeforeClass
public static void classSetup() {
- locator = clusterStartUp.startLocatorVM(0);
- clusterStartUp.startRedisVM(1, locator.getPort());
- clusterStartUp.startRedisVM(2, locator.getPort());
- clusterStartUp.startRedisVM(3, locator.getPort());
+ MemberVM locator = clusterStartUp.startLocatorVM(0);
+ int locatorPort = locator.getPort();
+ clusterStartUp.startRedisVM(1, locatorPort);
+ clusterStartUp.startRedisVM(2, locatorPort);
+ clusterStartUp.startRedisVM(3, locatorPort);
int redisServerPort1 = clusterStartUp.getRedisPort(1);
jedisCluster =
@@ -263,9 +261,9 @@ public class StringsDUnitTest {
hashtags.add(clusterStartUp.getKeyOnServer("append", 2));
hashtags.add(clusterStartUp.getKeyOnServer("append", 3));
- Runnable task1 = () -> appendPerformAndVerify(1, 10000, hashtags.get(0), running);
- Runnable task2 = () -> appendPerformAndVerify(2, 10000, hashtags.get(1), running);
- Runnable task3 = () -> appendPerformAndVerify(3, 10000, hashtags.get(2), running);
+ Runnable task1 = () -> appendPerformAndVerify(1, hashtags.get(0), running);
+ Runnable task2 = () -> appendPerformAndVerify(2, hashtags.get(1), running);
+ Runnable task3 = () -> appendPerformAndVerify(3, hashtags.get(2), running);
Future<Void> future1 = executor.runAsync(task1);
Future<Void> future2 = executor.runAsync(task2);
@@ -276,11 +274,6 @@ public class StringsDUnitTest {
GeodeAwaitility.await().during(Duration.ofMillis(200)).until(() -> true);
}
- for (int i = 0; i < 100 && running.get(); i++) {
- clusterStartUp.moveBucketForKey(hashtags.get(i % hashtags.size()));
- GeodeAwaitility.await().during(Duration.ofMillis(200)).until(() -> true);
- }
-
running.set(false);
future1.get();
@@ -288,12 +281,11 @@ public class StringsDUnitTest {
future3.get();
}
- private void appendPerformAndVerify(int index, int minimumIterations, String hashtag,
- AtomicBoolean isRunning) {
+ private void appendPerformAndVerify(int index, String hashtag, AtomicBoolean isRunning) {
String key = "{" + hashtag + "}-key-" + index;
int iterationCount = 0;
- while (iterationCount < minimumIterations || isRunning.get()) {
+ while (isRunning.get()) {
String appendString = "-" + key + "-" + iterationCount + "-";
try {
jedisCluster.append(key, appendString);
@@ -301,23 +293,46 @@ public class StringsDUnitTest {
isRunning.set(false);
throw new RuntimeException("Exception performing APPEND " + appendString, ex);
}
- iterationCount += 1;
+ iterationCount++;
}
String storedString = jedisCluster.get(key);
- int idx = 0;
+ int startIndex = 0;
int i = 0;
- while (i < iterationCount) {
+ boolean lastWasDuplicate = false;
+ boolean reachedEnd = false;
+ while (!reachedEnd) {
String expectedValue = "-" + key + "-" + i + "-";
- String foundValue = storedString.substring(idx, idx + expectedValue.length());
- if (!foundValue.equals(expectedValue)) {
- Assert.fail("unexpected " + foundValue + " at index " + i + " iterationCount="
- + iterationCount + " in string "
- + storedString);
- break;
+
+ int endIndex = startIndex + expectedValue.length();
+ reachedEnd = endIndex == storedString.length();
+ String foundValue = storedString.substring(startIndex, endIndex);
+
+ int subsectionStart = Math.max(0, startIndex - (2 * expectedValue.length()));
+ int subsectionEnd = Math.min(storedString.length(), endIndex + (2 * expectedValue.length()));
+
+ try {
+ assertThat(foundValue).as("unexpected " + foundValue
+ + " at index " + i
+ + " iterationCount=" + iterationCount
+ + " in String subsection: " + storedString.substring(subsectionStart, subsectionEnd))
+ .isEqualTo(expectedValue);
+ lastWasDuplicate = false;
+ } catch (AssertionError error) {
+ // Jedis client retries can lead to duplicated appends, so check if the append is a
+ // duplicate of the previous one, but only allow one duplicated append in a row
+ String previousAppend = "-" + key + "-" + (i - 1) + "-";
+ if (lastWasDuplicate || !foundValue.equals(previousAppend)) {
+ throw error;
+ }
+
+ // Decrement the counter to account for the duplicated append and reset the flag to detect
+ // multiple duplicated appends in a row
+ lastWasDuplicate = true;
+ i--;
}
- idx += expectedValue.length();
+ startIndex = endIndex;
i++;
}
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisString.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisString.java
index 5a644fc..fdd9326 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisString.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisString.java
@@ -43,11 +43,6 @@ public class NullRedisString extends RedisString {
}
@Override
- protected void valueAppend(byte[] bytes) {
- throw new UnsupportedOperationException();
- }
-
- @Override
public byte[] get() {
return null;
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java
index db9baf2..cfde92b 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisString.java
@@ -190,7 +190,9 @@ public class RedisString extends AbstractRedisData {
public void applyReplaceByteArrayAtOffsetDelta(int offset, byte[] valueToAdd) {
int totalLength = offset + valueToAdd.length;
if (totalLength < value.length) {
- System.arraycopy(valueToAdd, 0, value, offset, valueToAdd.length);
+ synchronized (this) {
+ System.arraycopy(valueToAdd, 0, value, offset, valueToAdd.length);
+ }
} else {
byte[] newBytes = Arrays.copyOf(value, totalLength);
System.arraycopy(valueToAdd, 0, newBytes, offset, valueToAdd.length);
@@ -362,7 +364,7 @@ public class RedisString extends AbstractRedisData {
/**
* Since GII (getInitialImage) can come in and call toData while other threads
* are modifying this object, the striped executor will not protect toData.
- * So any methods that modify "value", "appendSequence" need to be thread safe with toData.
+ * So any methods that modify "value" need to be thread safe with toData.
*/
@Override
[geode] 06/09: update redis svg to use new module name (#7288)
Posted by je...@apache.org.
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 731fcd0bb5a1ca29a78f5b7dbe07450ccc563c5f
Author: Hale Bales <hb...@vmware.com>
AuthorDate: Fri Jan 21 15:31:49 2022 -0800
update redis svg to use new module name (#7288)
(cherry picked from commit 66eb3f93aa44613a355515ee7d8e0bb36775d932)
---
geode-docs/images_svg/geode_for_redis.svg | 206 +++++++++++++++++++++++++++---
1 file changed, 188 insertions(+), 18 deletions(-)
diff --git a/geode-docs/images_svg/geode_for_redis.svg b/geode-docs/images_svg/geode_for_redis.svg
index f983cf8..26efc38 100644
--- a/geode-docs/images_svg/geode_for_redis.svg
+++ b/geode-docs/images_svg/geode_for_redis.svg
@@ -1,19 +1,189 @@
-<svg width="1127" height="319" viewBox="0 0 1127 319" fill="none" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
-<rect width="1127" height="319" fill="white"/>
-<rect x="0.5" y="0.5" width="749" height="318" rx="3.5" fill="white" stroke="black"/>
-<rect x="50" y="86" width="467" height="147" fill="url(#pattern0)"/>
-<rect x="567" width="183" height="92" rx="4" fill="#A41F10"/>
-<path d="M596.48 40H598.964L593.798 27.634H591.296L586.13 40H588.632L589.568 37.768H595.544L596.48 40ZM590.378 35.824L592.556 30.622L594.734 35.824H590.378ZM600.601 40H602.851V35.842H605.983C608.629 35.842 610.609 34.15 610.609 31.738C610.609 29.326 608.629 27.634 605.983 27.634H600.601V40ZM602.851 33.772V29.704H605.731C607.261 29.704 608.323 30.478 608.323 31.738C608.323 32.998 607.261 33.772 605.731 33.772H602.851ZM612.415 40H614.647V27.634H612.415V40ZM620.596 40.216C622.774 40.216 624 [...]
-<path d="M881.006 51H883.49L878.324 38.634H875.822L870.656 51H873.158L874.094 48.768H880.07L881.006 51ZM874.904 46.824L877.082 41.622L879.26 46.824H874.904ZM890.546 41.478C889.16 41.478 888.044 42.108 887.306 43.17V41.694H885.11V54.096H887.306V49.524C888.044 50.586 889.16 51.216 890.546 51.216C893.012 51.216 894.812 49.2 894.812 46.338C894.812 43.494 893.012 41.478 890.546 41.478ZM889.952 49.344C888.404 49.344 887.324 48.084 887.324 46.338C887.324 44.592 888.404 43.35 889.952 43.35C891.5 [...]
-<path d="M881.111 164H883.595L878.429 151.634H875.927L870.761 164H873.263L874.199 161.768H880.175L881.111 164ZM875.009 159.824L877.187 154.622L879.365 159.824H875.009ZM890.65 154.478C889.264 154.478 888.148 155.108 887.41 156.17V154.694H885.214V167.096H887.41V162.524C888.148 163.586 889.264 164.216 890.65 164.216C893.116 164.216 894.916 162.2 894.916 159.338C894.916 156.494 893.116 154.478 890.65 154.478ZM890.056 162.344C888.508 162.344 887.428 161.084 887.428 159.338C887.428 157.592 888 [...]
-<line x1="750" y1="45.5" x2="836" y2="45.5" stroke="black"/>
-<line x1="750" y1="158.5" x2="836" y2="158.5" stroke="black"/>
-<circle cx="835.5" cy="45.5" r="1.5" fill="black"/>
-<circle cx="835.5" cy="158.5" r="1.5" fill="black"/>
-<defs>
-<pattern id="pattern0" patternContentUnits="objectBoundingBox" width="1" height="1">
-<use xlink:href="#image0" transform="translate(0 -0.00215054) scale(0.00129032 0.00409919)"/>
-</pattern>
-<image id="image0" width="775" height="245" xlink:href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAwcAAAD1CAYAAAAF6onAAAAACXBIWXMAAC4jAAAuIwF4pT92AAAgAElEQVR4nO2dvW8jWZfeiy/GwTpRT7YbSQM7NEAO4NCwOCiAAKPmRHYmtrB5cwJq4ajZgOE1qKDZkR2xqXDhYChsIIBAoSXAwWaj+gtGzJy9YuTEAI2jeWq6ms0i7711b9WtqucHCPO+MxI/isWq85yP57S2221A6s24Px8EQXB/fXf5zI+aEEIIIYRk8RcemXoz7s+HQRD8KuJg3J+/avrxIIQQQggh2VAc1BgIg094h20KBEIIIYQQcgiKg5qyIwwSKBAIIYQQQkgmFAc1JEMYJFAgEEIIIYSQvVAcFEQvDM96Yeg8ID8iDBIKFQhFvG9CCCGEEJIfioPiWA [...]
-</defs>
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg
+ width="1127"
+ height="319"
+ viewBox="0 0 1127 319"
+ fill="none"
+ version="1.1"
+ id="svg713"
+ sodipodi:docname="geode_for_redis.svg"
+ inkscape:version="1.1.1 (c3084ef, 2021-09-22)"
+ xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
+ xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
+ xmlns:xlink="http://www.w3.org/1999/xlink"
+ xmlns="http://www.w3.org/2000/svg"
+ xmlns:svg="http://www.w3.org/2000/svg">
+ <sodipodi:namedview
+ id="namedview715"
+ pagecolor="#ffffff"
+ bordercolor="#999999"
+ borderopacity="1"
+ inkscape:pageshadow="0"
+ inkscape:pageopacity="0"
+ inkscape:pagecheckerboard="0"
+ showgrid="false"
+ fit-margin-top="0"
+ fit-margin-left="0"
+ fit-margin-right="0"
+ fit-margin-bottom="0"
+ borderlayer="false"
+ showguides="true"
+ inkscape:guide-bbox="true"
+ inkscape:zoom="0.97515528"
+ inkscape:cx="209.71019"
+ inkscape:cy="303.02866"
+ inkscape:window-width="1312"
+ inkscape:window-height="1027"
+ inkscape:window-x="4480"
+ inkscape:window-y="218"
+ inkscape:window-maximized="0"
+ inkscape:current-layer="svg713" />
+ <rect
+ x="122.96814"
+ width="183"
+ height="92"
+ rx="4"
+ fill="#a41f10"
+ id="rect691"
+ y="0.038216591" />
+ <path
+ d="m 436.97414,51.038217 h 2.484 l -5.166,-12.366 h -2.502 l -5.166,12.366 h 2.502 l 0.936,-2.232 h 5.976 z m -6.102,-4.176 2.178,-5.202 2.178,5.202 z m 15.642,-5.346 c -1.386,0 -2.502,0.63 -3.24,1.692 v -1.476 h -2.196 v 12.402 h 2.196 v -4.572 c 0.738,1.062 1.854,1.692 3.24,1.692 2.466,0 4.266,-2.016 4.266,-4.878 0,-2.844 -1.8,-4.86 -4.266,-4.86 z m -0.594,7.866 c -1.548,0 -2.628,-1.26 -2.628,-3.006 0,-1.746 1.08,-2.988 2.628,-2.988 1.566,0 2.682,1.242 2.682,2.988 0,1.746 -1.116,3 [...]
+ fill="#000000"
+ id="path695" />
+ <path
+ d="m 437.07914,164.03822 h 2.484 l -5.166,-12.366 h -2.502 l -5.166,12.366 h 2.502 l 0.936,-2.232 h 5.976 z m -6.102,-4.176 2.178,-5.202 2.178,5.202 z m 15.641,-5.346 c -1.386,0 -2.502,0.63 -3.24,1.692 v -1.476 h -2.196 v 12.402 h 2.196 v -4.572 c 0.738,1.062 1.854,1.692 3.24,1.692 2.466,0 4.266,-2.016 4.266,-4.878 0,-2.844 -1.8,-4.86 -4.266,-4.86 z m -0.594,7.866 c -1.548,0 -2.628,-1.26 -2.628,-3.006 0,-1.746 1.08,-2.988 2.628,-2.988 1.566,0 2.682,1.242 2.682,2.988 0,1.746 -1.116,3 [...]
+ fill="#000000"
+ id="path697" />
+ <line
+ x1="305.96814"
+ y1="45.538216"
+ x2="391.96814"
+ y2="45.538216"
+ stroke="#000000"
+ id="line699" />
+ <line
+ x1="305.96814"
+ y1="158.53822"
+ x2="391.96814"
+ y2="158.53822"
+ stroke="#000000"
+ id="line701" />
+ <circle
+ cx="391.46814"
+ cy="45.538216"
+ r="1.5"
+ fill="#000000"
+ id="circle703" />
+ <circle
+ cx="391.46814"
+ cy="158.53822"
+ r="1.5"
+ fill="#000000"
+ id="circle705" />
+ <defs
+ id="defs711">
+ <pattern
+ id="pattern0"
+ patternContentUnits="objectBoundingBox"
+ width="1"
+ height="1">
+ <use
+ xlink:href="#image0"
+ transform="translate(0 -0.00215054) scale(0.00129032 0.00409919)"
+ id="use707" />
+ </pattern>
+ <image
+ id="image0"
+ width="775"
+ height="245"
+ xlink:href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAwcAAAD1CAYAAAAF6onAAAAACXBIWXMAAC4jAAAuIwF4pT92AAAgAElEQVR4nO2dvW8jWZfeiy/GwTpRT7YbSQM7NEAO4NCwOCiAAKPmRHYmtrB5cwJq4ajZgOE1qKDZkR2xqXDhYChsIIBAoSXAwWaj+gtGzJy9YuTEAI2jeWq6ms0i7711b9WtqucHCPO+MxI/isWq85yP57S2221A6s24Px8EQXB/fXf5zI+aEEIIIYRk8RcemXoz7s+HQRD8KuJg3J+/avrxIIQQQggh2VAc1BgIg094h20KBEIIIYQQcgiKg5qyIwwSKBAIIYQQQkgmFAc1JEMYJFAgEEIIIYSQvVAcFEQvDM96Yeg8ID8iDBIKFQhFvG9CCCGEEJIfioPiWAZB8NgLw6GrZ1QUBgnOBUIvDLu9MHzCeyeEEEI [...]
+ </defs>
+ <text
+ xml:space="preserve"
+ style="font-style:normal;font-weight:normal;font-size:20.0182px;line-height:1.25;font-family:sans-serif;fill:#000000;fill-opacity:1;stroke:none;stroke-width:0.938356"
+ x="134.30402"
+ y="51.743076"
+ id="text4527"><tspan
+ sodipodi:role="line"
+ id="tspan4525"
+ x="134.30402"
+ y="51.743076"
+ style="font-size:20.0182px;fill:#ffffff;stroke-width:0.938356">Geode for Redis</tspan></text>
+ <rect
+ width="1127"
+ height="319"
+ fill="#ffffff"
+ id="rect685-1"
+ x="0"
+ y="0" />
+ <rect
+ x="0.5"
+ y="0.5"
+ width="749"
+ height="318"
+ rx="3.5"
+ fill="#ffffff"
+ stroke="#000000"
+ id="rect687-3" />
+ <rect
+ x="567.00006"
+ width="183"
+ height="92"
+ rx="4"
+ fill="#a41f10"
+ id="rect691-8"
+ y="0" />
+ <path
+ d="m 881.00604,51 h 2.484 l -5.166,-12.366 h -2.502 L 870.65604,51 h 2.502 l 0.936,-2.232 h 5.976 z m -6.102,-4.176 2.178,-5.202 2.178,5.202 z m 15.642,-5.346 c -1.386,0 -2.502,0.63 -3.24,1.692 v -1.476 h -2.196 v 12.402 h 2.196 v -4.572 c 0.738,1.062 1.854,1.692 3.24,1.692 2.466,0 4.266,-2.016 4.266,-4.878 0,-2.844 -1.8,-4.86 -4.266,-4.86 z m -0.594,7.866 c -1.548,0 -2.628,-1.26 -2.628,-3.006 0,-1.746 1.08,-2.988 2.628,-2.988 1.566,0 2.682,1.242 2.682,2.988 0,1.746 -1.116,3.006 -2. [...]
+ fill="#000000"
+ id="path695-4" />
+ <path
+ d="m 881.11104,164 h 2.484 l -5.166,-12.366 h -2.502 l -5.166,12.366 h 2.502 l 0.936,-2.232 h 5.976 z m -6.102,-4.176 2.178,-5.202 2.178,5.202 z m 15.641,-5.346 c -1.386,0 -2.502,0.63 -3.24,1.692 v -1.476 h -2.196 v 12.402 h 2.196 v -4.572 c 0.738,1.062 1.854,1.692 3.24,1.692 2.466,0 4.266,-2.016 4.266,-4.878 0,-2.844 -1.8,-4.86 -4.266,-4.86 z m -0.5941,7.866 c -1.548,0 -2.6279,-1.26 -2.6279,-3.006 0,-1.746 1.0799,-2.988 2.6279,-2.988 1.5661,0 2.6821,1.242 2.6821,2.988 0,1.746 -1.11 [...]
+ fill="#000000"
+ id="path697-8" />
+ <line
+ x1="750"
+ y1="45.5"
+ x2="836"
+ y2="45.5"
+ stroke="#000000"
+ id="line699-0" />
+ <line
+ x1="750"
+ y1="158.5"
+ x2="836"
+ y2="158.5"
+ stroke="#000000"
+ id="line701-4" />
+ <circle
+ cx="835.5"
+ cy="45.5"
+ r="1.5"
+ fill="#000000"
+ id="circle703-6" />
+ <circle
+ cx="835.5"
+ cy="158.5"
+ r="1.5"
+ fill="#000000"
+ id="circle705-0" />
+ <image
+ width="428.75452"
+ height="141.707"
+ preserveAspectRatio="none"
+ xlink:href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAsQAAADqCAYAAAClbFPsAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJ bWFnZVJlYWR5ccllPAAAA/RpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADw/eHBhY2tldCBiZWdp bj0i77u/IiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+IDx4OnhtcG1ldGEgeG1sbnM6 eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDUuNi1jMDE0IDc5LjE1 Njc5NywgMjAxNC8wOC8yMC0wOTo1MzowMiAgICAgICAgIj4gPHJkZjpSREYgeG1sbnM6cmRmPSJo dHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4gPHJkZjpE [...]
+ id="image1071-3"
+ x="78.576782"
+ y="110.10629" />
+ <text
+ xml:space="preserve"
+ style="font-style:normal;font-weight:normal;font-size:20.0182px;line-height:1.25;font-family:sans-serif;fill:#000000;fill-opacity:1;stroke:none;stroke-width:0.938356"
+ x="578.33582"
+ y="51.704861"
+ id="text4527-2"><tspan
+ sodipodi:role="line"
+ id="tspan4525-6"
+ x="578.33582"
+ y="51.704861"
+ style="font-size:20.0182px;fill:#ffffff;stroke-width:0.938356">Geode for Redis</tspan></text>
</svg>
[geode] 07/09: GEODE-9922: Move Redis cross-slot checking to RegionProvider (#7295)
Posted by je...@apache.org.
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 ebd17de286f5e12945c239ad6dfc9fd658ed7d8b
Author: Donal Evans <do...@vmware.com>
AuthorDate: Fri Jan 21 17:54:03 2022 -0800
GEODE-9922: Move Redis cross-slot checking to RegionProvider (#7295)
* GEODE-9922: Move Redis cross-slot checking to RegionProvider
- Move duplicated logic for determining if Keys are in different slots
from various Executors to RegionProvider
- Removed manual checks for if the key is local, as this is performed
as part of locking the primary bucket
- Created RedisCrossSlotException class
- Added unit tests for new method in RegionProvider
- Refactor SetOpExecutor to also lock the destination key for *STORE
commands
- Add missing test cases for cross-slot errors
- Correct some tests for cross-slot behaviour that were inadvertantly
testing the Jedis client's response rather than the Geode for Redis
server
- Changed name format for constants in AbstractSMoveIntegrationTest
- Modify patch file to ensure tcl tests use keys with the same slot
Authored-by: Donal Evans <do...@vmware.com>
(cherry picked from commit 7b0a88dbee36c6eb51513715af943f80ea6d93f9)
---
.../resources/0001-configure-redis-tests.patch | 340 ++++++++++++++++++++-
.../key/AbstractRenameIntegrationTest.java | 15 +-
.../key/AbstractRenameNXIntegrationTest.java | 12 +
.../executor/set/AbstractSDiffIntegrationTest.java | 15 +-
.../set/AbstractSDiffStoreIntegrationTest.java | 15 +-
.../set/AbstractSInterIntegrationTest.java | 14 +-
.../executor/set/AbstractSMoveIntegrationTest.java | 165 +++++-----
.../set/AbstractSUnionIntegrationTest.java | 10 +-
.../set/AbstractSUnionStoreIntegrationTest.java | 8 +-
.../string/AbstractMSetIntegrationTest.java | 3 +-
.../string/AbstractMSetNXIntegrationTest.java | 3 +-
.../apache/geode/codeAnalysis/excludedClasses.txt | 1 +
.../geode/redis/internal/RedisConstants.java | 2 -
.../executor/key/AbstractRenameExecutor.java | 5 -
.../commands/executor/set/SMoveExecutor.java | 9 -
.../commands/executor/set/SetOpExecutor.java | 22 +-
.../executor/sortedset/ZInterStoreExecutor.java | 2 +-
.../executor/sortedset/ZStoreExecutor.java | 14 +-
.../executor/sortedset/ZUnionStoreExecutor.java | 2 +-
.../executor/string/AbstractMSetExecutor.java | 8 +-
.../internal/data/RedisCrossSlotException.java | 30 ++
.../internal/netty/ExecutionHandlerContext.java | 3 +
.../redis/internal/services/RegionProvider.java | 20 +-
.../internal/services/RegionProviderTest.java | 74 +++++
24 files changed, 635 insertions(+), 157 deletions(-)
diff --git a/geode-for-redis/src/acceptanceTest/resources/0001-configure-redis-tests.patch b/geode-for-redis/src/acceptanceTest/resources/0001-configure-redis-tests.patch
index 88e1318..f9952bd 100644
--- a/geode-for-redis/src/acceptanceTest/resources/0001-configure-redis-tests.patch
+++ b/geode-for-redis/src/acceptanceTest/resources/0001-configure-redis-tests.patch
@@ -111,7 +111,7 @@ index f5da728e8..13985dce2 100644
test {Once AUTH succeeded we can actually send commands to the server} {
diff --git a/tests/unit/dump.tcl b/tests/unit/dump.tcl
-index 4c4e5d075..18bb694f2 100644
+index 4c4e5d075..e465300f4 100644
--- a/tests/unit/dump.tcl
+++ b/tests/unit/dump.tcl
@@ -41,34 +41,35 @@ start_server {tags {"dump"}} {
@@ -162,7 +162,7 @@ index 4c4e5d075..18bb694f2 100644
+# assert {$idle >= 1000 && $idle <= 1010}
+# r get foo
+# } {bar}
-+#
++#
+# test {RESTORE can set LFU} {
+# r set foo bar
+# set encoded [r dump foo]
@@ -1337,7 +1337,7 @@ index d2c679d32..6d17de48b 100644
# The following test can only be executed if we don't use Valgrind, and if
# we are using x86_64 architecture, because:
diff --git a/tests/unit/type/set.tcl b/tests/unit/type/set.tcl
-index 7b467f1c4..21f0721c4 100644
+index 7b467f1c4..0c5ca1753 100644
--- a/tests/unit/type/set.tcl
+++ b/tests/unit/type/set.tcl
@@ -34,8 +34,8 @@ start_server {
@@ -1360,15 +1360,341 @@ index 7b467f1c4..21f0721c4 100644
assert_encoding intset myintset
assert_encoding hashtable mylargeintset
assert_encoding hashtable myhashset
-@@ -157,7 +157,7 @@ start_server {
+@@ -113,19 +113,19 @@ start_server {
+
+ foreach {type} {hashtable intset} {
+ for {set i 1} {$i <= 5} {incr i} {
+- r del [format "set%d" $i]
++ r del [format "{tag}set%d" $i]
+ }
+ for {set i 0} {$i < 200} {incr i} {
+- r sadd set1 $i
+- r sadd set2 [expr $i+195]
++ r sadd "{tag}set1" $i
++ r sadd "{tag}set2" [expr $i+195]
+ }
+ foreach i {199 195 1000 2000} {
+- r sadd set3 $i
++ r sadd "{tag}set3" $i
+ }
+ for {set i 5} {$i < 200} {incr i} {
+- r sadd set4 $i
++ r sadd "{tag}set4" $i
+ }
+- r sadd set5 0
++ r sadd "{tag}set5" 0
+
+ # To make sure the sets are encoded as the type we are testing -- also
+ # when the VM is enabled and the values may be swapped in and out
+@@ -137,87 +137,87 @@ start_server {
+ }
+
+ for {set i 1} {$i <= 5} {incr i} {
+- r sadd [format "set%d" $i] $large
++ r sadd [format "{tag}set%d" $i] $large
+ }
+
+ test "Generated sets must be encoded as $type" {
+ for {set i 1} {$i <= 5} {incr i} {
+- assert_encoding $type [format "set%d" $i]
++ assert_encoding $type [format "{tag}set%d" $i]
+ }
+ }
+
+ test "SINTER with two sets - $type" {
+- assert_equal [list 195 196 197 198 199 $large] [lsort [r sinter set1 set2]]
++ assert_equal [list 195 196 197 198 199 $large] [lsort [r sinter "{tag}set1" "{tag}set2"]]
+ }
+
+ test "SINTERSTORE with two sets - $type" {
+- r sinterstore setres set1 set2
+- assert_encoding $type setres
+- assert_equal [list 195 196 197 198 199 $large] [lsort [r smembers setres]]
++ r sinterstore "{tag}setres" "{tag}set1" "{tag}set2"
++ assert_encoding $type "{tag}setres"
++ assert_equal [list 195 196 197 198 199 $large] [lsort [r smembers "{tag}setres"]]
}
test "SINTERSTORE with two sets, after a DEBUG RELOAD - $type" {
- r debug reload
+- r sinterstore setres set1 set2
+- assert_encoding $type setres
+- assert_equal [list 195 196 197 198 199 $large] [lsort [r smembers setres]]
+ #r debug reload
- r sinterstore setres set1 set2
- assert_encoding $type setres
- assert_equal [list 195 196 197 198 199 $large] [lsort [r smembers setres]]
++ r sinterstore "{tag}setres" "{tag}set1" "{tag}set2"
++ assert_encoding $type "{tag}setres"
++ assert_equal [list 195 196 197 198 199 $large] [lsort [r smembers "{tag}setres"]]
+ }
+
+ test "SUNION with two sets - $type" {
+- set expected [lsort -uniq "[r smembers set1] [r smembers set2]"]
+- assert_equal $expected [lsort [r sunion set1 set2]]
++ set expected [lsort -uniq "[r smembers "{tag}set1"] [r smembers "{tag}set2"]"]
++ assert_equal $expected [lsort [r sunion "{tag}set1" "{tag}set2"]]
+ }
+
+ test "SUNIONSTORE with two sets - $type" {
+- r sunionstore setres set1 set2
+- assert_encoding $type setres
+- set expected [lsort -uniq "[r smembers set1] [r smembers set2]"]
+- assert_equal $expected [lsort [r smembers setres]]
++ r sunionstore "{tag}setres" "{tag}set1" "{tag}set2"
++ assert_encoding $type "{tag}setres"
++ set expected [lsort -uniq "[r smembers "{tag}set1"] [r smembers "{tag}set2"]"]
++ assert_equal $expected [lsort [r smembers "{tag}setres"]]
+ }
+
+ test "SINTER against three sets - $type" {
+- assert_equal [list 195 199 $large] [lsort [r sinter set1 set2 set3]]
++ assert_equal [list 195 199 $large] [lsort [r sinter "{tag}set1" "{tag}set2" "{tag}set3"]]
+ }
+
+ test "SINTERSTORE with three sets - $type" {
+- r sinterstore setres set1 set2 set3
+- assert_equal [list 195 199 $large] [lsort [r smembers setres]]
++ r sinterstore "{tag}setres" "{tag}set1" "{tag}set2" "{tag}set3"
++ assert_equal [list 195 199 $large] [lsort [r smembers "{tag}setres"]]
+ }
+
+ test "SUNION with non existing keys - $type" {
+- set expected [lsort -uniq "[r smembers set1] [r smembers set2]"]
+- assert_equal $expected [lsort [r sunion nokey1 set1 set2 nokey2]]
++ set expected [lsort -uniq "[r smembers "{tag}set1"] [r smembers "{tag}set2"]"]
++ assert_equal $expected [lsort [r sunion "{tag}nokey1" "{tag}set1" "{tag}set2" "{tag}nokey2"]]
+ }
+
+ test "SDIFF with two sets - $type" {
+- assert_equal {0 1 2 3 4} [lsort [r sdiff set1 set4]]
++ assert_equal {0 1 2 3 4} [lsort [r sdiff "{tag}set1" "{tag}set4"]]
+ }
+
+ test "SDIFF with three sets - $type" {
+- assert_equal {1 2 3 4} [lsort [r sdiff set1 set4 set5]]
++ assert_equal {1 2 3 4} [lsort [r sdiff "{tag}set1" "{tag}set4" "{tag}set5"]]
+ }
+
+ test "SDIFFSTORE with three sets - $type" {
+- r sdiffstore setres set1 set4 set5
++ r sdiffstore "{tag}setres" "{tag}set1" "{tag}set4" "{tag}set5"
+ # When we start with intsets, we should always end with intsets.
+ if {$type eq {intset}} {
+- assert_encoding intset setres
++ assert_encoding intset "{tag}setres"
+ }
+- assert_equal {1 2 3 4} [lsort [r smembers setres]]
++ assert_equal {1 2 3 4} [lsort [r smembers "{tag}setres"]]
+ }
+ }
+
+ test "SDIFF with first set empty" {
+- r del set1 set2 set3
+- r sadd set2 1 2 3 4
+- r sadd set3 a b c d
+- r sdiff set1 set2 set3
++ r del "{tag}set1" "{tag}set2" "{tag}set3"
++ r sadd "{tag}set2" 1 2 3 4
++ r sadd "{tag}set3" a b c d
++ r sdiff "{tag}set1" "{tag}set2" "{tag}set3"
+ } {}
+
+ test "SDIFF with same set two times" {
+- r del set1
+- r sadd set1 a b c 1 2 3 4 5 6
+- r sdiff set1 set1
++ r del "{tag}set1"
++ r sadd "{tag}set1" a b c 1 2 3 4 5 6
++ r sdiff "{tag}set1" "{tag}set1"
+ } {}
+
+ test "SDIFF fuzzing" {
+@@ -228,11 +228,11 @@ start_server {
+ set num_sets [expr {[randomInt 10]+1}]
+ for {set i 0} {$i < $num_sets} {incr i} {
+ set num_elements [randomInt 100]
+- r del set_$i
+- lappend args set_$i
++ r del [format "{tag}set%d" $i]
++ lappend args [format "{tag}set%d" $i]
+ while {$num_elements} {
+ set ele [randomValue]
+- r sadd set_$i $ele
++ r sadd [format "{tag}set%d" $i] $ele
+ if {$i == 0} {
+ set s($ele) x
+ } else {
+@@ -247,42 +247,42 @@ start_server {
+ }
+
+ test "SINTER against non-set should throw error" {
+- r set key1 x
+- assert_error "WRONGTYPE*" {r sinter key1 noset}
++ r set "{tag}key1" x
++ assert_error "WRONGTYPE*" {r sinter "{tag}key1" "{tag}noset"}
+ }
+
+ test "SUNION against non-set should throw error" {
+- r set key1 x
+- assert_error "WRONGTYPE*" {r sunion key1 noset}
++ r set "{tag}key1" x
++ assert_error "WRONGTYPE*" {r sunion "{tag}key1" "{tag}noset"}
+ }
+
+ test "SINTER should handle non existing key as empty" {
+- r del set1 set2 set3
+- r sadd set1 a b c
+- r sadd set2 b c d
+- r sinter set1 set2 set3
++ r del "{tag}set1" "{tag}set2" "{tag}set3"
++ r sadd "{tag}set1" a b c
++ r sadd "{tag}set2" b c d
++ r sinter "{tag}set1" "{tag}set2" "{tag}set3"
+ } {}
+
+ test "SINTER with same integer elements but different encoding" {
+- r del set1 set2
+- r sadd set1 1 2 3
+- r sadd set2 1 2 3 a
+- r srem set2 a
+- assert_encoding intset set1
+- assert_encoding hashtable set2
+- lsort [r sinter set1 set2]
++ r del "{tag}set1" "{tag}set2"
++ r sadd "{tag}set1" 1 2 3
++ r sadd "{tag}set2" 1 2 3 a
++ r srem "{tag}set2" a
++ assert_encoding intset "{tag}set1"
++ assert_encoding hashtable "{tag}set2"
++ lsort [r sinter "{tag}set1" "{tag}set2"]
+ } {1 2 3}
+
+ test "SINTERSTORE against non existing keys should delete dstkey" {
+- r set setres xxx
+- assert_equal 0 [r sinterstore setres foo111 bar222]
+- assert_equal 0 [r exists setres]
++ r set "{tag}setres" xxx
++ assert_equal 0 [r sinterstore "{tag}setres" "{tag}foo111" "{tag}bar222"]
++ assert_equal 0 [r exists "{tag}setres"]
+ }
+
+ test "SUNIONSTORE against non existing keys should delete dstkey" {
+- r set setres xxx
+- assert_equal 0 [r sunionstore setres foo111 bar222]
+- assert_equal 0 [r exists setres]
++ r set "{tag}setres" xxx
++ assert_equal 0 [r sunionstore "{tag}setres" "{tag}foo111" "{tag}bar222"]
++ assert_equal 0 [r exists "{tag}setres"]
+ }
+
+ foreach {type contents} {hashtable {a b c} intset {1 2 3}} {
+@@ -486,74 +486,74 @@ start_server {
+ }
+
+ proc setup_move {} {
+- r del myset3 myset4
+- create_set myset1 {1 a b}
+- create_set myset2 {2 3 4}
+- assert_encoding hashtable myset1
+- assert_encoding intset myset2
++ r del "{tag}myset3" "{tag}myset4"
++ create_set "{tag}myset1" {1 a b}
++ create_set "{tag}myset2" {2 3 4}
++ assert_encoding hashtable "{tag}myset1"
++ assert_encoding intset "{tag}myset2"
+ }
+
+ test "SMOVE basics - from regular set to intset" {
+ # move a non-integer element to an intset should convert encoding
+ setup_move
+- assert_equal 1 [r smove myset1 myset2 a]
+- assert_equal {1 b} [lsort [r smembers myset1]]
+- assert_equal {2 3 4 a} [lsort [r smembers myset2]]
+- assert_encoding hashtable myset2
++ assert_equal 1 [r smove "{tag}myset1" "{tag}myset2" a]
++ assert_equal {1 b} [lsort [r smembers "{tag}myset1"]]
++ assert_equal {2 3 4 a} [lsort [r smembers "{tag}myset2"]]
++ assert_encoding hashtable "{tag}myset2"
+
+ # move an integer element should not convert the encoding
+ setup_move
+- assert_equal 1 [r smove myset1 myset2 1]
+- assert_equal {a b} [lsort [r smembers myset1]]
+- assert_equal {1 2 3 4} [lsort [r smembers myset2]]
+- assert_encoding intset myset2
++ assert_equal 1 [r smove "{tag}myset1" "{tag}myset2" 1]
++ assert_equal {a b} [lsort [r smembers "{tag}myset1"]]
++ assert_equal {1 2 3 4} [lsort [r smembers "{tag}myset2"]]
++ assert_encoding intset "{tag}myset2"
+ }
+
+ test "SMOVE basics - from intset to regular set" {
+ setup_move
+- assert_equal 1 [r smove myset2 myset1 2]
+- assert_equal {1 2 a b} [lsort [r smembers myset1]]
+- assert_equal {3 4} [lsort [r smembers myset2]]
++ assert_equal 1 [r smove "{tag}myset2" "{tag}myset1" 2]
++ assert_equal {1 2 a b} [lsort [r smembers "{tag}myset1"]]
++ assert_equal {3 4} [lsort [r smembers "{tag}myset2"]]
+ }
+
+ test "SMOVE non existing key" {
+ setup_move
+- assert_equal 0 [r smove myset1 myset2 foo]
+- assert_equal 0 [r smove myset1 myset1 foo]
+- assert_equal {1 a b} [lsort [r smembers myset1]]
+- assert_equal {2 3 4} [lsort [r smembers myset2]]
++ assert_equal 0 [r smove "{tag}myset1" "{tag}myset2" foo]
++ assert_equal 0 [r smove "{tag}myset1" "{tag}myset1" foo]
++ assert_equal {1 a b} [lsort [r smembers "{tag}myset1"]]
++ assert_equal {2 3 4} [lsort [r smembers "{tag}myset2"]]
+ }
+
+ test "SMOVE non existing src set" {
+ setup_move
+- assert_equal 0 [r smove noset myset2 foo]
+- assert_equal {2 3 4} [lsort [r smembers myset2]]
++ assert_equal 0 [r smove "{tag}noset" "{tag}myset2" foo]
++ assert_equal {2 3 4} [lsort [r smembers "{tag}myset2"]]
+ }
+
+ test "SMOVE from regular set to non existing destination set" {
+ setup_move
+- assert_equal 1 [r smove myset1 myset3 a]
+- assert_equal {1 b} [lsort [r smembers myset1]]
+- assert_equal {a} [lsort [r smembers myset3]]
+- assert_encoding hashtable myset3
++ assert_equal 1 [r smove "{tag}myset1" "{tag}myset3" a]
++ assert_equal {1 b} [lsort [r smembers "{tag}myset1"]]
++ assert_equal {a} [lsort [r smembers "{tag}myset3"]]
++ assert_encoding hashtable "{tag}myset3"
+ }
+
+ test "SMOVE from intset to non existing destination set" {
+ setup_move
+- assert_equal 1 [r smove myset2 myset3 2]
+- assert_equal {3 4} [lsort [r smembers myset2]]
+- assert_equal {2} [lsort [r smembers myset3]]
+- assert_encoding intset myset3
++ assert_equal 1 [r smove "{tag}myset2" "{tag}myset3" 2]
++ assert_equal {3 4} [lsort [r smembers "{tag}myset2"]]
++ assert_equal {2} [lsort [r smembers "{tag}myset3"]]
++ assert_encoding intset "{tag}myset3"
+ }
+
+ test "SMOVE wrong src key type" {
+- r set x 10
+- assert_error "WRONGTYPE*" {r smove x myset2 foo}
++ r set "{tag}x" 10
++ assert_error "WRONGTYPE*" {r smove "{tag}x" "{tag}myset2" foo}
+ }
+
+ test "SMOVE wrong dst key type" {
+- r set x 10
+- assert_error "WRONGTYPE*" {r smove myset2 x foo}
++ r set "{tag}x" 10
++ assert_error "WRONGTYPE*" {r smove "{tag}myset2" "{tag}x" foo}
+ }
+
+ test "SMOVE with identical source and destination" {
diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl
index 7122fd987..2274c82cc 100644
--- a/tests/unit/type/string.tcl
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameIntegrationTest.java
index 1feb6fb..a2af1b8 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameIntegrationTest.java
@@ -17,10 +17,12 @@ package org.apache.geode.redis.internal.commands.executor.key;
import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NO_SUCH_KEY;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
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.AssertionsForClassTypes.assertThatThrownBy;
import static org.junit.Assert.fail;
+import static redis.clients.jedis.Protocol.Command.RENAME;
import java.util.ArrayList;
import java.util.Arrays;
@@ -40,7 +42,6 @@ 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;
@@ -67,7 +68,17 @@ public abstract class AbstractRenameIntegrationTest implements RedisIntegrationT
@Test
public void errors_GivenWrongNumberOfArguments() {
- assertExactNumberOfArgs(jedis, Protocol.Command.RENAME, 2);
+ assertExactNumberOfArgs(jedis, RENAME, 2);
+ }
+
+ @Test
+ public void shouldReturnCrossSlotError_givenKeysInDifferentSlots() {
+ String key1 = "{tag1}key1";
+ String key2 = "{tag2}key2";
+ jedis.set(key1, "value1");
+ jedis.set(key2, "value1");
+ assertThatThrownBy(() -> jedis.sendCommand(key1, RENAME, key1, key2))
+ .hasMessageContaining("CROSSSLOT " + ERROR_WRONG_SLOT);
}
@Test
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameNXIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameNXIntegrationTest.java
index 954cebb..04b27ac 100644
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameNXIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameNXIntegrationTest.java
@@ -17,10 +17,12 @@ package org.apache.geode.redis.internal.commands.executor.key;
import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NO_SUCH_KEY;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
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.AssertionsForClassTypes.assertThatThrownBy;
import static org.junit.Assert.fail;
+import static redis.clients.jedis.Protocol.Command.RENAMENX;
import java.util.ArrayList;
import java.util.Arrays;
@@ -71,6 +73,16 @@ public abstract class AbstractRenameNXIntegrationTest implements RedisIntegratio
}
@Test
+ public void shouldReturnCrossSlotError_givenKeysInDifferentSlots() {
+ String key1 = "{tag1}key1";
+ String key2 = "{tag2}key2";
+ jedis.set(key1, "value1");
+ jedis.set(key2, "value1");
+ assertThatThrownBy(() -> jedis.sendCommand(key1, RENAMENX, key1, key2))
+ .hasMessageContaining("CROSSSLOT " + ERROR_WRONG_SLOT);
+ }
+
+ @Test
public void shouldRename_givenNewKey() {
jedis.set("{tag1}foo", "bar");
long result = jedis.renamenx("{tag1}foo", "{tag1}newfoo");
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffIntegrationTest.java
index 3b4ec8c..7b7f7e3 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffIntegrationTest.java
@@ -15,11 +15,13 @@
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.SDIFF;
import java.util.Arrays;
import java.util.HashSet;
@@ -31,7 +33,6 @@ 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;
@@ -54,7 +55,7 @@ public abstract class AbstractSDiffIntegrationTest implements RedisIntegrationTe
@Test
public void sdiffErrors_givenTooFewArguments() {
- assertAtLeastNArgs(jedis, Protocol.Command.SDIFF, 1);
+ assertAtLeastNArgs(jedis, SDIFF, 1);
}
@Test
@@ -64,6 +65,16 @@ public abstract class AbstractSDiffIntegrationTest implements RedisIntegrationTe
}
@Test
+ public void sdif_withSetsFromDifferentSlots_returnsCrossSlotError() {
+ String setKeyDifferentSlot = "{tag2}set2";
+ jedis.sadd(SET_KEY, "member1");
+ jedis.sadd(setKeyDifferentSlot, "member2");
+
+ assertThatThrownBy(() -> jedis.sendCommand(SET_KEY, SDIFF, SET_KEY, setKeyDifferentSlot))
+ .hasMessage("CROSSSLOT " + ERROR_WRONG_SLOT);
+ }
+
+ @Test
public void sdiffWithNonExistentSet_returnsEmptySet() {
assertThat(jedis.sdiff(NON_EXISTENT_SET_KEY)).isEmpty();
}
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffStoreIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffStoreIntegrationTest.java
index e34c3dd..c60fe6e 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffStoreIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSDiffStoreIntegrationTest.java
@@ -15,11 +15,13 @@
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.SDIFFSTORE;
import java.util.concurrent.atomic.AtomicLong;
@@ -28,7 +30,6 @@ 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;
@@ -56,7 +57,17 @@ public abstract class AbstractSDiffStoreIntegrationTest implements RedisIntegrat
@Test
public void sdiffstoreTooFewArguments_returnsError() {
- assertAtLeastNArgs(jedis, Protocol.Command.SDIFFSTORE, 2);
+ assertAtLeastNArgs(jedis, SDIFFSTORE, 2);
+ }
+
+ @Test
+ public void sdifstore_withSetsFromDifferentSlots_returnsCrossSlotError() {
+ String setKeyDifferentSlot = "{tag2}set2";
+ jedis.sadd(SET_KEY, "member1");
+ jedis.sadd(setKeyDifferentSlot, "member2");
+
+ assertThatThrownBy(() -> jedis.sendCommand(SET_KEY, SDIFFSTORE, SET_KEY, setKeyDifferentSlot))
+ .hasMessage("CROSSSLOT " + ERROR_WRONG_SLOT);
}
@Test
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 168c24f..115317d 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
@@ -15,9 +15,11 @@
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.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;
@@ -57,7 +59,7 @@ public abstract class AbstractSInterIntegrationTest implements RedisIntegrationT
@Test
public void sinterErrors_givenTooFewArguments() {
- assertAtLeastNArgs(jedis, Protocol.Command.SINTER, 1);
+ assertAtLeastNArgs(jedis, SINTER, 1);
}
@Test
@@ -66,6 +68,16 @@ public abstract class AbstractSInterIntegrationTest implements RedisIntegrationT
}
@Test
+ public void sinter_withSetsFromDifferentSlots_returnsCrossSlotError() {
+ String setKeyDifferentSlot = "{tag2}set2";
+ jedis.sadd(SET1, "member1");
+ jedis.sadd(setKeyDifferentSlot, "member2");
+
+ assertThatThrownBy(() -> jedis.sendCommand(SET1, SINTER, SET1, setKeyDifferentSlot))
+ .hasMessage("CROSSSLOT " + ERROR_WRONG_SLOT);
+ }
+
+ @Test
public void testSInter_givenIntersection_returnsIntersectedMembers() {
String[] firstSet = new String[] {"peach"};
String[] secondSet = new String[] {"linux", "apple", "peach"};
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSMoveIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSMoveIntegrationTest.java
index 2ca43e3..ec01096 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSMoveIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSMoveIntegrationTest.java
@@ -15,11 +15,12 @@
package org.apache.geode.redis.internal.commands.executor.set;
import static org.apache.geode.redis.RedisCommandArgumentsTestHelper.assertExactNumberOfArgs;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_DIFFERENT_SLOTS;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
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.SMOVE;
import java.util.concurrent.atomic.AtomicLong;
@@ -29,7 +30,6 @@ 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;
@@ -37,12 +37,12 @@ import org.apache.geode.redis.internal.RedisConstants;
public abstract class AbstractSMoveIntegrationTest implements RedisIntegrationTest {
private JedisCluster jedis;
- private static final String nonExistentSetKey = "{tag1}nonExistentSet";
- private static final String sourceKey = "{tag1}sourceKey";
- private static final String[] sourceMembers = {"one", "two", "three", "four", "five"};
- private static final String destKey = "{tag1}destKey";
- private static final String[] destMembers = {"a", "b", "c"};
- private static final String movedMember = "one";
+ private static final String NON_EXISTENT_SET_KEY = "{tag1}nonExistentSet";
+ private static final String SOURCE_KEY = "{tag1}sourceKey";
+ private static final String[] SOURCE_MEMBERS = {"one", "two", "three", "four", "five"};
+ private static final String DESTINATION_KEY = "{tag1}destKey";
+ private static final String[] DESTINATION_MEMBERS = {"a", "b", "c"};
+ private static final String MOVED_MEMBER = "one";
@Before
public void setUp() {
@@ -57,182 +57,187 @@ public abstract class AbstractSMoveIntegrationTest implements RedisIntegrationTe
@Test
public void smove_givenWrongNumberOfArguments_returnsError() {
- assertExactNumberOfArgs(jedis, Protocol.Command.SMOVE, 3);
+ assertExactNumberOfArgs(jedis, SMOVE, 3);
}
@Test
public void smove_withWrongTypeSource_returnsWrongTypeError() {
- jedis.set(sourceKey, "value");
- jedis.sadd(destKey, destMembers);
+ jedis.set(SOURCE_KEY, "value");
+ jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
- assertThatThrownBy(() -> jedis.smove(sourceKey, destKey, movedMember))
+ assertThatThrownBy(() -> jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER))
.hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE);
}
@Test
public void smove_withWrongTypeDest_returnsWrongTypeError() {
- jedis.sadd(sourceKey, sourceMembers);
- jedis.set(destKey, "value");
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ jedis.set(DESTINATION_KEY, "value");
- assertThatThrownBy(() -> jedis.smove(sourceKey, destKey, movedMember))
+ assertThatThrownBy(() -> jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER))
.hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE);
}
@Test
public void smove_withWrongTypeSourceAndDest_returnsWrongTypeError() {
- jedis.set(sourceKey, "sourceMember");
- jedis.set(destKey, "destMember");
+ jedis.set(SOURCE_KEY, "sourceMember");
+ jedis.set(DESTINATION_KEY, "destMember");
- assertThatThrownBy(() -> jedis.smove(sourceKey, destKey, movedMember))
+ assertThatThrownBy(() -> jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER))
.hasMessageContaining(RedisConstants.ERROR_WRONG_TYPE);
}
@Test
public void smove_withNonExistentSource_returnsZero_sourceKeyDoesNotExist() {
- jedis.sadd(destKey, destMembers);
+ jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
- assertThat(jedis.smove(nonExistentSetKey, destKey, movedMember))
+ assertThat(jedis.smove(NON_EXISTENT_SET_KEY, DESTINATION_KEY, MOVED_MEMBER))
.isEqualTo(0);
- assertThat(jedis.exists(nonExistentSetKey)).isFalse();
+ assertThat(jedis.exists(NON_EXISTENT_SET_KEY)).isFalse();
}
@Test
public void smove_withNonExistentMemberInSource_returnsZero_memberNotAddedToDest() {
String nonExistentMember = "foo";
- jedis.sadd(sourceKey, sourceMembers);
- jedis.sadd(destKey, destMembers);
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
- assertThat(jedis.smove(nonExistentSetKey, destKey, nonExistentMember))
+ assertThat(jedis.smove(NON_EXISTENT_SET_KEY, DESTINATION_KEY, nonExistentMember))
.isEqualTo(0);
- assertThat(jedis.sismember(destKey, nonExistentMember)).isFalse();
+ assertThat(jedis.sismember(DESTINATION_KEY, nonExistentMember)).isFalse();
}
@Test
public void smove_withExistentSourceAndNonExistentDest_returnsOne_memberMovedFromSourceToCreatedDest() {
- jedis.sadd(sourceKey, sourceMembers);
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
- String[] sourceResult = ArrayUtils.remove(sourceMembers, 0);
- String[] destResult = new String[] {movedMember};
+ String[] sourceResult = ArrayUtils.remove(SOURCE_MEMBERS, 0);
+ String[] destResult = new String[] {MOVED_MEMBER};
- assertThat(jedis.smove(sourceKey, destKey, movedMember))
+ assertThat(jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER))
.isEqualTo(1);
- assertThat(jedis.smembers(sourceKey)).containsExactlyInAnyOrder(sourceResult);
- assertThat(jedis.smembers(destKey)).containsExactlyInAnyOrder(destResult);
+ assertThat(jedis.smembers(SOURCE_KEY)).containsExactlyInAnyOrder(sourceResult);
+ assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(destResult);
}
@Test
public void smove_withExistentSourceAndDest_returnsOne_memberMovedFromSourceToDest() {
- jedis.sadd(sourceKey, sourceMembers);
- jedis.sadd(destKey, destMembers);
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
- String[] sourceResult = ArrayUtils.remove(sourceMembers, 0);
- String[] destResult = ArrayUtils.add(destMembers, movedMember);
+ String[] sourceResult = ArrayUtils.remove(SOURCE_MEMBERS, 0);
+ String[] destResult = ArrayUtils.add(DESTINATION_MEMBERS, MOVED_MEMBER);
- assertThat(jedis.smove(sourceKey, destKey, movedMember))
+ assertThat(jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER))
.isEqualTo(1);
- assertThat(jedis.smembers(sourceKey)).containsExactlyInAnyOrder(sourceResult);
- assertThat(jedis.smembers(destKey)).containsExactlyInAnyOrder(destResult);
+ assertThat(jedis.smembers(SOURCE_KEY)).containsExactlyInAnyOrder(sourceResult);
+ assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(destResult);
}
@Test
public void smove_withSameSourceAndDest_withMemberInDest_returnsOne_setNotModified() {
- jedis.sadd(sourceKey, sourceMembers);
- assertThat(jedis.smove(sourceKey, sourceKey, movedMember))
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ assertThat(jedis.smove(SOURCE_KEY, SOURCE_KEY, MOVED_MEMBER))
.isEqualTo(1);
- assertThat(jedis.smembers(sourceKey)).containsExactlyInAnyOrder(sourceMembers);
+ assertThat(jedis.smembers(SOURCE_KEY)).containsExactlyInAnyOrder(SOURCE_MEMBERS);
}
@Test
public void smove_withExistentSourceAndDest_withMemberInDest_returnsOne_memberRemovedFromSource() {
- jedis.sadd(sourceKey, sourceMembers);
- String[] newDestMembers = ArrayUtils.add(destMembers, movedMember);
- jedis.sadd(destKey, newDestMembers);
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ String[] newDestMembers = ArrayUtils.add(DESTINATION_MEMBERS, MOVED_MEMBER);
+ jedis.sadd(DESTINATION_KEY, newDestMembers);
- String[] sourceResult = ArrayUtils.remove(sourceMembers, 0);
+ String[] sourceResult = ArrayUtils.remove(SOURCE_MEMBERS, 0);
- assertThat(jedis.smove(sourceKey, destKey, movedMember))
+ assertThat(jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER))
.isEqualTo(1);
- assertThat(jedis.smembers(sourceKey)).containsExactlyInAnyOrder(sourceResult);
- assertThat(jedis.smembers(destKey)).containsExactlyInAnyOrder(newDestMembers);
+ assertThat(jedis.smembers(SOURCE_KEY)).containsExactlyInAnyOrder(sourceResult);
+ assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(newDestMembers);
}
@Test
public void smoveWithSetsFromDifferentSlots_returnsCrossSlotError() {
String setKeyDifferentSlot = "{tag2}setKey2";
- jedis.sadd(sourceKey, setKeyDifferentSlot);
- jedis.sadd(sourceKey, sourceMembers);
- jedis.sadd(setKeyDifferentSlot, destMembers);
+ jedis.sadd(SOURCE_KEY, setKeyDifferentSlot);
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ jedis.sadd(setKeyDifferentSlot, DESTINATION_MEMBERS);
- assertThatThrownBy(() -> jedis.smove(sourceKey, setKeyDifferentSlot, movedMember))
- .hasMessageContaining(ERROR_DIFFERENT_SLOTS);
+ assertThatThrownBy(
+ () -> jedis.sendCommand(SOURCE_KEY, SMOVE, SOURCE_KEY, setKeyDifferentSlot, MOVED_MEMBER))
+ .hasMessage("CROSSSLOT " + ERROR_WRONG_SLOT);
}
@Test
public void ensureSetConsistency_whenRunningConcurrently_withSRemAndSMove() {
- String[] sourceMemberRemoved = ArrayUtils.remove(sourceMembers, 0);
- String[] destMemberAdded = ArrayUtils.add(destMembers, movedMember);
+ String[] sourceMemberRemoved = ArrayUtils.remove(SOURCE_MEMBERS, 0);
+ String[] destMemberAdded = ArrayUtils.add(DESTINATION_MEMBERS, MOVED_MEMBER);
- jedis.sadd(sourceKey, sourceMembers);
- jedis.sadd(destKey, destMembers);
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
final AtomicLong moved = new AtomicLong(0);
new ConcurrentLoopingThreads(1000,
- i -> jedis.srem(sourceKey, movedMember),
- i -> moved.set(jedis.smove(sourceKey, destKey, movedMember)))
+ i -> jedis.srem(SOURCE_KEY, MOVED_MEMBER),
+ i -> moved.set(jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER)))
.runWithAction(() -> {
if (moved.get() == 1) {
- assertThat(jedis.smembers(sourceKey))
+ assertThat(jedis.smembers(SOURCE_KEY))
.containsExactlyInAnyOrder(sourceMemberRemoved);
- assertThat(jedis.smembers(destKey)).containsExactlyInAnyOrder(destMemberAdded);
+ assertThat(jedis.smembers(DESTINATION_KEY))
+ .containsExactlyInAnyOrder(destMemberAdded);
} else {
- assertThat(jedis.smembers(sourceKey))
+ assertThat(jedis.smembers(SOURCE_KEY))
.containsExactlyInAnyOrder(sourceMemberRemoved);
- assertThat(jedis.smembers(destKey)).containsExactlyInAnyOrder(destMembers);
+ assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(
+ DESTINATION_MEMBERS);
}
- jedis.sadd(sourceKey, movedMember);
- jedis.srem(destKey, movedMember);
+ jedis.sadd(SOURCE_KEY, MOVED_MEMBER);
+ jedis.srem(DESTINATION_KEY, MOVED_MEMBER);
});
}
@Test
public void ensureSetConsistency_whenRunningConcurrently_withSMovesFromSameSourceAndDifferentDestination() {
- String[] sourceMemberRemoved = ArrayUtils.remove(sourceMembers, 0);
- String[] destMemberAdded = ArrayUtils.add(destMembers, movedMember);
- String[] nonExisistentMemberAdded = {movedMember};
+ String[] sourceMemberRemoved = ArrayUtils.remove(SOURCE_MEMBERS, 0);
+ String[] destMemberAdded = ArrayUtils.add(DESTINATION_MEMBERS, MOVED_MEMBER);
+ String[] nonExisistentMemberAdded = {MOVED_MEMBER};
- jedis.sadd(sourceKey, sourceMembers);
- jedis.sadd(destKey, destMembers);
+ jedis.sadd(SOURCE_KEY, SOURCE_MEMBERS);
+ jedis.sadd(DESTINATION_KEY, DESTINATION_MEMBERS);
final AtomicLong movedToNonExistent = new AtomicLong(0);
final AtomicLong movedToDest = new AtomicLong(0);
new ConcurrentLoopingThreads(1000,
- i -> movedToNonExistent.set(jedis.smove(sourceKey, nonExistentSetKey, movedMember)),
- i -> movedToDest.set(jedis.smove(sourceKey, destKey, movedMember)))
+ i -> movedToNonExistent.set(jedis.smove(SOURCE_KEY, NON_EXISTENT_SET_KEY, MOVED_MEMBER)),
+ i -> movedToDest.set(jedis.smove(SOURCE_KEY, DESTINATION_KEY, MOVED_MEMBER)))
.runWithAction(() -> {
// Asserts that only one smove was preformed
assertThat(movedToNonExistent.get() ^ movedToDest.get()).isEqualTo(1);
- assertThat(jedis.smembers(sourceKey)).containsExactlyInAnyOrder(sourceMemberRemoved);
+ assertThat(jedis.smembers(SOURCE_KEY)).containsExactlyInAnyOrder(sourceMemberRemoved);
if (movedToNonExistent.get() == 1) {
- assertThat(jedis.smembers(nonExistentSetKey))
+ assertThat(jedis.smembers(NON_EXISTENT_SET_KEY))
.containsExactlyInAnyOrder(nonExisistentMemberAdded);
} else {
- assertThat(jedis.exists(nonExistentSetKey)).isFalse();
+ assertThat(jedis.exists(NON_EXISTENT_SET_KEY)).isFalse();
}
if (movedToDest.get() == 1) {
- assertThat(jedis.smembers(destKey)).containsExactlyInAnyOrder(destMemberAdded);
+ assertThat(jedis.smembers(DESTINATION_KEY))
+ .containsExactlyInAnyOrder(destMemberAdded);
} else {
- assertThat(jedis.smembers(destKey)).containsExactlyInAnyOrder(destMembers);
+ assertThat(jedis.smembers(DESTINATION_KEY)).containsExactlyInAnyOrder(
+ DESTINATION_MEMBERS);
}
- jedis.sadd(sourceKey, movedMember);
- jedis.srem(destKey, movedMember);
- jedis.srem(nonExistentSetKey, movedMember);
+ jedis.sadd(SOURCE_KEY, MOVED_MEMBER);
+ jedis.srem(DESTINATION_KEY, MOVED_MEMBER);
+ jedis.srem(NON_EXISTENT_SET_KEY, MOVED_MEMBER);
});
}
}
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 7e6ac15..40f0314 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
@@ -15,12 +15,13 @@
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_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.SUNION;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
@@ -30,7 +31,6 @@ 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;
@@ -55,7 +55,7 @@ public abstract class AbstractSUnionIntegrationTest implements RedisIntegrationT
@Test
public void sunionErrors_givenTooFewArguments() {
- assertAtLeastNArgs(jedis, Protocol.Command.SUNION, 1);
+ assertAtLeastNArgs(jedis, SUNION, 1);
}
@Test
@@ -126,8 +126,8 @@ public abstract class AbstractSUnionIntegrationTest implements RedisIntegrationT
jedis.sadd(SET_KEY_1, SET_MEMBERS_1);
jedis.sadd(setKeyDifferentSlot, secondSetMembers);
- assertThatThrownBy(() -> jedis.sunion(SET_KEY_1, setKeyDifferentSlot))
- .hasMessageContaining(ERROR_DIFFERENT_SLOTS);
+ assertThatThrownBy(() -> jedis.sendCommand(SET_KEY_1, SUNION, SET_KEY_1, setKeyDifferentSlot))
+ .hasMessage("CROSSSLOT " + ERROR_WRONG_SLOT);
}
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 7d0ee9b..b6526c1 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
@@ -15,12 +15,13 @@
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_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.SUNIONSTORE;
import java.util.concurrent.atomic.AtomicLong;
@@ -206,8 +207,9 @@ public abstract class AbstractSUnionStoreIntegrationTest implements RedisIntegra
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);
+ assertThatThrownBy(() -> jedis.sendCommand(DESTINATION_KEY, SUNIONSTORE, DESTINATION_KEY,
+ SET_KEY_1, setKeyDifferentSlot))
+ .hasMessage("CROSSSLOT " + ERROR_WRONG_SLOT);
}
@Test
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetIntegrationTest.java
index 806bbad..65462f3 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetIntegrationTest.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.redis.internal.commands.executor.string;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
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;
@@ -71,7 +72,7 @@ public abstract class AbstractMSetIntegrationTest implements RedisIntegrationTes
public void givenDifferentSlots_returnsError() {
assertThatThrownBy(
() -> jedis.sendCommand("key1", Protocol.Command.MSET, "key1", "value1", "key2", "value2"))
- .hasMessageContaining("CROSSSLOT Keys in request don't hash to the same slot");
+ .hasMessageContaining("CROSSSLOT " + ERROR_WRONG_SLOT);
}
@Test
diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetNXIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetNXIntegrationTest.java
index 2aa30d4..96f7ae5 100755
--- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetNXIntegrationTest.java
+++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetNXIntegrationTest.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.redis.internal.commands.executor.string;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
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;
@@ -73,7 +74,7 @@ public abstract class AbstractMSetNXIntegrationTest implements RedisIntegrationT
assertThatThrownBy(
() -> jedis.sendCommand("key1", Protocol.Command.MSETNX, "key1", "value1", "key2",
"value2"))
- .hasMessageContaining("CROSSSLOT Keys in request don't hash to the same slot");
+ .hasMessageContaining("CROSSSLOT " + ERROR_WRONG_SLOT);
}
@Test
diff --git a/geode-for-redis/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt b/geode-for-redis/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
index b0aec9c..3d87db1 100644
--- a/geode-for-redis/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
+++ b/geode-for-redis/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
@@ -2,6 +2,7 @@ org/apache/geode/redis/internal/services/cluster/RedisMemberInfoRetrievalFunctio
org/apache/geode/redis/internal/data/collections/Bytes2ObjectOpenHashMap
org/apache/geode/redis/internal/data/collections/SizeableBytes2ObjectOpenCustomHashMapWithCursor
org/apache/geode/redis/internal/data/collections/SizeableObjectOpenCustomHashSet
+org/apache/geode/redis/internal/data/RedisCrossSlotException
org/apache/geode/redis/internal/data/RedisDataMovedException
org/apache/geode/redis/internal/data/RedisDataTypeMismatchException
org/apache/geode/redis/internal/commands/executor/sortedset/ZAggregator
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
index 25894a3..3a0d4fe 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/RedisConstants.java
@@ -78,8 +78,6 @@ public class RedisConstants {
"DUMP payload version or checksum are wrong";
public static final String ERROR_WRONG_SLOT =
"Keys in request don't hash to the same slot";
- public static final String ERROR_DIFFERENT_SLOTS =
- "No way to dispatch this command to Redis Cluster because keys have different slots.";
public static final String ERROR_WEIGHT_NOT_A_FLOAT =
"weight value is not a float";
public static final String ERROR_INVALID_USERNAME_OR_PASSWORD =
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameExecutor.java
index 62c5a47..d022e20 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/key/AbstractRenameExecutor.java
@@ -16,7 +16,6 @@
package org.apache.geode.redis.internal.commands.executor.key;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NO_SUCH_KEY;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
import java.util.Arrays;
import java.util.List;
@@ -41,10 +40,6 @@ public abstract class AbstractRenameExecutor implements CommandExecutor {
return getTargetSameAsSourceResponse();
}
- if (key.getSlot() != newKey.getSlot()) {
- return RedisResponse.crossSlot(ERROR_WRONG_SLOT);
- }
-
try {
if (!executeRenameCommand(key, newKey, context)) {
return getNoSuchKeyResponse();
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SMoveExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SMoveExecutor.java
index 2a9b4e4..42647ba 100755
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SMoveExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/set/SMoveExecutor.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.redis.internal.commands.executor.set;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
import static org.apache.geode.redis.internal.data.RedisSet.smove;
import java.util.Arrays;
@@ -23,7 +22,6 @@ import java.util.List;
import org.apache.geode.redis.internal.commands.Command;
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.RedisDataMovedException;
import org.apache.geode.redis.internal.data.RedisKey;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
import org.apache.geode.redis.internal.services.RegionProvider;
@@ -38,13 +36,6 @@ public class SMoveExecutor implements CommandExecutor {
byte[] member = commandElems.get(3);
RegionProvider regionProvider = context.getRegionProvider();
- try {
- regionProvider.ensureKeyIsLocal(sourceKey);
- regionProvider.ensureKeyIsLocal(destKey);
- } catch (RedisDataMovedException ex) {
- return RedisResponse.crossSlot(ERROR_WRONG_SLOT);
- }
-
int removed = context.lockedExecute(sourceKey, Arrays.asList(sourceKey, destKey),
() -> smove(sourceKey, destKey, member, regionProvider));
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 b83c8e2..9f6fce0 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
@@ -29,7 +29,6 @@ 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.RedisDataMovedException;
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;
@@ -49,13 +48,6 @@ public abstract class SetOpExecutor implements CommandExecutor {
List<RedisKey> commandElements = command.getProcessedCommandKeys();
List<RedisKey> setKeys = commandElements.subList(setsStartIndex, commandElements.size());
RegionProvider regionProvider = context.getRegionProvider();
- try {
- for (RedisKey k : setKeys) {
- regionProvider.ensureKeyIsLocal(k);
- }
- } catch (RedisDataMovedException ex) {
- return RedisResponse.error(ex.getMessage());
- }
/*
* SINTERSTORE currently use the else part of the code for their implementation.
@@ -63,20 +55,24 @@ public abstract class SetOpExecutor implements CommandExecutor {
* 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, new ArrayList<>(setKeys),
+ int resultSize = context.lockedExecute(destinationKey, keysToLock,
() -> sdiffstore(regionProvider, destinationKey, setKeys));
return RedisResponse.integer(resultSize);
}
- Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys),
+ 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), new ArrayList<>(setKeys),
+ Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), keysToLock,
() -> sinter(regionProvider, setKeys));
return RedisResponse.array(resultSet, true);
@@ -84,12 +80,12 @@ public abstract class SetOpExecutor implements CommandExecutor {
|| command.isOfType(RedisCommandType.SUNIONSTORE)) {
if (isStorage()) {
RedisKey destinationKey = command.getKey();
- int resultSize = context.lockedExecute(destinationKey, new ArrayList<>(setKeys),
+ int resultSize = context.lockedExecute(destinationKey, keysToLock,
() -> sunionstore(regionProvider, destinationKey, setKeys));
return RedisResponse.integer(resultSize);
}
- Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), new ArrayList<>(setKeys),
+ Set<byte[]> resultSet = context.lockedExecute(setKeys.get(0), keysToLock,
() -> sunion(regionProvider, setKeys));
return RedisResponse.array(resultSet, true);
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZInterStoreExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZInterStoreExecutor.java
index 0ddff26..64bbc6a 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZInterStoreExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZInterStoreExecutor.java
@@ -32,7 +32,7 @@ public class ZInterStoreExecutor extends ZStoreExecutor {
List<ZKeyWeight> keyWeights, ZAggregator aggregator) {
RegionProvider regionProvider = context.getRegionProvider();
RedisKey key = command.getKey();
- List<RedisKey> keysToLock = getKeysToLock(regionProvider, key, keyWeights);
+ List<RedisKey> keysToLock = getKeysToLock(key, keyWeights);
return context.lockedExecute(key, keysToLock,
() -> zinterstore(regionProvider, key, keyWeights, aggregator));
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZStoreExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZStoreExecutor.java
index c9aa5d7..335a35e 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZStoreExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZStoreExecutor.java
@@ -17,7 +17,6 @@ package org.apache.geode.redis.internal.commands.executor.sortedset;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_NOT_INTEGER;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_SYNTAX;
import static org.apache.geode.redis.internal.RedisConstants.ERROR_WEIGHT_NOT_A_FLOAT;
-import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
import static org.apache.geode.redis.internal.netty.Coder.narrowLongToInt;
import static org.apache.geode.redis.internal.netty.Coder.toUpperCaseBytes;
import static org.apache.geode.redis.internal.netty.StringBytesGlossary.AGGREGATE;
@@ -34,7 +33,6 @@ import org.apache.geode.redis.internal.commands.executor.RedisResponse;
import org.apache.geode.redis.internal.data.RedisKey;
import org.apache.geode.redis.internal.netty.Coder;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
-import org.apache.geode.redis.internal.services.RegionProvider;
public abstract class ZStoreExecutor implements CommandExecutor {
@@ -105,24 +103,14 @@ public abstract class ZStoreExecutor implements CommandExecutor {
}
}
- int slot = command.getKey().getSlot();
- for (ZKeyWeight keyWeight : keyWeights) {
- if (keyWeight.getKey().getSlot() != slot) {
- return RedisResponse.crossSlot(ERROR_WRONG_SLOT);
- }
- }
-
return RedisResponse.integer(getResult(context, command, keyWeights, aggregator));
}
- protected List<RedisKey> getKeysToLock(RegionProvider regionProvider, RedisKey destinationKey,
- List<ZKeyWeight> keyWeights) {
+ protected List<RedisKey> getKeysToLock(RedisKey destinationKey, List<ZKeyWeight> keyWeights) {
List<RedisKey> keysToLock = new ArrayList<>(keyWeights.size());
for (ZKeyWeight kw : keyWeights) {
- regionProvider.ensureKeyIsLocal(kw.getKey());
keysToLock.add(kw.getKey());
}
- regionProvider.ensureKeyIsLocal(destinationKey);
keysToLock.add(destinationKey);
return keysToLock;
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZUnionStoreExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZUnionStoreExecutor.java
index 6d30155..dee9f1a 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZUnionStoreExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/sortedset/ZUnionStoreExecutor.java
@@ -31,7 +31,7 @@ public class ZUnionStoreExecutor extends ZStoreExecutor {
List<ZKeyWeight> keyWeights, ZAggregator aggregator) {
RegionProvider regionProvider = context.getRegionProvider();
RedisKey key = command.getKey();
- List<RedisKey> keysToLock = getKeysToLock(regionProvider, key, keyWeights);
+ List<RedisKey> keysToLock = getKeysToLock(key, keyWeights);
return context.lockedExecute(key, keysToLock,
() -> zunionstore(regionProvider, key, keyWeights, aggregator));
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetExecutor.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetExecutor.java
index 4d7c4da..e23db4b 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetExecutor.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/commands/executor/string/AbstractMSetExecutor.java
@@ -66,15 +66,9 @@ public abstract class AbstractMSetExecutor implements CommandExecutor {
protected void mset(ExecutionHandlerContext context, List<RedisKey> keys, List<byte[]> values,
boolean nx) {
- List<RedisKey> keysToLock = new ArrayList<>(keys.size());
+ List<RedisKey> keysToLock = new ArrayList<>(keys);
RegionProvider regionProvider = context.getRegionProvider();
- for (RedisKey key : keys) {
- regionProvider.ensureKeyIsLocal(key);
- keysToLock.add(key);
- }
- // Pass a key in so that the bucket will be locked. Since all keys are already guaranteed to be
- // in the same bucket we can use any key for this.
context.lockedExecuteInTransaction(keysToLock.get(0), keysToLock,
() -> mset0(regionProvider, keys, values, nx));
}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisCrossSlotException.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisCrossSlotException.java
new file mode 100644
index 0000000..e05d46b
--- /dev/null
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisCrossSlotException.java
@@ -0,0 +1,30 @@
+/*
+ * 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.data;
+
+import org.apache.geode.redis.internal.RedisException;
+
+public class RedisCrossSlotException extends RedisException {
+
+ private static final long serialVersionUID = -2126545465506588852L;
+
+ public RedisCrossSlotException() {
+ super();
+ }
+
+ public RedisCrossSlotException(String message) {
+ super(message);
+ }
+}
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
index 1011eb7..000adf5 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/netty/ExecutionHandlerContext.java
@@ -51,6 +51,7 @@ import org.apache.geode.redis.internal.commands.RedisCommandType;
import org.apache.geode.redis.internal.commands.executor.RedisResponse;
import org.apache.geode.redis.internal.commands.executor.UnknownExecutor;
import org.apache.geode.redis.internal.commands.parameters.RedisParametersMismatchException;
+import org.apache.geode.redis.internal.data.RedisCrossSlotException;
import org.apache.geode.redis.internal.data.RedisData;
import org.apache.geode.redis.internal.data.RedisDataMovedException;
import org.apache.geode.redis.internal.data.RedisDataType;
@@ -182,6 +183,8 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter {
return RedisResponse.moved(rootCause.getMessage());
} else if (rootCause instanceof RedisDataTypeMismatchException) {
return RedisResponse.wrongType(rootCause.getMessage());
+ } else if (rootCause instanceof RedisCrossSlotException) {
+ return RedisResponse.crossSlot(rootCause.getMessage());
} else if (rootCause instanceof IllegalStateException
|| rootCause instanceof RedisParametersMismatchException
|| rootCause instanceof RedisException
diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java
index 0f8fdd2..55e37e8 100644
--- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java
+++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/RegionProvider.java
@@ -14,6 +14,7 @@
*/
package org.apache.geode.redis.internal.services;
+import static org.apache.geode.redis.internal.RedisConstants.ERROR_WRONG_SLOT;
import static org.apache.geode.redis.internal.RedisProperties.REDIS_REGION_NAME_PROPERTY;
import static org.apache.geode.redis.internal.RedisProperties.REGION_BUCKETS;
import static org.apache.geode.redis.internal.RedisProperties.getIntegerSystemProperty;
@@ -27,6 +28,7 @@ import java.util.List;
import java.util.Set;
import java.util.concurrent.Callable;
+import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.cache.CacheTransactionManager;
import org.apache.geode.cache.PartitionAttributesFactory;
import org.apache.geode.cache.Region;
@@ -46,6 +48,7 @@ import org.apache.geode.internal.cache.execute.BucketMovedException;
import org.apache.geode.redis.internal.RedisConstants;
import org.apache.geode.redis.internal.RedisException;
import org.apache.geode.redis.internal.commands.executor.cluster.RedisPartitionResolver;
+import org.apache.geode.redis.internal.data.RedisCrossSlotException;
import org.apache.geode.redis.internal.data.RedisData;
import org.apache.geode.redis.internal.data.RedisDataMovedException;
import org.apache.geode.redis.internal.data.RedisDataType;
@@ -63,13 +66,12 @@ public class RegionProvider {
* The name of the region that holds data stored in redis.
*/
public static final String DEFAULT_REDIS_REGION_NAME = "GEODE_FOR_REDIS";
- public static final String REDIS_REGION_BUCKETS_PARAM = REGION_BUCKETS;
public static final int REDIS_SLOTS = 16384;
// Ideally the bucket count should be a power of 2, but technically it is not required.
public static final int REDIS_REGION_BUCKETS =
- getIntegerSystemProperty(REDIS_REGION_BUCKETS_PARAM, 128, 1, REDIS_SLOTS);
+ getIntegerSystemProperty(REGION_BUCKETS, 128, 1, REDIS_SLOTS);
public static final int REDIS_SLOTS_PER_BUCKET = REDIS_SLOTS / REDIS_REGION_BUCKETS;
@@ -144,6 +146,9 @@ public class RegionProvider {
public <T> T lockedExecute(RedisKey key, List<RedisKey> keysToLock, Callable<T> callable) {
try {
+ if (areKeysCrossSlots(keysToLock)) {
+ throw new RedisCrossSlotException(ERROR_WRONG_SLOT);
+ }
return partitionedRegion.computeWithPrimaryLocked(key,
() -> stripedCoordinator.execute(keysToLock, callable));
} catch (PrimaryBucketLockException | BucketMovedException | RegionDestroyedException ex) {
@@ -155,6 +160,17 @@ public class RegionProvider {
}
}
+ @VisibleForTesting
+ static boolean areKeysCrossSlots(List<RedisKey> keysToLock) {
+ int slot = keysToLock.get(0).getSlot();
+ for (RedisKey key : keysToLock) {
+ if (key.getSlot() != slot) {
+ return true;
+ }
+ }
+ return false;
+ }
+
/**
* Execute the given Callable in the context of a GemFire transaction. On failure there is no
* attempt to retry.
diff --git a/geode-for-redis/src/test/java/org/apache/geode/redis/internal/services/RegionProviderTest.java b/geode-for-redis/src/test/java/org/apache/geode/redis/internal/services/RegionProviderTest.java
new file mode 100644
index 0000000..ea1433c
--- /dev/null
+++ b/geode-for-redis/src/test/java/org/apache/geode/redis/internal/services/RegionProviderTest.java
@@ -0,0 +1,74 @@
+/*
+ * 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.services;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Test;
+
+import org.apache.geode.redis.internal.data.RedisKey;
+
+public class RegionProviderTest {
+
+ @Test
+ public void areKeysCrossSlotsReturnsFalseWhenKeysAreSameSlot() {
+ RedisKey key1 = mock(RedisKey.class);
+ int slot1 = 1;
+ when(key1.getSlot()).thenReturn(slot1);
+ RedisKey key2 = mock(RedisKey.class);
+ when(key2.getSlot()).thenReturn(slot1);
+
+ List<RedisKey> keyList = Arrays.asList(key1, key2);
+
+ assertThat(RegionProvider.areKeysCrossSlots(keyList)).isFalse();
+ }
+
+ @Test
+ public void areKeysCrossSlotsReturnsTrueWhenKeysAreCrossSlots() {
+ RedisKey key1 = mock(RedisKey.class);
+ int slot1 = 1;
+ when(key1.getSlot()).thenReturn(slot1);
+ RedisKey key2 = mock(RedisKey.class);
+ int slot2 = 2;
+ when(key2.getSlot()).thenReturn(slot2);
+
+ List<RedisKey> keyList = Arrays.asList(key1, key2);
+
+ assertThat(RegionProvider.areKeysCrossSlots(keyList)).isTrue();
+ }
+
+ @Test
+ public void areKeysCrossSlotsReturnsTrueWhenKeysAreCrossSlotsForManyKeys() {
+ List<RedisKey> keyList = new ArrayList<>();
+ for (int i = 0; i < 100; ++i) {
+ RedisKey key = mock(RedisKey.class);
+ int slot1 = 1;
+ when(key.getSlot()).thenReturn(slot1);
+ keyList.add(key);
+ }
+ RedisKey finalKey = mock(RedisKey.class);
+ int slot2 = 2;
+ when(finalKey.getSlot()).thenReturn(slot2);
+ keyList.add(finalKey);
+
+ assertThat(RegionProvider.areKeysCrossSlots(keyList)).isTrue();
+ }
+}
[geode] 03/09: GEODE-9837: SUNIONSTORE Command Support (#7284)
Posted by je...@apache.org.
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,
[geode] 08/09: GEODE-9883 Update Geode for Redis docs file (#7274)
Posted by je...@apache.org.
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 9b8a768aa059ef1a08b8574d87f90a91d7409276
Author: Eric Zoerner <zo...@vmware.com>
AuthorDate: Wed Jan 26 16:05:17 2022 -0800
GEODE-9883 Update Geode for Redis docs file (#7274)
Co-authored-by: Dave Barnes <db...@apache.org>
(cherry picked from commit 1eeccabe35466883611803ed2516145564c4cfa3)
---
.../source/subnavs/geode-subnav.erb | 24 +-
.../tools_modules/geode_for_redis.html.md.erb | 294 +++++++++++----------
2 files changed, 158 insertions(+), 160 deletions(-)
diff --git a/geode-book/master_middleman/source/subnavs/geode-subnav.erb b/geode-book/master_middleman/source/subnavs/geode-subnav.erb
index 094959b..372a42a 100644
--- a/geode-book/master_middleman/source/subnavs/geode-subnav.erb
+++ b/geode-book/master_middleman/source/subnavs/geode-subnav.erb
@@ -2105,28 +2105,8 @@ limitations under the License.
</li>
</ul>
</li>
- <li class="has_submenu">
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html"><%=vars.product_name%> for Redis</a>
- <ul>
- <li>
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#using-the-api">Using <%=vars.product_name%> for Redis</a>
- </li>
- <li>
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#supported-commands">Supported Redis Commands</a>
- </li>
- <li>
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#advantages-over-redis">Advantages of <%=vars.product_name%> over Redis</a>
- </li>
- <li>
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#expiration-accuracy">Expiration Accuracy</a>
- </li>
- <li>
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#high-availability-model">High Availability Model</a>
- </li>
- <li>
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#loss-of-connections">Loss of Connections</a>
- </li>
- </ul>
+ <li>
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html">Geode for Redis</a>
</li>
<li class="has_submenu">
<a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/gemcached/chapter_overview.html">Gemcached</a>
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 475fc7d..482d817 100644
--- a/geode-docs/tools_modules/geode_for_redis.html.md.erb
+++ b/geode-docs/tools_modules/geode_for_redis.html.md.erb
@@ -25,215 +25,233 @@ optional password authentication.
<img src="../images_svg/geode_for_redis.svg" class="image" />
-## <a id="using-the-api" class="no-quick-link"></a>Using <%=vars.product_name%> for Redis
+## <a id="using-geode-for-redis"></a>Using <%=vars.product_name%> for Redis
The <%=vars.product_name%> cluster must have at least one server that is set up to handle the incoming Redis commands.
-Use gfsh to start at least one server with a command of the form:
+Prerequisites for running the examples:
-```pre
-start server \
- --name=<serverName> \
- --locators=<locatorPort> \
- --J=-Dgemfire.geode-for-redis-enabled=true \
- --J=-Dgemfire.geode-for-redis-port=<geodeForRedisPort> \
- --J=-Dgemfire.geode-for-redis-bind-address=<geodeForRedisBindAddress>
-```
-
-If the gemfire property `geode-for-redis-enabled`, is set to `true`, a <%=vars.product_name%>
-server with <%=vars.product_name%> for Redis will be started.
+1. **Install <%=vars.product_name%>** <br/>
+Using the instructions in the `README.md` file in the root of the <%=vars.product_name%> checkout directory, build and install <%=vars.product_name%>.
+2. **Install the Redis CLI** <br/>
+Follow installation instructions at https://redis.io/download
-Replace `<serverName>` with the name of your server.
+Use `gfsh` to start a locator for managing a <%=vars.product_name%> cluster:
-Replace `<locatorPort>` with your locator port.
+```commandLine
+gfsh> start locator
+```
-Replace `<geodeForRedisPort>` with the port that the <%=vars.product_name%> server
- listens on for Redis commands. The typical port used with a cluster compatible with Redis is 6379.
+Use `gfsh` to start at least one server with a command of the form:
-Replace `<geodeForRedisBindAddress>` with the address of the server host.
+```commandLine
+gfsh> start server --J=-Dgemfire.geode-for-redis-enabled=true --J=-Dgemfire.geode-for-redis-port=6379
+```
-Replace `<geodeForRedisPassword>` with the password clients use to authenticate.
+More information about the options when starting a server is given in the section [Start Server Options](#redis-start-server-options) below.
-To confirm the server is listening, run:
+To confirm the server is listening, in a separate terminal run:
-``` pre
-redis-cli -h <geodeForRedisBindAddress> -p <geodeForRedisPort> -a <geodeForRedisPassword> ping
+```commandLine
+$ redis-cli -c ping
```
-Replace `<geodeForRedisBindAddress>`,`<geodeForRedisPort>`, and `<geodeForRedisPassword>` with the same values as the server.
+The `-c` option enables cluster mode in the redis-cli, which is necessary since
+<%=vars.product_name%> for Redis runs as a Redis Cluster.
If the server is functioning properly, you should see a response of `PONG`.
-## <a id="supported-commands" class="no-quick-link"></a>Supported Redis Commands
+### <a id="adding-a-server"></a> Add an additional server
+If you’re interested in testing <%=vars.product_name%> scalability, in gfsh run the `start server` command again.
-<%=vars.product_name%> for Redis supports the following Redis commands.
-<br/>
-
- - APPEND <br/>
- - AUTH <br/>
- - DECR <br/>
- - DECRBY <br/>
- - DEL <br/>
- - EXISTS <br/>
- - EXPIRE <br/>
- - EXPIREAT <br/>
- - GET <br/>
- - GETRANGE <br/>
- - HDEL <br/>
- - HEXISTS <br/>
- - HGET <br/>
- - HGETALL <br/>
- - HINCRBY <br/>
- - HINCRBYFLOAT <br/>
- - HLEN <br/>
- - HMGET <br/>
- - HMSET <br/>
- - HSCAN **[1]** <br/>
- - HSET <br/>
- - HSETNX <br/>
- - HSTRLEN <br/>
- - HVALS <br/>
- - HKEYS <br/>
- - INCR <br/>
- - INCRBY <br/>
- - INCRBYFLOAT <br/>
- - INFO **[2]** <br/>
- - KEYS <br/>
- - MGET <br/>
- - PERSIST <br/>
- - PEXPIRE <br/>
- - PEXPIREAT <br/>
- - PING <br/>
- - PSUBSCRIBE <br/>
- - PTTL <br/>
- - PUBLISH <br/>
- - PUNSUBSCRIBE <br/>
- - QUIT <br/>
- - RENAME <br/>
- - SADD <br/>
- - SCARD <br/>
- - SDIFF <br/>
- - SDIFFSTORE <br/>
- - SINTER <br/>
- - SISMEMBER <br/>
- - SET <br/>
- - SETNX <br/>
- - SLOWLOG **[3]** <br/>
- - SMEMBERS <br/>
- - SMOVE <br/>
- - SRANDMEMBER <br/>
- - SREM <br/>
- - STRLEN <br/>
- - SUBSCRIBE <br/>
- - SUNION <br/>
- - SUNIONSTORE <br/>
- - TTL <br/>
- - TYPE <br/>
- - UNSUBSCRIBE <br/>
-
-<br/>
-Commands not listed above are **not implemented**.
+However, there are two ports that must be unique for each server in the cluster, the
+`gemfire.geode-for-redis-port`, used for receiving Redis commands, and the
+`server-port`, which is used for cluster communication.
-<br/>
-**NOTES:**
+The first server used `6379` for the redis port; we'll use `6380` for the second server.
-These commands are supported for Redis 5.
+The first server was started without
+a server port specified, so it used the default `40404`. To start up an additional server, you need to specify
+a different server port, or use `--server-port=0` which tells <%=vars.product_name%> to use
+an arbitrary available port for the server port.
-**[1]** Redis accepts 64-bit signed integers for the HSCAN cursor and COUNT parameters.
- <%=vars.product_name%> for Redis is limited to 32-bit integer values for these parameters.
+For example:
-**[2]** INFO is implemented for the sections and fields listed below:
+```commandLine
+gfsh> start server --J=-Dgemfire.geode-for-redis-enabled=true --J=-Dgemfire.geode-for-redis-port=6380 --server-port=0
+```
- - clients
+### <a id="shutting-down"></a>Shutting Down
+To shut down the <%=vars.product_name%> cluster you started, in the terminal with gfsh running type the following command
- - connected_clients
+```commandLine
+gfsh> shutdown --include-locators=true
+```
- - blocked_clients (always returns 0)
+This command shuts down the entire <%=vars.product_name%> cluster.
- - cluster
+To confirm that everything shut down correctly, if you execute a Redis command in the `redis-cli` you should see the following message:
- - cluster_enables (always returns 0)
+```commandline
+Could not connect to Redis at 127.0.0.1:6379: Connection refused
+```
- - keyspace
+## <a id="redis-start-server-options"></a>Start Server Options
- - returns stats for db: 0
+The options that are specific to starting a server for <%=vars.product_name%> for Redis are listed below.
+For other options see [start server](gfsh/command-pages/start.html#topic_3764EE2DB18B4AE4A625E0354471738A).
- - memory
+`--J=-Dgemfire.geode-for-redis-enabled` (Default: `false`) <br/>
+If set to `true`, a <%=vars.product_name%> server with <%=vars.product_name%> for Redis will be started.
- - maxmemory
+`--J=-Dgemfire.geode-for-redis-port` (Default: `6379`) <br/>
+Specifies the port on which the <%=vars.product_name%> server
+listens for Redis commands. Note that the default port `6379` is the same port that native Redis
+uses by default.
- - used_memory
+`--J=-Dgemfire.geode-for-redis-bind-address` (Default: `""`) <br/>
+Specifies the host address on which <%=vars.product_name%> for Redis is listening. If set to the
+empty string or if not specified, the server listens on all local addresses.
- - mem_fragmentation_ratio (always reports 1.00)
+`--J=-Dgemfire.geode-for-redis-username` (Default: `"default"`) <br/>
+Specifies the default username that the server uses when a client attempts to authenticate using
+only a password. See section on [Security](#security) for more information.
- - persistence
+`--J=-Dgemfire.geode-for-redis-redundant-copies` (Default: `1`) <br/>
+Specifies the number of redundant copies <%=vars.product_name%> for Redis will attempt to keep in
+the cluster. A value of 0 means no extra copies of data will be stored in the cluster.
+Note that extra servers need to be running for redundant copies to be made. For
+example if the cluster only has one server then no redundant copies will exist no matter what the
+value of this property is. Also note that <%=vars.product_name%> for Redis uses a <%=vars.product_name%> partitioned region
+to implement redundant copies and this property corresponds to the partitioned region's
+"redundant-copies" attribute. This property must be set the same on every server in the cluster that is running a
+<%=vars.product_name%> for Redis server.
- - loading (always returns 0)
+## <a id="security"></a>Security
- - rdb_changes_since_last_save (always returns 0)
+Security is implemented slightly differently to OSS Redis. Redis stores password information in plain text in the redis.conf file.
- - rdb_last_save_time (always returns 0)
+When using <%=vars.product_name%>, to enable security, a Security Manager needs to be configured on the server(s).
+This Security Manager will authenticate `AUTH <password>` commands and `AUTH <username> <password>` commands.
+Users can set a custom `default` username using the `geode-for-redis-username` parameter.
+If users don't set this parameter, the default username will be "default".
+This username will be used when `AUTH <password>` commands are sent without a `<username>`.
- - replication
+Note that the `geode-for-redis-username` property is only needed if `AUTH` commands are issued without a username.
+In this case, the Security Manager will need to respond to authentication requests using this username.
- - role
+Note also that _any_ `AUTH` requests will fail if no Security Manager has been configured.
- - connected_slaves (always returns 0)
+For more information about configuring a Security Manager for authentication, see [Implementing Authentication](../managing/security/implementing_authentication.html).
- - server
+In addition to authentication, each command is authorized according to <%=vars.product_name%>'s
+[security model](../managing/security/implementing_authorization.html).
+Commands are divided into Read operations and Write operations for which the resource permissions
+DATA:READ:GEODE_FOR_REDIS and DATA:WRITE:GEODE_FOR_REDIS are respectively required.
- - redis_version
+For information on configuring the cluster for SSL, see [Configuring SSL](../managing/security/implementing_ssl.html).
- - redis_mode
+## <a id="application-development"></a>Application Development
- - tcp_port
+### <a id="thingstoknowbeforyoubegin"></a>Things to know before you begin
+- <%=vars.product_name%> for Redis currently implements a subset of the full Redis set of commands
+- Applications must be using a redis client that supports Redis Cluster mode.
+- If your application is using Spring Session Data Redis you will need to add the following code to disable Spring Session from calling CONFIG (CONFIG is not supported).
- - uptime_in_seconds
+```java
+@Bean
+public static ConfigureRedisAction configureRedisAction() {
+ return ConfigureRedisAction.NO_OP;
+}
+```
+This is a known solution for many Managed Redis products (ElastiCache, Azure Cache for Redis, etc.) that disable the CONFIG command for security reasons.
+You can read more about why this is done in the [Spring Session issue report](https://github.com/spring-projects/spring-session/issues/124).
- - uptime_in_days
+## <a id="redis-commands"></a>Redis Commands
- - stats
+<%=vars.product_name%> for Redis supports the following Redis commands.
- - total_commands_processed
+| Supported Commands ||||
+|-----|-----|-----|-----|
+| APPEND | AUTH | CLIENT | CLUSTER **[1]** |
+| COMMAND **[2]** | DECR | DECRBY | DEL |
+| DUMP | ECHO | EXISTS | EXPIRE |
+| EXPIREAT | GET | GETRANGE | GETSET |
+| HDEL | HEXISTS | HGET | HGETALL |
+| HINCRBY | HINCRBYFLOAT | HKEYS | HLEN |
+| HMGET | HMSET | HSCAN **[3]** | HSET |
+| HSETNX | HSTRLEN | HVALS | INCR |
+| INCRBY | INCRBYFLOAT | INFO **[4]** | KEYS |
+| LOLWUT | MGET | MSET | MSETNX |
+| PERSIST | PEXPIRE | PEXPIREAT | PING |
+| PSETEX | PSUBSCRIBE | PTTL | PUBLISH |
+| PUBSUB | PUNSUBSCRIBE | RENAME | RENAMENX |
+| RESTORE | SADD | SCARD | SDIFF |
+| SDIFFSTORE | SET | SETEX | SETNX |
+| SETRANGE | SINTER | SINTERSTORE | SISMEMBER |
+| SMEMBERS | SMOVE | SPOP | SRANDMEMBER |
+| SREM | SSCAN **[3]** | STRLEN | SUBSCRIBE |
+| SUNION | SUNIONSTORE | TTL | TYPE |
+| UNSUBSCRIBE | QUIT | ZADD | ZCARD |
+| ZCOUNT | ZINCRBY | ZINTERSTORE | ZLEXCOUNT |
+| ZPOPMAX | ZPOPMIN | ZRANGE | ZRANGEBYLEX |
+| ZRANGEBYSCORE | ZRANK | ZREM | ZREMRANGEBYLEX |
+| ZREMRANGEBYRANK | ZREMRANGEBYSCORE | ZREVRANGE | ZREVRANGEBYLEX |
+| ZREVRANGEBYSCORE | ZREVRANK | ZSCAN **[3]** | ZSCORE |
+| ZUNIONSTORE ||||
- - instantaneous_ops_per_sec
- - total_net_input_bytes
+Commands not listed above are **not implemented**.
- - instantaneous_input_kbps
+**NOTES:**
- - total_connections_received
+These commands are supported for Redis 5.
- - keyspace_hits
+**[1]** CLUSTER is implemented for the subcommands INFO, NODES, SLOTS, and KEYSLOT.
- - keyspace_misses
+**[2]** COMMAND is implemented only with no subcommands.
- - evicted_keys (always returns 0)
+**[3]** Native Redis supports a range of values of +/- the capacity of unsigned 64-bit integers
+(+/- 1.8446744e+19) for the CURSOR, but 64-bit signed integers for COUNT. <%=vars.product_name%> for Redis matches
+native Redis's behavior for COUNT, but only supports values of +/- the capacity of a signed 64-bit
+integer (+/- 9223372036854775807) for CURSOR.
- - rejected_connections (always returns 0)
+**[4]** INFO is implemented for the sections and fields listed below:
-**[3]** SLOWLOG is implemented as a NoOp.
+| INFO section | Field(s) |
+|--------------|----------|
+| clients | connected_clients<br/>blocked_clients (always returns 0) |
+| cluster | cluster_enabled (always returns 1) |
+| keyspace | db0:keys<br/>expires (always returns 0)<br/>avg_ttl (always returns 0)<br/> |
+| memory | maxmemory<br/>used_memory<br/>mem_fragmentation_ratio |
+| persistence | loading (always returns 0)<br/>rdb_changes_since_last_save (always returns 0)<br/>rdb_last_save_time (always returns 0) |
+| replication | role (always returns "master")<br/>connected_slaves (always returns 0) |
+| server | redis_version<br/>redis_mode (always returns "cluster" because <%=vars.product_name%> for Redis always runs in cluster mode.)<br/>tcp_port<br/>uptime_in_seconds<br/>uptime_in_days |
+| stats | total_commands_processed<br/>instantaneous_ops_per_sec<br/>total_net_input_bytes<br/>instantaneous_input_kbps<br/>total_connections_received<br/>keyspace_hits<br/>keyspace_misses<br/>evicted_keys (always returns 0)<br/>rejected_connections (always returns 0)<br/>pubsub_channels<br/>pubsub_patterns|
-## <a id="advantages-over-redis" class="no-quick-link"></a>Advantages of <%=vars.product_name%> over Redis
+## <a id="advantages-over-redis"></a>Advantages of <%=vars.product_name%> over Redis
<%=vars.product_name%>’s primary advantage is its **scalability**. While the Redis server is single threaded, <%=vars.product_name%> supports high concurrency. Many Redis clients can execute commands on the <%=vars.product_name%> cluster simultaneously.
<%=vars.product_name%>'s architecture and management features help detect and resolve **network partitioning** problems without explicit management on the part of the Redis client.
-## <a id="expiration-accuracy" class="no-quick-link"></a>Expiration Accuracy
+<%=vars.product_name%> for Redis partitions data across multiple servers and keeps replicated data up to date _synchronously_, whereas Redis uses asynchronous replication.
+This provides a higher level of data consistency within the cluster.
+
+## <a id="expiration-accuracy"></a>Expiration Accuracy
Keys are expired in two ways, actively and passively:
- With active expiration, expiration is evaluated whenever a key is accessed. If the key is due to expire, it is deleted. Active expiration is accurate to the millisecond.
-- With passive expiration, keys are evaluated every second. If they are due to expire, they are deleted. Passive expiration is accurate to the second.
+- With passive expiration, keys are evaluated every three minutes. If they are due to expire, they are deleted. Passive expiration is accurate to the second.
-## <a id="high-availability-model" class="no-quick-link"></a>High Availability Model
+## <a id="high-availability-model"></a>High Availability Model
-Data is stored in a single partitioned region that has one redundant copy.
+Data is stored in a single partitioned region that by default has one redundant copy.
In practice this means that the cluster can tolerate the loss of a single server without the loss of
data.
-## <a id="loss-of-connections" class="no-quick-link"></a>Loss of Connections
+## <a id="loss-of-connections"></a>Loss of Connections
There are a number of events that might occur within the <%=vars.product_name%> cluster that can result
in the cluster closing the connection to the Redis client. Losing the connection to the cluster does not
[geode] 01/09: Revert "GEODE-9883 Update Geode for Redis docs file (#7274)"
Posted by je...@apache.org.
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 08d2006cac31be6bf97cc30f3edd63eacc241a39
Author: Jens Deppe <jd...@vmware.com>
AuthorDate: Fri Jan 28 09:38:42 2022 -0800
Revert "GEODE-9883 Update Geode for Redis docs file (#7274)"
This reverts commit 49367309e65b5b9710cac0646fe950173af2e1bc.
---
.../source/subnavs/geode-subnav.erb | 24 +-
.../tools_modules/geode_for_redis.html.md.erb | 292 ++++++++++-----------
2 files changed, 158 insertions(+), 158 deletions(-)
diff --git a/geode-book/master_middleman/source/subnavs/geode-subnav.erb b/geode-book/master_middleman/source/subnavs/geode-subnav.erb
index 372a42a..094959b 100644
--- a/geode-book/master_middleman/source/subnavs/geode-subnav.erb
+++ b/geode-book/master_middleman/source/subnavs/geode-subnav.erb
@@ -2105,8 +2105,28 @@ limitations under the License.
</li>
</ul>
</li>
- <li>
- <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html">Geode for Redis</a>
+ <li class="has_submenu">
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html"><%=vars.product_name%> for Redis</a>
+ <ul>
+ <li>
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#using-the-api">Using <%=vars.product_name%> for Redis</a>
+ </li>
+ <li>
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#supported-commands">Supported Redis Commands</a>
+ </li>
+ <li>
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#advantages-over-redis">Advantages of <%=vars.product_name%> over Redis</a>
+ </li>
+ <li>
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#expiration-accuracy">Expiration Accuracy</a>
+ </li>
+ <li>
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#high-availability-model">High Availability Model</a>
+ </li>
+ <li>
+ <a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/geode_for_redis.html#loss-of-connections">Loss of Connections</a>
+ </li>
+ </ul>
</li>
<li class="has_submenu">
<a href="/docs/guide/<%=vars.product_version_nodot%>/tools_modules/gemcached/chapter_overview.html">Gemcached</a>
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 482d817..9451dc7 100644
--- a/geode-docs/tools_modules/geode_for_redis.html.md.erb
+++ b/geode-docs/tools_modules/geode_for_redis.html.md.erb
@@ -25,233 +25,213 @@ optional password authentication.
<img src="../images_svg/geode_for_redis.svg" class="image" />
-## <a id="using-geode-for-redis"></a>Using <%=vars.product_name%> for Redis
+## <a id="using-the-api" class="no-quick-link"></a>Using <%=vars.product_name%> for Redis
The <%=vars.product_name%> cluster must have at least one server that is set up to handle the incoming Redis commands.
-Prerequisites for running the examples:
+Use gfsh to start at least one server with a command of the form:
-1. **Install <%=vars.product_name%>** <br/>
-Using the instructions in the `README.md` file in the root of the <%=vars.product_name%> checkout directory, build and install <%=vars.product_name%>.
-2. **Install the Redis CLI** <br/>
-Follow installation instructions at https://redis.io/download
+```pre
+start server \
+ --name=<serverName> \
+ --locators=<locatorPort> \
+ --J=-Dgemfire.geode-for-redis-enabled=true \
+ --J=-Dgemfire.geode-for-redis-port=<geodeForRedisPort> \
+ --J=-Dgemfire.geode-for-redis-bind-address=<geodeForRedisBindAddress>
+```
-Use `gfsh` to start a locator for managing a <%=vars.product_name%> cluster:
+If the gemfire property `geode-for-redis-enabled`, is set to `true`, a <%=vars.product_name%>
+server with <%=vars.product_name%> for Redis will be started.
-```commandLine
-gfsh> start locator
-```
+Replace `<serverName>` with the name of your server.
-Use `gfsh` to start at least one server with a command of the form:
+Replace `<locatorPort>` with your locator port.
-```commandLine
-gfsh> start server --J=-Dgemfire.geode-for-redis-enabled=true --J=-Dgemfire.geode-for-redis-port=6379
-```
+Replace `<geodeForRedisPort>` with the port that the <%=vars.product_name%> server
+ listens on for Redis commands. The typical port used with a cluster compatible with Redis is 6379.
-More information about the options when starting a server is given in the section [Start Server Options](#redis-start-server-options) below.
+Replace `<geodeForRedisBindAddress>` with the address of the server host.
-To confirm the server is listening, in a separate terminal run:
+Replace `<geodeForRedisPassword>` with the password clients use to authenticate.
-```commandLine
-$ redis-cli -c ping
+To confirm the server is listening, run:
+
+``` pre
+redis-cli -h <geodeForRedisBindAddress> -p <geodeForRedisPort> -a <geodeForRedisPassword> ping
```
-The `-c` option enables cluster mode in the redis-cli, which is necessary since
-<%=vars.product_name%> for Redis runs as a Redis Cluster.
+Replace `<geodeForRedisBindAddress>`,`<geodeForRedisPort>`, and `<geodeForRedisPassword>` with the same values as the server.
If the server is functioning properly, you should see a response of `PONG`.
-### <a id="adding-a-server"></a> Add an additional server
-If you’re interested in testing <%=vars.product_name%> scalability, in gfsh run the `start server` command again.
+## <a id="supported-commands" class="no-quick-link"></a>Supported Redis Commands
-However, there are two ports that must be unique for each server in the cluster, the
-`gemfire.geode-for-redis-port`, used for receiving Redis commands, and the
-`server-port`, which is used for cluster communication.
+<%=vars.product_name%> for Redis supports the following Redis commands.
+<br/>
+
+ - APPEND <br/>
+ - AUTH <br/>
+ - DECR <br/>
+ - DECRBY <br/>
+ - DEL <br/>
+ - EXISTS <br/>
+ - EXPIRE <br/>
+ - EXPIREAT <br/>
+ - GET <br/>
+ - GETRANGE <br/>
+ - HDEL <br/>
+ - HEXISTS <br/>
+ - HGET <br/>
+ - HGETALL <br/>
+ - HINCRBY <br/>
+ - HINCRBYFLOAT <br/>
+ - HLEN <br/>
+ - HMGET <br/>
+ - HMSET <br/>
+ - HSCAN **[1]** <br/>
+ - HSET <br/>
+ - HSETNX <br/>
+ - HSTRLEN <br/>
+ - HVALS <br/>
+ - HKEYS <br/>
+ - INCR <br/>
+ - INCRBY <br/>
+ - INCRBYFLOAT <br/>
+ - INFO **[2]** <br/>
+ - KEYS <br/>
+ - MGET <br/>
+ - PERSIST <br/>
+ - PEXPIRE <br/>
+ - PEXPIREAT <br/>
+ - PING <br/>
+ - PSUBSCRIBE <br/>
+ - PTTL <br/>
+ - PUBLISH <br/>
+ - PUNSUBSCRIBE <br/>
+ - QUIT <br/>
+ - RENAME <br/>
+ - SADD <br/>
+ - SCARD <br/>
+ - SDIFF <br/>
+ - SDIFFSTORE <br/>
+ - SINTER <br/>
+ - SISMEMBER <br/>
+ - SET <br/>
+ - SETNX <br/>
+ - SLOWLOG **[3]** <br/>
+ - SMEMBERS <br/>
+ - SMOVE <br/>
+ - SREM <br/>
+ - STRLEN <br/>
+ - SUBSCRIBE <br/>
+ - SUNION <br/>
+ - TTL <br/>
+ - TYPE <br/>
+ - UNSUBSCRIBE <br/>
+
+<br/>
+Commands not listed above are **not implemented**.
-The first server used `6379` for the redis port; we'll use `6380` for the second server.
+<br/>
+**NOTES:**
-The first server was started without
-a server port specified, so it used the default `40404`. To start up an additional server, you need to specify
-a different server port, or use `--server-port=0` which tells <%=vars.product_name%> to use
-an arbitrary available port for the server port.
+These commands are supported for Redis 5.
-For example:
+**[1]** Redis accepts 64-bit signed integers for the HSCAN cursor and COUNT parameters.
+ <%=vars.product_name%> for Redis is limited to 32-bit integer values for these parameters.
-```commandLine
-gfsh> start server --J=-Dgemfire.geode-for-redis-enabled=true --J=-Dgemfire.geode-for-redis-port=6380 --server-port=0
-```
+**[2]** INFO is implemented for the sections and fields listed below:
-### <a id="shutting-down"></a>Shutting Down
-To shut down the <%=vars.product_name%> cluster you started, in the terminal with gfsh running type the following command
+ - clients
-```commandLine
-gfsh> shutdown --include-locators=true
-```
+ - connected_clients
-This command shuts down the entire <%=vars.product_name%> cluster.
+ - blocked_clients (always returns 0)
-To confirm that everything shut down correctly, if you execute a Redis command in the `redis-cli` you should see the following message:
+ - cluster
-```commandline
-Could not connect to Redis at 127.0.0.1:6379: Connection refused
-```
+ - cluster_enables (always returns 0)
-## <a id="redis-start-server-options"></a>Start Server Options
+ - keyspace
-The options that are specific to starting a server for <%=vars.product_name%> for Redis are listed below.
-For other options see [start server](gfsh/command-pages/start.html#topic_3764EE2DB18B4AE4A625E0354471738A).
+ - returns stats for db: 0
-`--J=-Dgemfire.geode-for-redis-enabled` (Default: `false`) <br/>
-If set to `true`, a <%=vars.product_name%> server with <%=vars.product_name%> for Redis will be started.
+ - memory
-`--J=-Dgemfire.geode-for-redis-port` (Default: `6379`) <br/>
-Specifies the port on which the <%=vars.product_name%> server
-listens for Redis commands. Note that the default port `6379` is the same port that native Redis
-uses by default.
+ - maxmemory
-`--J=-Dgemfire.geode-for-redis-bind-address` (Default: `""`) <br/>
-Specifies the host address on which <%=vars.product_name%> for Redis is listening. If set to the
-empty string or if not specified, the server listens on all local addresses.
+ - used_memory
-`--J=-Dgemfire.geode-for-redis-username` (Default: `"default"`) <br/>
-Specifies the default username that the server uses when a client attempts to authenticate using
-only a password. See section on [Security](#security) for more information.
+ - mem_fragmentation_ratio (always reports 1.00)
-`--J=-Dgemfire.geode-for-redis-redundant-copies` (Default: `1`) <br/>
-Specifies the number of redundant copies <%=vars.product_name%> for Redis will attempt to keep in
-the cluster. A value of 0 means no extra copies of data will be stored in the cluster.
-Note that extra servers need to be running for redundant copies to be made. For
-example if the cluster only has one server then no redundant copies will exist no matter what the
-value of this property is. Also note that <%=vars.product_name%> for Redis uses a <%=vars.product_name%> partitioned region
-to implement redundant copies and this property corresponds to the partitioned region's
-"redundant-copies" attribute. This property must be set the same on every server in the cluster that is running a
-<%=vars.product_name%> for Redis server.
+ - persistence
-## <a id="security"></a>Security
+ - loading (always returns 0)
-Security is implemented slightly differently to OSS Redis. Redis stores password information in plain text in the redis.conf file.
+ - rdb_changes_since_last_save (always returns 0)
-When using <%=vars.product_name%>, to enable security, a Security Manager needs to be configured on the server(s).
-This Security Manager will authenticate `AUTH <password>` commands and `AUTH <username> <password>` commands.
-Users can set a custom `default` username using the `geode-for-redis-username` parameter.
-If users don't set this parameter, the default username will be "default".
-This username will be used when `AUTH <password>` commands are sent without a `<username>`.
+ - rdb_last_save_time (always returns 0)
-Note that the `geode-for-redis-username` property is only needed if `AUTH` commands are issued without a username.
-In this case, the Security Manager will need to respond to authentication requests using this username.
+ - replication
-Note also that _any_ `AUTH` requests will fail if no Security Manager has been configured.
+ - role
-For more information about configuring a Security Manager for authentication, see [Implementing Authentication](../managing/security/implementing_authentication.html).
+ - connected_slaves (always returns 0)
-In addition to authentication, each command is authorized according to <%=vars.product_name%>'s
-[security model](../managing/security/implementing_authorization.html).
-Commands are divided into Read operations and Write operations for which the resource permissions
-DATA:READ:GEODE_FOR_REDIS and DATA:WRITE:GEODE_FOR_REDIS are respectively required.
+ - server
-For information on configuring the cluster for SSL, see [Configuring SSL](../managing/security/implementing_ssl.html).
+ - redis_version
-## <a id="application-development"></a>Application Development
+ - redis_mode
-### <a id="thingstoknowbeforyoubegin"></a>Things to know before you begin
-- <%=vars.product_name%> for Redis currently implements a subset of the full Redis set of commands
-- Applications must be using a redis client that supports Redis Cluster mode.
-- If your application is using Spring Session Data Redis you will need to add the following code to disable Spring Session from calling CONFIG (CONFIG is not supported).
+ - tcp_port
-```java
-@Bean
-public static ConfigureRedisAction configureRedisAction() {
- return ConfigureRedisAction.NO_OP;
-}
-```
-This is a known solution for many Managed Redis products (ElastiCache, Azure Cache for Redis, etc.) that disable the CONFIG command for security reasons.
-You can read more about why this is done in the [Spring Session issue report](https://github.com/spring-projects/spring-session/issues/124).
+ - uptime_in_seconds
-## <a id="redis-commands"></a>Redis Commands
+ - uptime_in_days
-<%=vars.product_name%> for Redis supports the following Redis commands.
+ - stats
-| Supported Commands ||||
-|-----|-----|-----|-----|
-| APPEND | AUTH | CLIENT | CLUSTER **[1]** |
-| COMMAND **[2]** | DECR | DECRBY | DEL |
-| DUMP | ECHO | EXISTS | EXPIRE |
-| EXPIREAT | GET | GETRANGE | GETSET |
-| HDEL | HEXISTS | HGET | HGETALL |
-| HINCRBY | HINCRBYFLOAT | HKEYS | HLEN |
-| HMGET | HMSET | HSCAN **[3]** | HSET |
-| HSETNX | HSTRLEN | HVALS | INCR |
-| INCRBY | INCRBYFLOAT | INFO **[4]** | KEYS |
-| LOLWUT | MGET | MSET | MSETNX |
-| PERSIST | PEXPIRE | PEXPIREAT | PING |
-| PSETEX | PSUBSCRIBE | PTTL | PUBLISH |
-| PUBSUB | PUNSUBSCRIBE | RENAME | RENAMENX |
-| RESTORE | SADD | SCARD | SDIFF |
-| SDIFFSTORE | SET | SETEX | SETNX |
-| SETRANGE | SINTER | SINTERSTORE | SISMEMBER |
-| SMEMBERS | SMOVE | SPOP | SRANDMEMBER |
-| SREM | SSCAN **[3]** | STRLEN | SUBSCRIBE |
-| SUNION | SUNIONSTORE | TTL | TYPE |
-| UNSUBSCRIBE | QUIT | ZADD | ZCARD |
-| ZCOUNT | ZINCRBY | ZINTERSTORE | ZLEXCOUNT |
-| ZPOPMAX | ZPOPMIN | ZRANGE | ZRANGEBYLEX |
-| ZRANGEBYSCORE | ZRANK | ZREM | ZREMRANGEBYLEX |
-| ZREMRANGEBYRANK | ZREMRANGEBYSCORE | ZREVRANGE | ZREVRANGEBYLEX |
-| ZREVRANGEBYSCORE | ZREVRANK | ZSCAN **[3]** | ZSCORE |
-| ZUNIONSTORE ||||
+ - total_commands_processed
+ - instantaneous_ops_per_sec
-Commands not listed above are **not implemented**.
+ - total_net_input_bytes
-**NOTES:**
+ - instantaneous_input_kbps
-These commands are supported for Redis 5.
+ - total_connections_received
-**[1]** CLUSTER is implemented for the subcommands INFO, NODES, SLOTS, and KEYSLOT.
+ - keyspace_hits
-**[2]** COMMAND is implemented only with no subcommands.
+ - keyspace_misses
-**[3]** Native Redis supports a range of values of +/- the capacity of unsigned 64-bit integers
-(+/- 1.8446744e+19) for the CURSOR, but 64-bit signed integers for COUNT. <%=vars.product_name%> for Redis matches
-native Redis's behavior for COUNT, but only supports values of +/- the capacity of a signed 64-bit
-integer (+/- 9223372036854775807) for CURSOR.
+ - evicted_keys (always returns 0)
-**[4]** INFO is implemented for the sections and fields listed below:
+ - rejected_connections (always returns 0)
-| INFO section | Field(s) |
-|--------------|----------|
-| clients | connected_clients<br/>blocked_clients (always returns 0) |
-| cluster | cluster_enabled (always returns 1) |
-| keyspace | db0:keys<br/>expires (always returns 0)<br/>avg_ttl (always returns 0)<br/> |
-| memory | maxmemory<br/>used_memory<br/>mem_fragmentation_ratio |
-| persistence | loading (always returns 0)<br/>rdb_changes_since_last_save (always returns 0)<br/>rdb_last_save_time (always returns 0) |
-| replication | role (always returns "master")<br/>connected_slaves (always returns 0) |
-| server | redis_version<br/>redis_mode (always returns "cluster" because <%=vars.product_name%> for Redis always runs in cluster mode.)<br/>tcp_port<br/>uptime_in_seconds<br/>uptime_in_days |
-| stats | total_commands_processed<br/>instantaneous_ops_per_sec<br/>total_net_input_bytes<br/>instantaneous_input_kbps<br/>total_connections_received<br/>keyspace_hits<br/>keyspace_misses<br/>evicted_keys (always returns 0)<br/>rejected_connections (always returns 0)<br/>pubsub_channels<br/>pubsub_patterns|
+**[3]** SLOWLOG is implemented as a NoOp.
-## <a id="advantages-over-redis"></a>Advantages of <%=vars.product_name%> over Redis
+## <a id="advantages-over-redis" class="no-quick-link"></a>Advantages of <%=vars.product_name%> over Redis
<%=vars.product_name%>’s primary advantage is its **scalability**. While the Redis server is single threaded, <%=vars.product_name%> supports high concurrency. Many Redis clients can execute commands on the <%=vars.product_name%> cluster simultaneously.
<%=vars.product_name%>'s architecture and management features help detect and resolve **network partitioning** problems without explicit management on the part of the Redis client.
-<%=vars.product_name%> for Redis partitions data across multiple servers and keeps replicated data up to date _synchronously_, whereas Redis uses asynchronous replication.
-This provides a higher level of data consistency within the cluster.
-
-## <a id="expiration-accuracy"></a>Expiration Accuracy
+## <a id="expiration-accuracy" class="no-quick-link"></a>Expiration Accuracy
Keys are expired in two ways, actively and passively:
- With active expiration, expiration is evaluated whenever a key is accessed. If the key is due to expire, it is deleted. Active expiration is accurate to the millisecond.
-- With passive expiration, keys are evaluated every three minutes. If they are due to expire, they are deleted. Passive expiration is accurate to the second.
+- With passive expiration, keys are evaluated every second. If they are due to expire, they are deleted. Passive expiration is accurate to the second.
-## <a id="high-availability-model"></a>High Availability Model
+## <a id="high-availability-model" class="no-quick-link"></a>High Availability Model
-Data is stored in a single partitioned region that by default has one redundant copy.
+Data is stored in a single partitioned region that has one redundant copy.
In practice this means that the cluster can tolerate the loss of a single server without the loss of
data.
-## <a id="loss-of-connections"></a>Loss of Connections
+## <a id="loss-of-connections" class="no-quick-link"></a>Loss of Connections
There are a number of events that might occur within the <%=vars.product_name%> cluster that can result
in the cluster closing the connection to the Redis client. Losing the connection to the cluster does not