[GitHub] [hadoop] anuengineer commented on a change in pull request #949: HDDS-1672. Improve locking in OzoneManager.

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

anuengineer commented on a change in pull request #949: HDDS-1672. Improve locking in OzoneManager.
URL: https://github.com/apache/hadoop/pull/949#discussion_r295898352
 
 

 ##########
 File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerLock.java
 ##########
 @@ -96,30 +112,39 @@ public OzoneManagerLock(Configuration conf) {
   }
 
   /**
-   * Acquires user lock on the given resource.
+   * Acquires S3 Bucket lock on the given resource.
    *
    * <p>If the lock is not available then the current thread becomes
-   * disabled for thread scheduling purposes and lies dormant until the
-   * lock has been acquired.
+   * disabled for thread scheduling purposes and lies dormant until the lock has
+   * been acquired.
    *
-   * @param user User on which the lock has to be acquired
+   * @param s3BucketName S3Bucket Name on which the lock has to be acquired
    */
-  public void acquireUserLock(String user) {
-    // Calling thread should not hold any volume or bucket lock.
-    if (hasAnyVolumeLock() || hasAnyBucketLock() || hasAnyS3Lock()) {
+  public void acquireS3BucketLock(String s3BucketName) {
+    // Calling thread should not hold any volume/bucket/user lock.
+
+    // Not added checks for prefix/s3 secret lock, as they will never be
+    // taken with s3Bucket Lock. In this way, we can avoid 2 checks every
+    // time we acquire s3Bucket lock.
+
+    // Or do we need to add this for future safe?
+
+    if (hasAnyVolumeLock() || hasAnyBucketLock() || hasAnyUserLock()) {
 
 Review comment:
   if we do the bitmap trick, we can write a generic function that looks like this , hasAnyHigherLocks(UserLock), and this can check from 1-5 -- which is a series of simple xor operations and return the value. All we care is if the lock has been taken; so a simple bit map for each thread is all we need. Assuming that our lock hierarchy would not cross 32 locking levels 

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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org