Kris-10-0 commented on a change in pull request #7319:
URL: https://github.com/apache/geode/pull/7319#discussion_r794763148
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -245,33 +244,25 @@ static int setOpStoreResult(RegionProvider
regionProvider, RedisKey destinationK
return scanResult;
}
- public Collection<byte[]> spop(Region<RedisKey, RedisData> region, RedisKey
key, int popCount) {
- int originalSize = scard();
- if (originalSize == 0) {
- return emptyList();
- }
-
- if (popCount >= originalSize) {
- region.remove(key, this);
- return members;
- }
-
- List<byte[]> popped = new ArrayList<>();
- byte[][] setMembers = members.toArray(new byte[originalSize][]);
- Random rand = new Random();
- while (popped.size() < popCount) {
- int idx = rand.nextInt(originalSize);
- byte[] memberToPop = setMembers[idx];
- if (memberToPop != null) {
- setMembers[idx] = null;
- popped.add(memberToPop);
- membersRemove(memberToPop);
+ public List<byte[]> spop(int count, Region<RedisKey, RedisData> region,
RedisKey key) {
+ List<byte[]> result = new ArrayList<>();
+ if (members.size() <= count) {
+ result.addAll(members);
+ members.clear();
+ } else {
+ Random rand = new Random();
+ while (result.size() != count) {
+ int randIndex = rand.nextInt(members.getBackingArrayLength());
+ byte[] member = members.getFromBackingArray(randIndex);
+ if (member != null) {
Review comment:
The load factor is 3/4, so I believe it would be null 1/4 of the time. I
used this method to get set members from the backing array, because of Darrel's
comment on not copying over the member set. I do think this check is necessary
for the implementation I have now, but I could change the way I am doing it if
it can be stuck here infinitely.
I could be wrong, but I don't think it would be stuck in the loop
infinitely. In SPOPEXECUTOR there is a check to make sure that the set size is
greater than 0, and in spop there is a check before this that makes sure the
count is less than the size of the set. So when entering this loop there would
be some members in the set, and it would remove an amount that is less than the
total size of the set. I guess it could loop infinitely if spop is called
outside SPOPEXECUTOR, but I could move the 0 size check in this method.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
