sreejasahithi commented on code in PR #9157:
URL: https://github.com/apache/ozone/pull/9157#discussion_r2540529302


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java:
##########
@@ -254,7 +255,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager 
ozoneManager, Execut
           volBucketInfoMap.entrySet()) {
         entry.setValue(entry.getValue().copyObject());
       }
-      omMetadataManager.getLock().releaseWriteLocks(BUCKET_LOCK, 
bucketLockKeys);

Review Comment:
   The previous check was `if (!lockAcquired && 
!purgeDirsRequest.getBucketNameInfosList().isEmpty())` which is a conditional 
guard so it is possible for `lockAcquired` to be false but for the second part 
of the condition to also be false, allowing execution to proceed into the try 
block without a lock. Thats why the finally check was needed here.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyPurgeRequest.java:
##########
@@ -223,7 +223,9 @@ private List<OmBucketInfo> 
updateBucketSize(List<BucketPurgeKeysSize> bucketPurg
       }
       return bucketInfoList;
     } finally {
-      
mergeOmLockDetails(omMetadataManager.getLock().releaseWriteLocks(BUCKET_LOCK, 
bucketKeyList));
+      if (acquiredLock) {

Review Comment:
   Yes, this is a redundant check 
   Thanks for identifying it, will fix this.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to