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]