dschneider-pivotal commented on a change in pull request #6686:
URL: https://github.com/apache/geode/pull/6686#discussion_r670664004
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisKeyCommandsFunctionExecutor.java
##########
@@ -136,7 +136,11 @@ public void restore(RedisKey key, long ttl, byte[] data,
RestoreOptions options)
@Override
public boolean rename(RedisKey oldKey, RedisKey newKey) {
List<RedisKey> orderedKeys = orderForLocking(oldKey, newKey);
- return stripedExecute(orderedKeys.get(0), () ->
rename0(orderedKeys.get(1), oldKey, newKey));
+ return stripedExecute(orderedKeys.get(0), () -> {
+ // Trigger MOVED if necessary
+ getRedisData(newKey);
Review comment:
Jens and I talked about this and it turns out we now require that the
two keys passed to rename are on the same primary bucket. That is why having
done the first stripedExecute will have locked the primary even though the
first stripedExecute may have been done on oldKey not newKey. We agreed that it
would be better to do a check to ensure that both keys are on the same primary
and then this extra getRedisData(newKey) call is not needed.
--
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]
