jdeppe-pivotal commented on a change in pull request #6686:
URL: https://github.com/apache/geode/pull/6686#discussion_r672576947
##########
File path:
geode-apis-compatible-with-redis/src/commonTest/java/org/apache/geode/test/dunit/rules/RedisClusterStartupRule.java
##########
@@ -154,4 +161,47 @@ public void enableDebugLogging(int vmId) {
FastLogger.setDelegating(true);
});
}
+
+ /**
+ * Assuming a redundancy of 1, and at least 3 members, move the given key's
primary bucket to a
+ * non-hosting member.
+ */
+ public DistributedMember moveBucketForKey(String key) {
+ return getMember(1).invoke("moveBucketForKey: " + key, () -> {
+ Region<RedisKey, RedisData> r = RedisClusterStartupRule.getCache()
+ .getRegion(RegionProvider.REDIS_DATA_REGION);
+
+ RedisKey redisKey = new RedisKey(key.getBytes());
+ DistributedMember primaryMember =
PartitionRegionHelper.getPrimaryMemberForKey(r, redisKey);
+ Set<DistributedMember> allHosting =
PartitionRegionHelper.getAllMembersForKey(r, redisKey);
+
+ // Returns all members, except the one calling.
+ Set<DistributedMember> allMembers = getCache().getMembers(r);
+ allMembers.add(getCache().getDistributedSystem().getDistributedMember());
+
+ allMembers.removeAll(allHosting);
+ DistributedMember targetMember =
allMembers.stream().findFirst().orElseThrow(
+ () -> new IllegalStateException("No non-hosting member found for
key: " + key));
+
+ PartitionRegionHelper.moveBucketByKey(r, primaryMember, targetMember,
redisKey);
+
+ // Who is the primary now?
+ return PartitionRegionHelper.getPrimaryMemberForKey(r, redisKey);
+ });
+ }
+
+ public String getKeyOnServer(String keyPrefix, int vmId) {
Review comment:
Done
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/RedisRestoreKeyExistsException.java
##########
@@ -17,7 +17,7 @@
/**
* An exception thrown when the key being restored already exists.
*/
-public class RedisRestoreKeyExistsException extends RuntimeException {
+public class RedisRestoreKeyExistsException extends RedisException {
Review comment:
Done
##########
File path:
geode-apis-compatible-with-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java
##########
@@ -49,6 +49,12 @@ protected void before() {
cache = cacheFactory.create();
server = new GeodeRedisServer("localhost", 0, (InternalCache) cache);
server.setAllowUnsupportedCommands(enableUnsupportedCommands);
+
+ // Ensure that buckets are created up front
+ try {
Review comment:
Regarding the error case where the member info cannot be retrieved -
that would result in a 'non-existent' slot (or range of slots) at which point
any clients should produce some kind of error if they were trying to write to a
missing slot.
Regarding pulling this into `GeodeRedisServer` - are you thinking it should
run as part of startup? If so, I'd be wary of that since it would create a
bucket imbalance unless we also did a rebalance as part of that lifecycle. As
it is, any cluster-aware client will always retrieve the topology as the first
thing it does which would implicitly create the buckets. It's still a problem
if all members are not up before the very first client starts to connect.
--
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]
