DonalEvans commented on a change in pull request #7236:
URL: https://github.com/apache/geode/pull/7236#discussion_r779143928
##########
File path:
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSInterIntegrationTest.java
##########
@@ -41,6 +40,9 @@
private JedisCluster jedis;
private static final int REDIS_CLIENT_TIMEOUT =
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
+ private static final String REDIS_USER_SET1 = "{user1}set1";
+ private static final String REDIS_USER_SET2 = "{user1}set2";
+ private static final String REDIS_USER_SET3 = "{user1}set3";
Review comment:
The naming of these constants is potentially a little misleading. The
use of a tag at the beginning of the key is not related to users at all, but
rather provides a way for Redis (or geode-for-redis) to guarantee that keys
will be mapped to the same slot in the database ([see the documentation for the
CLUSTER KEYSLOT command](https://redis.io/commands/cluster-keyslot)). These
keys could just as well be "{tag}set1", "{tag}set2" etc. so having the
constants be named "REDIS_USER_*" could cause confusion.
I think it might be best to change the constant names to just "SET1", "SET2"
etc. and change the tag to something more generic so that there's no confusion
in future.
--
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]
