sabbey37 commented on a change in pull request #6296:
URL: https://github.com/apache/geode/pull/6296#discussion_r628595826
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java
##########
@@ -243,24 +251,28 @@ private boolean membersRemove(ByteArrayWrapper
memberToRemove) {
boolean isRemoved = members.remove(memberToRemove);
if (isRemoved) {
sizeInBytes -= PER_MEMBER_OVERHEAD + memberToRemove.length();
+ if (members.isEmpty()) {
+ sizeInBytes = BASE_REDIS_SET_OVERHEAD;
+ }
}
return isRemoved;
}
- private synchronized boolean membersAddAll(AddsDeltaInfo addsDeltaInfo) {
+ private synchronized void membersAddAll(AddsDeltaInfo addsDeltaInfo) {
ArrayList<ByteArrayWrapper> adds = addsDeltaInfo.getAdds();
sizeInBytes += adds.stream().mapToInt(a -> a.length() +
PER_MEMBER_OVERHEAD).sum();
- return members.addAll(adds);
+ if (members.size() == 1) {
+ sizeInBytes += INTERNAL_HASH_SET_STORAGE_OVERHEAD;
+ }
+ members.addAll(adds);
Review comment:
Thinking about it more, we don't seem to need this check at all.
Similar to `membersRemoveAll`, because this method is only ever called from
`applyDelta`, we actually don't need to check if the set is empty. The set will
never be empty in this method. When the key/value are initially created in the
region, there is no delta applied/sent (from the perspective of the `RedisSet`
class).
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
