dschneider-pivotal commented on a change in pull request #6686:
URL: https://github.com/apache/geode/pull/6686#discussion_r669009122
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/hash/HScanExecutor.java
##########
@@ -67,12 +67,13 @@ public RedisResponse executeCommand(Command command,
}
RedisKey key = command.getKey();
- if (!getDataRegion(context).containsKey(key)) {
+ RedisData value = context.getRegionProvider().getRedisData(key, null);
Review comment:
you could use "getRedisData(key)" to set value and then "value.isNull()"
instead of "value == null".
##########
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:
I'm not sure I understand why this call of getRedisData(newKey) has been
added but here are some questions/comments about it. Calling
getRedisData(newKey) seems like a race. My understanding is that you are
calling it so that it can throw an exception saying the bucket moved. But
couldn't move right after you finish calling it and before rename0 is called?
And since the get can go to the primary or secondary but the put that rename0
does on newKey goes to the primary isn't possible for that put to fail anyway?
Also at this point of time we don't know that we have locked newKey. So if
getRedisData(newKey) is needed shouldn't it be done in rename0 after both
newKey and oldKey have been locked?
--
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]
