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 <kod...@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
