DonalEvans commented on a change in pull request #6487: URL: https://github.com/apache/geode/pull/6487#discussion_r634816154
########## File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSet.java ########## @@ -340,7 +306,7 @@ long srem(ArrayList<ByteArrayWrapper> membersToRemove, Region<RedisKey, RedisDat * @return a set containing all the members in this set */ @VisibleForTesting - Set<ByteArrayWrapper> smembers() { + public Set<byte[]> smembers() { return new HashSet<>(members); Review comment: Correction, the hashing strategy is correctly employed in the current implementation, it's just the serialization issue that's present. Nevertheless, extending the class allows serialization and accurate calculation of size down the line when we revisit sizeable for RedisSet, so it's still the best choice, I think. -- 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: us...@infra.apache.org