[GitHub] [geode] dschneider-pivotal commented on a change in pull request #5501: GEODE-8339: Redis Rename hangs when servers are killed and revived

[GitBox <gi...@apache.org>]

dschneider-pivotal commented on a change in pull request #5501:
URL: https://github.com/apache/geode/pull/5501#discussion_r486450726



##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -611,7 +611,8 @@ PartitionedRegionRedundancyTracker getRedundancyTracker() {
     return redundancyTracker;
   }
 
-  public void computeWithPrimaryLocked(Object key, Runnable r) throws PrimaryBucketLockException {
+  public boolean computeWithPrimaryLocked(Object key, Runnable r, boolean tryLock)

Review comment:
       My understanding is that we no longer use this new "tryLock" parameter. So should we remove it from this method so that this PR does not change this code?

##########
File path: geode-redis/src/main/java/org/apache/geode/redis/internal/executor/key/RedisKeyCommandsFunctionInvoker.java
##########
@@ -74,18 +73,18 @@ public String type(ByteArrayWrapper key) {
 
 
   @Override
-  @SuppressWarnings("unchecked")
   public boolean rename(ByteArrayWrapper oldKey, ByteArrayWrapper newKey) {
+    while (true) {

Review comment:
       I think the "while (true)" loop should be removed. I think it is a remnant of the tryLock code. As it is now I don't see that it would ever loop.




----------------------------------------------------------------
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:
users@infra.apache.org