linyiqun commented on a change in pull request #1489:
URL: https://github.com/apache/ozone/pull/1489#discussion_r515938620
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -206,8 +206,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
// acquire lock
- acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
- volumeName, bucketName);
+ acquireVolumeLock = omMetadataManager.getLock().acquireWriteLock(
+ VOLUME_LOCK, volumeName);
Review comment:
>The similar concurrent update issue for bucket usedBytes will happens
if we release bucket lock here and then try to acquire bucket lock again.
As mentioned above, we should both lock the volume and bucket lock for the
whole method.
@captainzmc , can you address above comment, bucket lock is also needed
since we also do the bucket used bytes update. Can you update this in all below
places?
Other change looks good to me.
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -206,6 +208,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
// acquire lock
+ acquireVolumeLock = omMetadataManager.getLock().acquireWriteLock(
+ VOLUME_LOCK, volumeName);
acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
Review comment:
Nit: Can we rename all variable name of acquiredLock to
acquireBucketLock?
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -206,6 +208,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
IAccessAuthorizer.ACLType.CREATE, OzoneObj.ResourceType.KEY);
// acquire lock
+ acquireVolumeLock = omMetadataManager.getLock().acquireWriteLock(
+ VOLUME_LOCK, volumeName);
acquiredLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
Review comment:
Nit: Can we rename all variable name of acquiredLock to
acquireBucketLock in current PR change?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
