Kris-10-0 commented on a change in pull request #7228:
URL: https://github.com/apache/geode/pull/7228#discussion_r780464930
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -202,48 +202,50 @@ static int setOpStoreResult(RegionProvider
regionProvider, RedisKey destinationK
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) {
+ boolean uniqueNumberList = count > 0;
+ count = uniqueNumberList ? count : -count;
+ int memberMapSize = members.getMemberMapSize();
- if (!duplicatesAllowed && membersSize <= count && count != 1) {
- return new ArrayList<>(members);
+ // TODO: Optomize algorithm
+ List<byte[]> result = new ArrayList<>(count);
+ if (uniqueNumberList) {
+ if (count >= members.size()) {
+ result.addAll(members);
+ } else {
+ srandomUniqueList(count, memberMapSize, result);
+ }
+ } else {
+ srandomDuplicateList(count, memberMapSize, result);
}
+ return result;
+ }
- Random rand = new Random();
+ private void srandomDuplicateList(int count, int memberMapSize, List<byte[]>
result) {
+ while (result.size() != count) {
+ int randIndex = rand.nextInt(memberMapSize);
+ byte[] member = members.getKeyAtIndex(randIndex);
- // 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--;
+ if (member != null) {
+ result.add(member);
}
- 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 srandomUniqueList(int count, int memberMapSize, List<byte[]>
result) {
+ List<Integer> allIndexes = new ArrayList<>(memberMapSize);
+ for (int i = 0; i < memberMapSize; i++) {
+ allIndexes.add(i);
+ }
+ Collections.shuffle(allIndexes);
+ int i = 0;
+ while (result.size() != count) {
+ byte[] member = members.getKeyAtIndex(i);
Review comment:
I think I was worried since getKey(pos) returned an object K it wouldn't
be a byte[] when I called it directly. I tried the direct call and it worked.
So I'll make the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
