diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 42ce374e64af4..28f89071402ce 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -171,6 +171,7 @@ public static Versioning getVersioning(boolean versioning) { public static final String OM_S3_PREFIX ="S3:"; public static final String OM_S3_VOLUME_PREFIX = "s3"; public static final String OM_S3_SECRET = "S3Secret:"; + public static final String OM_PREFIX = "Prefix:"; /** * Max chunk size limit. diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java index 34d81cf50c7b9..dbac5f3abcbe9 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMMetadataManager.java @@ -27,6 +27,7 @@ import org.apache.hadoop.ozone.om.helpers.OmPrefixInfo; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.S3SecretValue; +import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.VolumeList; import org.apache.hadoop.ozone.security.OzoneTokenIdentifier; import org.apache.hadoop.utils.db.DBStore; diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerLock.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerLock.java deleted file mode 100644 index c569c09823d19..0000000000000 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerLock.java +++ /dev/null @@ -1,269 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with this - * work for additional information regarding copyright ownership. The ASF - * licenses this file to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - *
- * http://www.apache.org/licenses/LICENSE-2.0 - *
- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS,WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ - -package org.apache.hadoop.ozone.om; - -import com.google.common.collect.ImmutableMap; -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.ozone.lock.LockManager; - -import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; - -import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; -import static org.apache.hadoop.ozone.OzoneConsts.OM_S3_PREFIX; -import static org.apache.hadoop.ozone.OzoneConsts.OM_USER_PREFIX; - -/** - * Provides different locks to handle concurrency in OzoneMaster. - * We also maintain lock hierarchy, based on the weight. - * - *
- * For example: - * - * {@literal ->} acquireVolumeLock (will work) - * {@literal +->} acquireBucketLock (will work) - * {@literal +-->} acquireUserLock (will throw Exception) - *
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. - * - * @param user User 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()) { - throw new RuntimeException( - "Thread '" + Thread.currentThread().getName() + - "' cannot acquire user lock" + - " while holding volume, bucket or S3 bucket lock(s)."); - } - manager.lock(OM_USER_PREFIX + user); - } - - /** - * Releases the user lock on given resource. - */ - public void releaseUserLock(String user) { - manager.unlock(OM_USER_PREFIX + user); - } - - /** - * Acquires volume lock on the given resource. - * - *
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. - * - * @param volume Volume on which the lock has to be acquired - */ - public void acquireVolumeLock(String volume) { - // Calling thread should not hold any bucket lock. - // You can take an Volume while holding S3 bucket lock, since - // semantically an S3 bucket maps to the ozone volume. So we check here - // only if ozone bucket lock is taken. - if (hasAnyBucketLock()) { - throw new RuntimeException( - "Thread '" + Thread.currentThread().getName() + - "' cannot acquire volume lock while holding bucket lock(s)."); - } - manager.lock(OM_KEY_PREFIX + volume); - myLocks.get().get(VOLUME_LOCK).incrementAndGet(); - } - - /** - * Releases the volume lock on given resource. - */ - public void releaseVolumeLock(String volume) { - manager.unlock(OM_KEY_PREFIX + volume); - myLocks.get().get(VOLUME_LOCK).decrementAndGet(); - } - - /** - * Acquires S3 Bucket lock on the given resource. - * - *
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. - * - * @param s3BucketName S3Bucket Name on which the lock has to be acquired - */ - public void acquireS3Lock(String s3BucketName) { - // Calling thread should not hold any bucket lock. - // You can take an Volume while holding S3 bucket lock, since - // semantically an S3 bucket maps to the ozone volume. So we check here - // only if ozone bucket lock is taken. - if (hasAnyBucketLock()) { - throw new RuntimeException( - "Thread '" + Thread.currentThread().getName() + - "' cannot acquire S3 bucket lock while holding Ozone bucket " + - "lock(s)."); - } - manager.lock(OM_S3_PREFIX + s3BucketName); - myLocks.get().get(S3_BUCKET_LOCK).incrementAndGet(); - } - - /** - * Releases the volume lock on given resource. - */ - public void releaseS3Lock(String s3BucketName) { - manager.unlock(OM_S3_PREFIX + s3BucketName); - myLocks.get().get(S3_BUCKET_LOCK).decrementAndGet(); - } - - /** - * Acquires bucket lock on the given resource. - * - *
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. - * - * @param bucket Bucket on which the lock has to be acquired - */ - public void acquireBucketLock(String volume, String bucket) { - manager.lock(OM_KEY_PREFIX + volume + OM_KEY_PREFIX + bucket); - myLocks.get().get(BUCKET_LOCK).incrementAndGet(); - } - - /** - * Releases the bucket lock on given resource. - */ - public void releaseBucketLock(String volume, String bucket) { - manager.unlock(OM_KEY_PREFIX + volume + OM_KEY_PREFIX + bucket); - myLocks.get().get(BUCKET_LOCK).decrementAndGet(); - } - - /** - * Returns true if the current thread holds any volume lock. - * @return true if current thread holds volume lock, else false - */ - private boolean hasAnyVolumeLock() { - return myLocks.get().get(VOLUME_LOCK).get() != 0; - } - - /** - * Returns true if the current thread holds any bucket lock. - * @return true if current thread holds bucket lock, else false - */ - private boolean hasAnyBucketLock() { - return myLocks.get().get(BUCKET_LOCK).get() != 0; - } - - private boolean hasAnyS3Lock() { - return myLocks.get().get(S3_BUCKET_LOCK).get() != 0; - } - - public void acquireS3SecretLock(String awsAccessId) { - if (hasAnyS3SecretLock()) { - throw new RuntimeException( - "Thread '" + Thread.currentThread().getName() + - "' cannot acquire S3 Secret lock while holding S3 " + - "awsAccessKey lock(s)."); - } - manager.lock(awsAccessId); - myLocks.get().get(S3_SECRET_LOCK).incrementAndGet(); - } - - private boolean hasAnyS3SecretLock() { - return myLocks.get().get(S3_SECRET_LOCK).get() != 0; - } - - public void releaseS3SecretLock(String awsAccessId) { - manager.unlock(awsAccessId); - myLocks.get().get(S3_SECRET_LOCK).decrementAndGet(); - } - - public void acquirePrefixLock(String prefixPath) { - if (hasAnyPrefixLock()) { - throw new RuntimeException( - "Thread '" + Thread.currentThread().getName() + - "' cannot acquire prefix path lock while holding prefix " + - "path lock(s) for path: " + prefixPath + "."); - } - manager.lock(prefixPath); - myLocks.get().get(PREFIX_LOCK).incrementAndGet(); - } - - private boolean hasAnyPrefixLock() { - return myLocks.get().get(PREFIX_LOCK).get() != 0; - } - - public void releasePrefixLock(String prefixPath) { - manager.unlock(prefixPath); - myLocks.get().get(PREFIX_LOCK).decrementAndGet(); - } -} diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java index c76a757a936ac..2fdf543f31bec 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java @@ -30,6 +30,7 @@ import java.io.IOException; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.S3_SECRET_LOCK; import static org.apache.hadoop.ozone.security.OzoneSecurityException.ResultCodes.S3_SECRET_NOT_FOUND; /** @@ -60,7 +61,7 @@ public S3SecretValue getS3Secret(String kerberosID) throws IOException { Preconditions.checkArgument(Strings.isNotBlank(kerberosID), "kerberosID cannot be null or empty."); S3SecretValue result = null; - omMetadataManager.getLock().acquireS3SecretLock(kerberosID); + omMetadataManager.getLock().acquireLock(S3_SECRET_LOCK, kerberosID); try { S3SecretValue s3Secret = omMetadataManager.getS3SecretTable().get(kerberosID); @@ -72,7 +73,7 @@ public S3SecretValue getS3Secret(String kerberosID) throws IOException { return s3Secret; } } finally { - omMetadataManager.getLock().releaseS3SecretLock(kerberosID); + omMetadataManager.getLock().releaseLock(S3_SECRET_LOCK, kerberosID); } LOG.trace("Secret for accessKey:{}, proto:{}", kerberosID, result); return result; @@ -86,7 +87,7 @@ public String getS3UserSecretString(String kerberosID) LOG.trace("Get secret for awsAccessKey:{}", kerberosID); S3SecretValue s3Secret; - omMetadataManager.getLock().acquireS3SecretLock(kerberosID); + omMetadataManager.getLock().acquireLock(S3_SECRET_LOCK, kerberosID); try { s3Secret = omMetadataManager.getS3SecretTable().get(kerberosID); if (s3Secret == null) { @@ -94,7 +95,7 @@ public String getS3UserSecretString(String kerberosID) "awsAccessKeyId " + kerberosID, S3_SECRET_NOT_FOUND); } } finally { - omMetadataManager.getLock().releaseS3SecretLock(kerberosID); + omMetadataManager.getLock().releaseLock(S3_SECRET_LOCK, kerberosID); } return s3Secret.getAwsSecret(); diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java index 7d62bc5af9eed..133cb285bff6d 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java @@ -91,18 +91,22 @@ public OzoneManagerLock(Configuration conf) { /** * Acquire lock on resource. * - * For S3_Bucket, VOLUME, BUCKET type resource, same thread acquiring lock - * again is allowed. + * For S3_BUCKET_LOCK, VOLUME_LOCK, BUCKET_LOCK type resource, same + * thread acquiring lock again is allowed. * - * For USER, PREFIX, S3_SECRET type resource, same thread acquiring lock - * again is not allowed. + * For USER_LOCK, PREFIX_LOCK, S3_SECRET_LOCK type resource, same thread + * acquiring lock again is not allowed. * - * Special Note for UserLock: Single thread can acquire single user lock/ + * Special Note for USER_LOCK: Single thread can acquire single user lock/ * multi user lock. But not both at the same time. - * @param resourceName - Resource name on which user want to acquire lock. * @param resource - Type of the resource. + * @param resources - Resource names on which user want to acquire lock. + * For Resource type BUCKET_LOCK, first param should be volume, second param + * should be bucket name. For remaining all resource only one param should + * be passed. */ - public void acquireLock(String resourceName, Resource resource) { + public void acquireLock(Resource resource, String... resources) { + String resourceName = generateResourceName(resource, resources); if (!resource.canLock(lockSet.get())) { String errorMessage = getErrorMessage(resource); LOG.error(errorMessage); @@ -115,6 +119,24 @@ public void acquireLock(String resourceName, Resource resource) { } } + /** + * Generate resource name to be locked. + * @param resource + * @param resources + */ + private String generateResourceName(Resource resource, String... resources) { + if (resources.length == 1 && resource != Resource.BUCKET_LOCK) { + return OzoneManagerLockUtil.generateResourceLockName(resource, + resources[0]); + } else if (resources.length == 2 && resource == Resource.BUCKET_LOCK) { + return OzoneManagerLockUtil.generateBucketLockName(resources[0], + resources[1]); + } else { + throw new IllegalArgumentException("acquire lock is supported on single" + + " resource for all locks except for resource bucket"); + } + } + private String getErrorMessage(Resource resource) { return "Thread '" + Thread.currentThread().getName() + "' cannot " + "acquire " + resource.name + " lock while holding " + @@ -124,7 +146,6 @@ private String getErrorMessage(Resource resource) { private List getCurrentLocks() { List currentLocks = new ArrayList<>(); - int i=0; short lockSetVal = lockSet.get(); for (Resource value : Resource.values()) { if (value.isLevelLocked(lockSetVal)) { @@ -140,7 +161,10 @@ private List getCurrentLocks() { * @param secondUser */ public void acquireMultiUserLock(String firstUser, String secondUser) { - Resource resource = Resource.USER; + Resource resource = Resource.USER_LOCK; + firstUser = generateResourceName(resource, firstUser); + secondUser = generateResourceName(resource, secondUser); + if (!resource.canLock(lockSet.get())) { String errorMessage = getErrorMessage(resource); LOG.error(errorMessage); @@ -198,11 +222,13 @@ public void acquireMultiUserLock(String firstUser, String secondUser) { * @param secondUser */ public void releaseMultiUserLock(String firstUser, String secondUser) { - Resource resource = Resource.USER; + Resource resource = Resource.USER_LOCK; + firstUser = generateResourceName(resource, firstUser); + secondUser = generateResourceName(resource, secondUser); + int compare = firstUser.compareTo(secondUser); String temp; - // Order the user names in sorted order. Swap them. if (compare > 0) { temp = secondUser; @@ -222,9 +248,16 @@ public void releaseMultiUserLock(String firstUser, String secondUser) { lockSet.set(resource.clearLock(lockSet.get())); } - - public void releaseLock(String resourceName, Resource resource) { - + /** + * Release lock on resource. + * @param resource - Type of the resource. + * @param resources - Resource names on which user want to acquire lock. + * For Resource type BUCKET_LOCK, first param should be volume, second param + * should be bucket name. For remaining all resource only one param should + * be passed. + */ + public void releaseLock(Resource resource, String... resources) { + String resourceName = generateResourceName(resource, resources); // TODO: Not checking release of higher order level lock happened while // releasing lower order level lock, as for that we need counter for // locks, as some locks support acquiring lock again. @@ -241,21 +274,21 @@ public void releaseLock(String resourceName, Resource resource) { */ public enum Resource { // For S3 Bucket need to allow only for S3, that should be means only 1. - S3_BUCKET((byte) 0, "S3_BUCKET"), // = 1 + S3_BUCKET_LOCK((byte) 0, "S3_BUCKET_LOCK"), // = 1 // For volume need to allow both s3 bucket and volume. 01 + 10 = 11 (3) - VOLUME((byte) 1, "VOLUME"), // = 2 + VOLUME_LOCK((byte) 1, "VOLUME_LOCK"), // = 2 // For bucket we need to allow both s3 bucket, volume and bucket. Which // is equal to 100 + 010 + 001 = 111 = 4 + 2 + 1 = 7 - BUCKET((byte) 2, "BUCKET"), // = 4 + BUCKET_LOCK((byte) 2, "BUCKET_LOCK"), // = 4 // For user we need to allow s3 bucket, volume, bucket and user lock. // Which is 8 4 + 2 + 1 = 15 - USER((byte) 3, "USER"), // 15 + USER_LOCK((byte) 3, "USER_LOCK"), // 15 - S3_SECRET((byte) 4, "S3_SECRET"), // 31 - PREFIX((byte) 5, "PREFIX"); //63 + S3_SECRET_LOCK((byte) 4, "S3_SECRET_LOCK"), // 31 + PREFIX_LOCK((byte) 5, "PREFIX_LOCK"); //63 // level of the resource private byte lockLevel; @@ -279,13 +312,13 @@ public enum Resource { boolean canLock(short lockSetVal) { - // For USER, S3_SECRET and PREFIX we shall not allow re-acquire locks at - // from single thread. 2nd condition is we have acquired one of these - // locks, but after that trying to acquire a lock with less than equal of - // lockLevel, we should disallow. - if (((USER.setMask & lockSetVal) == USER.setMask || - (S3_SECRET.setMask & lockSetVal) == S3_SECRET.setMask || - (PREFIX.setMask & lockSetVal) == PREFIX.setMask) + // For USER_LOCK, S3_SECRET_LOCK and PREFIX_LOCK we shall not allow + // re-acquire locks from single thread. 2nd condition is we have + // acquired one of these locks, but after that trying to acquire a lock + // with less than equal of lockLevel, we should disallow. + if (((USER_LOCK.setMask & lockSetVal) == USER_LOCK.setMask || + (S3_SECRET_LOCK.setMask & lockSetVal) == S3_SECRET_LOCK.setMask || + (PREFIX_LOCK.setMask & lockSetVal) == PREFIX_LOCK.setMask) && setMask <= lockSetVal) { return false; } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLockUtil.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLockUtil.java index 27f28f0e06c97..78a42aa10a8b5 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLockUtil.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLockUtil.java @@ -19,6 +19,7 @@ package org.apache.hadoop.ozone.om.lock; import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; +import static org.apache.hadoop.ozone.OzoneConsts.OM_PREFIX; import static org.apache.hadoop.ozone.OzoneConsts.OM_S3_PREFIX; import static org.apache.hadoop.ozone.OzoneConsts.OM_S3_SECRET; import static org.apache.hadoop.ozone.OzoneConsts.OM_USER_PREFIX; @@ -26,7 +27,7 @@ /** * Utility class contains helper functions required for OM lock. */ -public final class OzoneManagerLockUtil { +final class OzoneManagerLockUtil { private OzoneManagerLockUtil() { @@ -41,16 +42,16 @@ private OzoneManagerLockUtil() { public static String generateResourceLockName( OzoneManagerLock.Resource resource, String resourceName) { - if (resource == OzoneManagerLock.Resource.S3_BUCKET) { + if (resource == OzoneManagerLock.Resource.S3_BUCKET_LOCK) { return OM_S3_PREFIX + resourceName; - } else if (resource == OzoneManagerLock.Resource.VOLUME) { + } else if (resource == OzoneManagerLock.Resource.VOLUME_LOCK) { return OM_KEY_PREFIX + resourceName; - } else if (resource == OzoneManagerLock.Resource.USER) { + } else if (resource == OzoneManagerLock.Resource.USER_LOCK) { return OM_USER_PREFIX + resourceName; - } else if (resource == OzoneManagerLock.Resource.S3_SECRET) { + } else if (resource == OzoneManagerLock.Resource.S3_SECRET_LOCK) { return OM_S3_SECRET + resourceName; - } else if (resource == OzoneManagerLock.Resource.PREFIX) { - return OM_S3_PREFIX + resourceName; + } else if (resource == OzoneManagerLock.Resource.PREFIX_LOCK) { + return OM_PREFIX + resourceName; } else { // This is for developers who mistakenly call this method with resource // bucket type, as for bucket type we need bucket and volumeName. diff --git a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java index 0df6cf2281d29..85c75dd2a26e0 100644 --- a/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java +++ b/hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java @@ -39,56 +39,56 @@ public class TestOzoneManagerLock { @Test public void acquireResourceLock() { - String resourceName; + String[] resourceName; for (OzoneManagerLock.Resource resource : OzoneManagerLock.Resource.values()) { - resourceName = generateResourceLockName(resource); + resourceName = generateResourceName(resource); testResourceLock(resourceName, resource); } } - private void testResourceLock(String resourceName, + private void testResourceLock(String[] resourceName, OzoneManagerLock.Resource resource) { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireLock(resourceName, resource); - lock.releaseLock(resourceName, resource); + lock.acquireLock(resource, resourceName); + lock.releaseLock(resource, resourceName); Assert.assertTrue(true); } @Test public void reacquireResourceLock() { - String resourceName; + String[] resourceName; for (OzoneManagerLock.Resource resource : OzoneManagerLock.Resource.values()) { - resourceName = generateResourceLockName(resource); + resourceName = generateResourceName(resource); testResourceReacquireLock(resourceName, resource); } } - private void testResourceReacquireLock(String resourceName, + private void testResourceReacquireLock(String[] resourceName, OzoneManagerLock.Resource resource) { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); // Lock re-acquire not allowed by same thread. - if (resource == OzoneManagerLock.Resource.USER || - resource == OzoneManagerLock.Resource.S3_SECRET || - resource == OzoneManagerLock.Resource.PREFIX){ - lock.acquireLock(resourceName, resource); + if (resource == OzoneManagerLock.Resource.USER_LOCK || + resource == OzoneManagerLock.Resource.S3_SECRET_LOCK || + resource == OzoneManagerLock.Resource.PREFIX_LOCK){ + lock.acquireLock(resource, resourceName); try { - lock.acquireLock(resourceName, resource); + lock.acquireLock(resource, resourceName); fail("reacquireResourceLock failed"); } catch (RuntimeException ex) { String message = "cannot acquire " + resource.getName() + " lock " + "while holding [" + resource.getName() + "] lock(s)."; Assert.assertTrue(ex.getMessage(), ex.getMessage().contains(message)); } - lock.releaseLock(resourceName, resource); + lock.releaseLock(resource, resourceName); Assert.assertTrue(true); } else { - lock.acquireLock(resourceName, resource); - lock.acquireLock(resourceName, resource); - lock.releaseLock(resourceName, resource); - lock.releaseLock(resourceName, resource); + lock.acquireLock(resource, resourceName); + lock.acquireLock(resource, resourceName); + lock.releaseLock(resource, resourceName); + lock.releaseLock(resource, resourceName); Assert.assertTrue(true); } } @@ -96,7 +96,7 @@ private void testResourceReacquireLock(String resourceName, @Test public void testLockingOrder() { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - String resourceName; + String[] resourceName; // What this test does is iterate all resources. For each resource // acquire lock, and then in inner loop acquire all locks with higher @@ -104,22 +104,22 @@ public void testLockingOrder() { for (OzoneManagerLock.Resource resource : OzoneManagerLock.Resource.values()) { Stack stack = new Stack<>(); - resourceName = generateResourceLockName(resource); - lock.acquireLock(resourceName, resource); + resourceName = generateResourceName(resource); + lock.acquireLock(resource, resourceName); stack.push(new ResourceInfo(resourceName, resource)); for (OzoneManagerLock.Resource higherResource : OzoneManagerLock.Resource.values()) { if (higherResource.getMask() > resource.getMask()) { - resourceName = generateResourceLockName(higherResource); - lock.acquireLock(resourceName, higherResource); + resourceName = generateResourceName(higherResource); + lock.acquireLock(higherResource, resourceName); stack.push(new ResourceInfo(resourceName, higherResource)); } } // Now release locks while (!stack.empty()) { ResourceInfo resourceInfo = stack.pop(); - lock.releaseLock(resourceInfo.getLockName(), - resourceInfo.getResource()); + lock.releaseLock(resourceInfo.getResource(), + resourceInfo.getLockName()); } } Assert.assertTrue(true); @@ -133,10 +133,10 @@ public void testLockViolationsWithOneHigherLevelLock() { for (OzoneManagerLock.Resource higherResource : OzoneManagerLock.Resource.values()) { if (higherResource.getMask() > resource.getMask()) { - String resourceName = generateResourceLockName(higherResource); - lock.acquireLock(resourceName, higherResource); + String[] resourceName = generateResourceName(higherResource); + lock.acquireLock(higherResource, resourceName); try { - lock.acquireLock(generateResourceLockName(resource), resource); + lock.acquireLock(resource, generateResourceName(resource)); fail("testLockViolationsWithOneHigherLevelLock failed"); } catch (RuntimeException ex) { String message = "cannot acquire " + resource.getName() + " lock " + @@ -144,7 +144,7 @@ public void testLockViolationsWithOneHigherLevelLock() { Assert.assertTrue(ex.getMessage(), ex.getMessage().contains(message)); } - lock.releaseLock(resourceName, higherResource); + lock.releaseLock(higherResource, resourceName); } } } @@ -153,7 +153,7 @@ public void testLockViolationsWithOneHigherLevelLock() { @Test public void testLockViolations() { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - String resourceName; + String[] resourceName; // What this test does is iterate all resources. For each resource // acquire an higher level lock above the resource, and then take the the @@ -166,15 +166,15 @@ public void testLockViolations() { for (OzoneManagerLock.Resource higherResource : OzoneManagerLock.Resource.values()) { if (higherResource.getMask() > resource.getMask()) { - resourceName = generateResourceLockName(higherResource); - lock.acquireLock(resourceName, higherResource); + resourceName = generateResourceName(higherResource); + lock.acquireLock(higherResource, resourceName); stack.push(new ResourceInfo(resourceName, higherResource)); currentLocks.add(higherResource.getName()); queue.add(new ResourceInfo(resourceName, higherResource)); // try to acquire lower level lock try { - resourceName = generateResourceLockName(resource); - lock.acquireLock(resourceName, resource); + resourceName = generateResourceName(resource); + lock.acquireLock(resource, resourceName); } catch (RuntimeException ex) { String message = "cannot acquire " + resource.getName() + " lock " + "while holding " + currentLocks.toString() + " lock(s)."; @@ -187,21 +187,18 @@ public void testLockViolations() { // Now release locks while (!stack.empty()) { ResourceInfo resourceInfo = stack.pop(); - lock.releaseLock(resourceInfo.getLockName(), - resourceInfo.getResource()); + lock.releaseLock(resourceInfo.getResource(), + resourceInfo.getLockName()); } } } @Test public void releaseLockWithOutAcquiringLock() { - String userLock = - OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user3"); OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); try { - lock.releaseLock(userLock, OzoneManagerLock.Resource.USER); + lock.releaseLock(OzoneManagerLock.Resource.USER_LOCK, "user3"); fail("releaseLockWithOutAcquiringLock failed"); } catch (IllegalMonitorStateException ex) { String message = "Releasing lock on resource $user3 without acquiring " + @@ -211,13 +208,12 @@ public void releaseLockWithOutAcquiringLock() { } - private String generateResourceLockName(OzoneManagerLock.Resource resource) { - if (resource == OzoneManagerLock.Resource.BUCKET) { - return OzoneManagerLockUtil.generateBucketLockName( - UUID.randomUUID().toString(), UUID.randomUUID().toString()); + private String[] generateResourceName(OzoneManagerLock.Resource resource) { + if (resource == OzoneManagerLock.Resource.BUCKET_LOCK) { + return new String[]{UUID.randomUUID().toString(), + UUID.randomUUID().toString()}; } else { - return OzoneManagerLockUtil.generateResourceLockName(resource, - UUID.randomUUID().toString()); + return new String[]{UUID.randomUUID().toString()}; } } @@ -226,15 +222,15 @@ private String generateResourceLockName(OzoneManagerLock.Resource resource) { * Class used to store locked resource info. */ public class ResourceInfo { - private String lockName; + private String[] lockName; private OzoneManagerLock.Resource resource; - ResourceInfo(String resourceName, OzoneManagerLock.Resource resource) { + ResourceInfo(String[] resourceName, OzoneManagerLock.Resource resource) { this.lockName = resourceName; this.resource = resource; } - public String getLockName() { + public String[] getLockName() { return lockName; } @@ -246,71 +242,51 @@ public OzoneManagerLock.Resource getResource() { @Test public void acquireMultiUserLock() { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - String oldUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user1"); - String newUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user2"); - lock.acquireMultiUserLock(oldUserLock, newUserLock); - lock.releaseMultiUserLock(oldUserLock, newUserLock); + lock.acquireMultiUserLock("user1", "user2"); + lock.releaseMultiUserLock("user1", "user2"); Assert.assertTrue(true); } @Test public void reAcquireMultiUserLock() { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - String oldUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user1"); - String newUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user2"); - lock.acquireMultiUserLock(oldUserLock, newUserLock); + lock.acquireMultiUserLock("user1", "user2"); try { - lock.acquireMultiUserLock(oldUserLock, newUserLock); + lock.acquireMultiUserLock("user1", "user2"); fail("reAcquireMultiUserLock failed"); } catch (RuntimeException ex) { - String message = "cannot acquire USER lock while holding [USER] lock(s)."; + String message = "cannot acquire USER_LOCK lock while holding [USER_LOCK] lock(s)."; Assert.assertTrue(ex.getMessage(), ex.getMessage().contains(message)); } - lock.releaseMultiUserLock(oldUserLock, newUserLock); + lock.releaseMultiUserLock("user1", "user2"); } @Test public void acquireMultiUserLockAfterUserLock() { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - String oldUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user1"); - String newUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user2"); - String userLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user3"); - lock.acquireLock(userLock, OzoneManagerLock.Resource.USER); + lock.acquireLock(OzoneManagerLock.Resource.USER_LOCK, "user3"); try { - lock.acquireMultiUserLock(oldUserLock, newUserLock); + lock.acquireMultiUserLock("user1", "user2"); fail("acquireMultiUserLockAfterUserLock failed"); } catch (RuntimeException ex) { - String message = "cannot acquire USER lock while holding [USER] lock(s)."; + String message = "cannot acquire USER_LOCK lock while holding [USER_LOCK] lock(s)."; Assert.assertTrue(ex.getMessage(), ex.getMessage().contains(message)); } - lock.releaseLock(userLock, OzoneManagerLock.Resource.USER); + lock.releaseLock(OzoneManagerLock.Resource.USER_LOCK, "user3"); } @Test public void acquireUserLockAfterMultiUserLock() { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - String oldUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user1"); - String newUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user2"); - String userLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user3"); - lock.acquireMultiUserLock(oldUserLock, newUserLock); + lock.acquireMultiUserLock("user1", "user2"); try { - lock.acquireLock(userLock, OzoneManagerLock.Resource.USER); + lock.acquireLock(OzoneManagerLock.Resource.USER_LOCK, "user3"); fail("acquireUserLockAfterMultiUserLock failed"); } catch (RuntimeException ex) { - String message = "cannot acquire USER lock while holding [USER] lock(s)."; + String message = "cannot acquire USER_LOCK lock while holding [USER_LOCK] lock(s)."; Assert.assertTrue(ex.getMessage(), ex.getMessage().contains(message)); } - lock.releaseMultiUserLock(oldUserLock, newUserLock); + lock.releaseMultiUserLock("user1", "user2"); } @Test @@ -319,21 +295,21 @@ public void testLockResourceParallel() throws Exception { for (OzoneManagerLock.Resource resource : OzoneManagerLock.Resource.values()) { - final String resourceName = generateResourceLockName(resource); - lock.acquireLock(resourceName, resource); + final String[] resourceName = generateResourceName(resource); + lock.acquireLock(resource, resourceName); AtomicBoolean gotLock = new AtomicBoolean(false); new Thread(() -> { - lock.acquireLock(resourceName, resource); + lock.acquireLock(resource, resourceName); gotLock.set(true); - lock.releaseLock(resourceName, resource); + lock.releaseLock(resource, resourceName); }).start(); // Let's give some time for the new thread to run Thread.sleep(100); - // Since the new thread is trying to get lock on same volume, + // Since the new thread is trying to get lock on same resource, // it will wait. Assert.assertFalse(gotLock.get()); - lock.releaseLock(resourceName, OzoneManagerLock.Resource.VOLUME); + lock.releaseLock(resource, resourceName); // Since we have released the lock, the new thread should have the lock // now. // Let's give some time for the new thread to run @@ -346,25 +322,20 @@ public void testLockResourceParallel() throws Exception { @Test public void testMultiLockResourceParallel() throws Exception { OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - - String oldUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user1"); - String newUserLock = OzoneManagerLockUtil.generateResourceLockName( - OzoneManagerLock.Resource.USER, "user2"); - - lock.acquireMultiUserLock(oldUserLock, newUserLock); + lock.acquireMultiUserLock("user2", "user1"); AtomicBoolean gotLock = new AtomicBoolean(false); new Thread(() -> { - lock.acquireMultiUserLock(newUserLock, oldUserLock); + lock.acquireMultiUserLock("user1", "user2"); gotLock.set(true); - lock.releaseMultiUserLock(newUserLock, oldUserLock); + lock.releaseMultiUserLock("user1", "user2"); }).start(); // Let's give some time for the new thread to run Thread.sleep(100); - // Since the new thread is trying to get lock on same volume, it will wait. + // Since the new thread is trying to get lock on same resource, it will + // wait. Assert.assertFalse(gotLock.get()); - lock.releaseMultiUserLock(oldUserLock, newUserLock); + lock.releaseMultiUserLock("user2", "user1"); // Since we have released the lock, the new thread should have the lock // now. // Let's give some time for the new thread to run diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java index ea8f5f052171f..298897e4aedf0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java @@ -44,7 +44,8 @@ import static org.apache.hadoop.ozone.OzoneAcl.ZERO_BITSET; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND; - +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; /** * OM bucket manager. */ @@ -114,9 +115,12 @@ public void createBucket(OmBucketInfo bucketInfo) throws IOException { Preconditions.checkNotNull(bucketInfo); String volumeName = bucketInfo.getVolumeName(); String bucketName = bucketInfo.getBucketName(); - metadataManager.getLock().acquireVolumeLock(volumeName); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + boolean acquiredBucketLock = false; + metadataManager.getLock().acquireLock(VOLUME_LOCK, volumeName); try { + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, + bucketName); + acquiredBucketLock = true; String volumeKey = metadataManager.getVolumeKey(volumeName); String bucketKey = metadataManager.getBucketKey(volumeName, bucketName); @@ -182,8 +186,11 @@ public void createBucket(OmBucketInfo bucketInfo) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); - metadataManager.getLock().releaseVolumeLock(volumeName); + if (acquiredBucketLock) { + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, + bucketName); + } + metadataManager.getLock().releaseLock(VOLUME_LOCK, volumeName); } } @@ -207,7 +214,7 @@ public OmBucketInfo getBucketInfo(String volumeName, String bucketName) throws IOException { Preconditions.checkNotNull(volumeName); Preconditions.checkNotNull(bucketName); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { String bucketKey = metadataManager.getBucketKey(volumeName, bucketName); OmBucketInfo value = metadataManager.getBucketTable().get(bucketKey); @@ -225,7 +232,8 @@ public OmBucketInfo getBucketInfo(String volumeName, String bucketName) } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, + bucketName); } } @@ -240,7 +248,7 @@ public void setBucketProperty(OmBucketArgs args) throws IOException { Preconditions.checkNotNull(args); String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { String bucketKey = metadataManager.getBucketKey(volumeName, bucketName); OmBucketInfo oldBucketInfo = @@ -297,7 +305,8 @@ public void setBucketProperty(OmBucketArgs args) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, + bucketName); } } @@ -334,7 +343,7 @@ public void deleteBucket(String volumeName, String bucketName) throws IOException { Preconditions.checkNotNull(volumeName); Preconditions.checkNotNull(bucketName); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { //Check if bucket exists String bucketKey = metadataManager.getBucketKey(volumeName, bucketName); @@ -357,7 +366,8 @@ public void deleteBucket(String volumeName, String bucketName) } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, + bucketName); } } @@ -397,7 +407,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } String volume = obj.getVolumeName(); String bucket = obj.getBucketName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { String dbBucketKey = metadataManager.getBucketKey(volume, bucket); OmBucketInfo bucketInfo = @@ -452,7 +462,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } return true; @@ -476,7 +486,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } String volume = obj.getVolumeName(); String bucket = obj.getBucketName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { String dbBucketKey = metadataManager.getBucketKey(volume, bucket); OmBucketInfo bucketInfo = @@ -518,7 +528,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } return true; @@ -542,7 +552,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { } String volume = obj.getVolumeName(); String bucket = obj.getBucketName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { String dbBucketKey = metadataManager.getBucketKey(volume, bucket); OmBucketInfo bucketInfo = @@ -571,7 +581,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } return true; @@ -593,7 +603,7 @@ public List getAcl(OzoneObj obj) throws IOException { } String volume = obj.getVolumeName(); String bucket = obj.getBucketName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { String dbBucketKey = metadataManager.getBucketKey(volume, bucket); OmBucketInfo bucketInfo = @@ -611,7 +621,7 @@ public List getAcl(OzoneObj obj) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index b7b4b12a2e6d1..192e4f8f40079 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -118,6 +118,7 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.KEY; import static org.apache.hadoop.util.Time.monotonicNow; @@ -446,9 +447,8 @@ public OpenKeySession openKey(OmKeyArgs args) throws IOException { args.getVolumeName(), args.getBucketName(), args.getKeyName()); FileEncryptionInfo encInfo; - + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); OmBucketInfo bucketInfo = getBucketInfo(volumeName, bucketName); encInfo = getFileEncryptionInfo(bucketInfo); keyInfo = prepareKeyInfo(args, dbKeyName, size, locations, encInfo); @@ -459,7 +459,7 @@ public OpenKeySession openKey(OmKeyArgs args) throws IOException { volumeName, bucketName, keyName, ex); throw new OMException(ex.getMessage(), ResultCodes.KEY_ALLOCATION_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } if (keyInfo == null) { // the key does not exist, create a new object, the new blocks are the @@ -558,7 +558,7 @@ public void applyOpenKey(KeyArgs omKeyArgs, // check? validateBucket(volumeName, bucketName); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); String keyName = omKeyArgs.getKeyName(); // TODO: here if on OM machines clocks are skewed and there is a chance @@ -577,7 +577,7 @@ public void applyOpenKey(KeyArgs omKeyArgs, throw new OMException(ex.getMessage(), ResultCodes.KEY_ALLOCATION_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -628,7 +628,7 @@ public void commitKey(OmKeyArgs args, long clientID) throws IOException { .getOpenKey(volumeName, bucketName, keyName, clientID); Preconditions.checkNotNull(locationInfoList); try { - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); validateBucket(volumeName, bucketName); OmKeyInfo keyInfo = metadataManager.getOpenKeyTable().get(openKey); if (keyInfo == null) { @@ -654,7 +654,7 @@ public void commitKey(OmKeyArgs args, long clientID) throws IOException { throw new OMException(ex.getMessage(), ResultCodes.KEY_ALLOCATION_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -664,7 +664,7 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { String keyBytes = metadataManager.getOzoneKey( volumeName, bucketName, keyName); @@ -720,7 +720,7 @@ public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException { throw new OMException(ex.getMessage(), KEY_NOT_FOUND); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -738,7 +738,7 @@ public void renameKey(OmKeyArgs args, String toKeyName) throws IOException { ResultCodes.INVALID_KEY_NAME); } - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { // fromKeyName should exist String fromKey = metadataManager.getOzoneKey( @@ -791,7 +791,7 @@ public void renameKey(OmKeyArgs args, String toKeyName) throws IOException { throw new OMException(ex.getMessage(), ResultCodes.KEY_RENAME_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -801,7 +801,7 @@ public void deleteKey(OmKeyArgs args) throws IOException { String volumeName = args.getVolumeName(); String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { String objectKey = metadataManager.getOzoneKey( volumeName, bucketName, keyName); @@ -829,7 +829,7 @@ public void deleteKey(OmKeyArgs args) throws IOException { throw new OMException(ex.getMessage(), ex, ResultCodes.KEY_DELETION_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -902,7 +902,7 @@ public OmMultipartInfo applyInitiateMultipartUpload(OmKeyArgs keyArgs, String bucketName = keyArgs.getBucketName(); String keyName = keyArgs.getKeyName(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); validateS3Bucket(volumeName, bucketName); try { @@ -961,7 +961,7 @@ public OmMultipartInfo applyInitiateMultipartUpload(OmKeyArgs keyArgs, throw new OMException(ex.getMessage(), ResultCodes.INITIATE_MULTIPART_UPLOAD_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -975,7 +975,7 @@ public OmMultipartCommitUploadPartInfo commitMultipartUploadPart( String uploadID = omKeyArgs.getMultipartUploadID(); int partNumber = omKeyArgs.getMultipartUploadPartNumber(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); validateS3Bucket(volumeName, bucketName); String partName; try { @@ -1047,7 +1047,7 @@ public OmMultipartCommitUploadPartInfo commitMultipartUploadPart( throw new OMException(ex.getMessage(), ResultCodes.MULTIPART_UPLOAD_PARTFILE_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } return new OmMultipartCommitUploadPartInfo(partName); @@ -1064,7 +1064,7 @@ public OmMultipartUploadCompleteInfo completeMultipartUpload( String bucketName = omKeyArgs.getBucketName(); String keyName = omKeyArgs.getKeyName(); String uploadID = omKeyArgs.getMultipartUploadID(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); validateS3Bucket(volumeName, bucketName); try { String multipartKey = metadataManager.getMultipartKey(volumeName, @@ -1204,7 +1204,7 @@ public OmMultipartUploadCompleteInfo completeMultipartUpload( throw new OMException(ex.getMessage(), ResultCodes .COMPLETE_MULTIPART_UPLOAD_ERROR); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -1218,7 +1218,7 @@ public void abortMultipartUpload(OmKeyArgs omKeyArgs) throws IOException { String uploadID = omKeyArgs.getMultipartUploadID(); Preconditions.checkNotNull(uploadID, "uploadID cannot be null"); validateS3Bucket(volumeName, bucketName); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { String multipartKey = metadataManager.getMultipartKey(volumeName, @@ -1268,7 +1268,7 @@ public void abortMultipartUpload(OmKeyArgs omKeyArgs) throws IOException { throw new OMException(ex.getMessage(), ResultCodes .ABORT_MULTIPART_UPLOAD_FAILED); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -1285,7 +1285,7 @@ public OmMultipartUploadListParts listParts(String volumeName, boolean isTruncated = false; int nextPartNumberMarker = 0; - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { String multipartKey = metadataManager.getMultipartKey(volumeName, bucketName, keyName, uploadID); @@ -1346,7 +1346,7 @@ public OmMultipartUploadListParts listParts(String volumeName, throw new OMException(ex.getMessage(), ResultCodes .LIST_MULTIPART_UPLOAD_PARTS_FAILED); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -1365,7 +1365,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { String bucket = obj.getBucketName(); String keyName = obj.getKeyName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { validateBucket(volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); @@ -1425,7 +1425,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } return true; } @@ -1445,7 +1445,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { String bucket = obj.getBucketName(); String keyName = obj.getKeyName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { validateBucket(volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); @@ -1513,7 +1513,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } return true; } @@ -1533,7 +1533,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { String bucket = obj.getBucketName(); String keyName = obj.getKeyName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { validateBucket(volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); @@ -1576,7 +1576,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } return true; } @@ -1594,7 +1594,7 @@ public List getAcl(OzoneObj obj) throws IOException { String bucket = obj.getBucketName(); String keyName = obj.getKeyName(); - metadataManager.getLock().acquireBucketLock(volume, bucket); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volume, bucket); try { validateBucket(volume, bucket); String objectKey = metadataManager.getOzoneKey(volume, bucket, keyName); @@ -1619,7 +1619,7 @@ public List getAcl(OzoneObj obj) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseBucketLock(volume, bucket); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } } @@ -1664,7 +1664,7 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { // Check if this is the root of the filesystem. if (keyName.length() == 0) { @@ -1702,7 +1702,7 @@ public OzoneFileStatus getFileStatus(OmKeyArgs args) throws IOException { volumeName + " bucket: " + bucketName + " key: " + keyName, ResultCodes.FILE_NOT_FOUND); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -1722,7 +1722,7 @@ public void createDirectory(OmKeyArgs args) throws IOException { String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { // Check if this is the root of the filesystem. @@ -1744,7 +1744,7 @@ public void createDirectory(OmKeyArgs args) throws IOException { .getOzoneKey(volumeName, bucketName, dirDbKeyInfo.getKeyName()); metadataManager.getKeyTable().put(dirDbKey, dirDbKeyInfo); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } } @@ -1796,7 +1796,7 @@ public OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, String keyName = args.getKeyName(); OpenKeySession keySession; - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { OzoneFileStatus fileStatus; try { @@ -1822,7 +1822,7 @@ public OpenKeySession createFile(OmKeyArgs args, boolean isOverWrite, // filestatus. We can avoid some operations in openKey call. keySession = openKey(args); } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } return keySession; @@ -1844,7 +1844,7 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws IOException { String bucketName = args.getBucketName(); String keyName = args.getKeyName(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { OzoneFileStatus fileStatus = getFileStatus(args); if (fileStatus.isFile()) { @@ -1852,7 +1852,7 @@ public OmKeyInfo lookupFile(OmKeyArgs args) throws IOException { } //if key is not of type file or if key is not found we throw an exception } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } throw new OMException("Can not write to directory: " + keyName, @@ -1878,7 +1878,7 @@ public List listStatus(OmKeyArgs args, boolean recursive, String keyName = args.getKeyName(); List fileStatusList = new ArrayList<>(); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { if (Strings.isNullOrEmpty(startKey)) { OzoneFileStatus fileStatus = getFileStatus(args); @@ -1940,7 +1940,7 @@ public List listStatus(OmKeyArgs args, boolean recursive, } } } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } return fileStatusList; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java index c0b7bdc94a15f..84bdc8e0661f7 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java @@ -47,6 +47,7 @@ import org.apache.hadoop.ozone.om.helpers.OmPrefixInfo; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.S3SecretValue; +import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.VolumeList; import org.apache.hadoop.ozone.security.OzoneTokenIdentifier; import org.apache.hadoop.utils.db.DBStore; @@ -403,7 +404,7 @@ public String getMultipartKey(String volume, String bucket, String key, * @return OzoneManagerLock */ @Override - public OzoneManagerLock getLock() { + public org.apache.hadoop.ozone.om.lock.OzoneManagerLock getLock() { return lock; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 2f3daf37a08db..a4a4b9f18f942 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -77,6 +77,7 @@ import org.apache.hadoop.ozone.om.helpers.OmDeleteVolumeResponse; import org.apache.hadoop.ozone.om.helpers.OmVolumeOwnerChangeResponse; import org.apache.hadoop.ozone.om.helpers.S3SecretValue; +import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; import org.apache.hadoop.ozone.om.protocol.OzoneManagerServerProtocol; import org.apache.hadoop.ozone.om.snapshot.OzoneManagerSnapshotProvider; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos @@ -200,6 +201,8 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_AUTH_METHOD; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TOKEN_ERROR_OTHER; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.S3_BUCKET_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; import static org.apache.hadoop.ozone.protocol.proto .OzoneManagerProtocolProtos.OzoneManagerService .newReflectiveBlockingService; @@ -2557,13 +2560,21 @@ public List getServiceList() throws IOException { */ public void createS3Bucket(String userName, String s3BucketName) throws IOException { + + boolean acquiredS3Lock = false; + boolean acquiredVolumeLock = false; try { if(isAclEnabled) { checkAcls(ResourceType.BUCKET, StoreType.S3, ACLType.CREATE, null, s3BucketName, null); } metrics.incNumBucketCreates(); + metadataManager.getLock().acquireLock(S3_BUCKET_LOCK, s3BucketName); + acquiredS3Lock = true; try { + metadataManager.getLock().acquireLock(VOLUME_LOCK, + s3BucketManager.formatOzoneVolumeName(userName)); + acquiredVolumeLock = true; boolean newVolumeCreate = s3BucketManager.createOzoneVolumeIfNeeded( userName); if (newVolumeCreate) { @@ -2578,12 +2589,19 @@ public void createS3Bucket(String userName, String s3BucketName) metrics.incNumVolumeCreateFails(); throw ex; } - s3BucketManager.createS3Bucket(userName, s3BucketName); metrics.incNumBuckets(); } catch (IOException ex) { metrics.incNumBucketCreateFails(); throw ex; + } finally { + if (acquiredVolumeLock) { + metadataManager.getLock().releaseLock(VOLUME_LOCK, + s3BucketManager.formatOzoneVolumeName(userName)); + } + if (acquiredS3Lock) { + metadataManager.getLock().releaseLock(S3_BUCKET_LOCK, s3BucketName); + } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java index b9aff890982d4..6e8954a89a13a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/PrefixManagerImpl.java @@ -20,6 +20,7 @@ import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmPrefixInfo; +import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.util.RadixNode; import org.apache.hadoop.ozone.util.RadixTree; @@ -37,6 +38,7 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PREFIX_NOT_FOUND; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.PREFIX_LOCK; import static org.apache.hadoop.ozone.security.acl.OzoneObj.ResourceType.PREFIX; /** @@ -91,7 +93,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { validateOzoneObj(obj); String prefixPath = obj.getPath(); - metadataManager.getLock().acquirePrefixLock(prefixPath); + metadataManager.getLock().acquireLock(PREFIX_LOCK, prefixPath); try { OmPrefixInfo prefixInfo = metadataManager.getPrefixTable().get(prefixPath); @@ -135,7 +137,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releasePrefixLock(prefixPath); + metadataManager.getLock().releaseLock(PREFIX_LOCK, prefixPath); } return true; } @@ -152,7 +154,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { validateOzoneObj(obj); String prefixPath = obj.getPath(); - metadataManager.getLock().acquirePrefixLock(prefixPath); + metadataManager.getLock().acquireLock(PREFIX_LOCK, prefixPath); try { OmPrefixInfo prefixInfo = metadataManager.getPrefixTable().get(prefixPath); @@ -205,7 +207,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releasePrefixLock(prefixPath); + metadataManager.getLock().releaseLock(PREFIX_LOCK, prefixPath); } return true; } @@ -222,7 +224,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { public boolean setAcl(OzoneObj obj, List acls) throws IOException { validateOzoneObj(obj); String prefixPath = obj.getPath(); - metadataManager.getLock().acquirePrefixLock(prefixPath); + metadataManager.getLock().acquireLock(PREFIX_LOCK, prefixPath); try { OmPrefixInfo prefixInfo = metadataManager.getPrefixTable().get(prefixPath); @@ -241,7 +243,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { } throw ex; } finally { - metadataManager.getLock().releasePrefixLock(prefixPath); + metadataManager.getLock().releaseLock(PREFIX_LOCK, prefixPath); } return true; } @@ -256,7 +258,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { public List getAcl(OzoneObj obj) throws IOException { validateOzoneObj(obj); String prefixPath = obj.getPath(); - metadataManager.getLock().acquirePrefixLock(prefixPath); + metadataManager.getLock().acquireLock(PREFIX_LOCK, prefixPath); try { String longestPrefix = prefixTree.getLongestPrefix(prefixPath); if (prefixPath.equals(longestPrefix)) { @@ -267,7 +269,7 @@ public List getAcl(OzoneObj obj) throws IOException { } } } finally { - metadataManager.getLock().releasePrefixLock(prefixPath); + metadataManager.getLock().releaseLock(PREFIX_LOCK, prefixPath); } return EMPTY_ACL_LIST; } @@ -275,12 +277,12 @@ public List getAcl(OzoneObj obj) throws IOException { @Override public List getLongestPrefixPath(String path) { String prefixPath = prefixTree.getLongestPrefix(path); - metadataManager.getLock().acquirePrefixLock(prefixPath); + metadataManager.getLock().acquireLock(PREFIX_LOCK, prefixPath); try { return prefixTree.getLongestPrefixPath(prefixPath).stream() .map(c -> c.getValue()).collect(Collectors.toList()); } finally { - metadataManager.getLock().releasePrefixLock(prefixPath); + metadataManager.getLock().releaseLock(PREFIX_LOCK, prefixPath); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManager.java index 9e3523f407af1..dfd0ac3ea341c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManager.java @@ -78,4 +78,10 @@ public interface S3BucketManager { * @throws IOException - incase of volume creation failure. */ boolean createOzoneVolumeIfNeeded(String userName) throws IOException; + + /** + * Return volume name from userName. + * @param userName + */ + String formatOzoneVolumeName(String userName); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManagerImpl.java index c234266f887e2..6fe0881147e6e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3BucketManagerImpl.java @@ -27,6 +27,7 @@ import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_ALREADY_EXISTS; + import org.apache.logging.log4j.util.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,6 +36,7 @@ import java.util.Objects; import static org.apache.hadoop.ozone.OzoneConsts.OM_S3_VOLUME_PREFIX; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.S3_BUCKET_LOCK; /** * S3 Bucket Manager, this class maintains a mapping between S3 Bucket and Ozone @@ -101,10 +103,9 @@ public void createS3Bucket(String userName, String bucketName) // anonymous access to bucket where the user name is absent. String ozoneVolumeName = formatOzoneVolumeName(userName); - omMetadataManager.getLock().acquireS3Lock(bucketName); + omMetadataManager.getLock().acquireLock(S3_BUCKET_LOCK, bucketName); try { - String bucket = - omMetadataManager.getS3Table().get(bucketName); + String bucket = omMetadataManager.getS3Table().get(bucketName); if (bucket != null) { LOG.debug("Bucket already exists. {}", bucketName); @@ -119,8 +120,9 @@ public void createS3Bucket(String userName, String bucketName) omMetadataManager.getS3Table().put(bucketName, finalName); } finally { - omMetadataManager.getLock().releaseS3Lock(bucketName); + omMetadataManager.getLock().releaseLock(S3_BUCKET_LOCK, bucketName); } + } @Override @@ -128,7 +130,7 @@ public void deleteS3Bucket(String bucketName) throws IOException { Preconditions.checkArgument( Strings.isNotBlank(bucketName), "Bucket name cannot be null or empty"); - omMetadataManager.getLock().acquireS3Lock(bucketName); + omMetadataManager.getLock().acquireLock(S3_BUCKET_LOCK, bucketName); try { String map = omMetadataManager.getS3Table().get(bucketName); @@ -142,12 +144,13 @@ public void deleteS3Bucket(String bucketName) throws IOException { } catch(IOException ex) { throw ex; } finally { - omMetadataManager.getLock().releaseS3Lock(bucketName); + omMetadataManager.getLock().releaseLock(S3_BUCKET_LOCK, bucketName); } } - private String formatOzoneVolumeName(String userName) { + @Override + public String formatOzoneVolumeName(String userName) { return String.format(OM_S3_VOLUME_PREFIX + "%s", userName); } @@ -207,7 +210,7 @@ public String getOzoneBucketMapping(String s3BucketName) throws IOException { Preconditions.checkArgument(s3BucketName.length() >=3 && s3BucketName.length() < 64, "Length of the S3 Bucket is not correct."); - omMetadataManager.getLock().acquireS3Lock(s3BucketName); + omMetadataManager.getLock().acquireLock(S3_BUCKET_LOCK, s3BucketName); try { String mapping = omMetadataManager.getS3Table().get(s3BucketName); if (mapping != null) { @@ -216,7 +219,7 @@ public String getOzoneBucketMapping(String s3BucketName) throws IOException { throw new OMException("No such S3 bucket.", OMException.ResultCodes.S3_BUCKET_NOT_FOUND); } finally { - omMetadataManager.getLock().releaseS3Lock(s3BucketName); + omMetadataManager.getLock().releaseLock(S3_BUCKET_LOCK, s3BucketName); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java index 6ff289a81bbe9..0cf05ea25de8c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/VolumeManagerImpl.java @@ -37,6 +37,8 @@ import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_USER_MAX_VOLUME_DEFAULT; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -120,9 +122,12 @@ private VolumeList delVolumeFromOwnerList(String volume, String owner) @Override public VolumeList createVolume(OmVolumeArgs omVolumeArgs) throws IOException { Preconditions.checkNotNull(omVolumeArgs); - metadataManager.getLock().acquireUserLock(omVolumeArgs.getOwnerName()); - metadataManager.getLock().acquireVolumeLock(omVolumeArgs.getVolume()); + + boolean acquiredLock = false; + metadataManager.getLock().acquireLock(VOLUME_LOCK, omVolumeArgs.getVolume()); try { + metadataManager.getLock().acquireLock(USER_LOCK, omVolumeArgs.getOwnerName()); + acquiredLock = true; String dbVolumeKey = metadataManager.getVolumeKey( omVolumeArgs.getVolume()); String dbUserKey = metadataManager.getUserKey( @@ -156,8 +161,11 @@ public VolumeList createVolume(OmVolumeArgs omVolumeArgs) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(omVolumeArgs.getVolume()); - metadataManager.getLock().releaseUserLock(omVolumeArgs.getOwnerName()); + if (acquiredLock) { + metadataManager.getLock().releaseLock(USER_LOCK, + omVolumeArgs.getOwnerName()); + } + metadataManager.getLock().releaseLock(VOLUME_LOCK, omVolumeArgs.getVolume()); } } @@ -206,8 +214,9 @@ public OmVolumeOwnerChangeResponse setOwner(String volume, String owner) throws IOException { Preconditions.checkNotNull(volume); Preconditions.checkNotNull(owner); - metadataManager.getLock().acquireUserLock(owner); - metadataManager.getLock().acquireVolumeLock(volume); + boolean acquiredLock = false; + String actualOwner = null; + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = metadataManager @@ -221,8 +230,11 @@ public OmVolumeOwnerChangeResponse setOwner(String volume, String owner) Preconditions.checkState(volume.equals(volumeArgs.getVolume())); - String originalOwner = - metadataManager.getUserKey(volumeArgs.getOwnerName()); + actualOwner = volumeArgs.getOwnerName(); + String originalOwner = metadataManager.getUserKey(actualOwner); + + metadataManager.getLock().acquireMultiUserLock(owner, originalOwner); + acquiredLock = true; VolumeList oldOwnerVolumeList = delVolumeFromOwnerList(volume, originalOwner); @@ -243,8 +255,10 @@ public OmVolumeOwnerChangeResponse setOwner(String volume, String owner) } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); - metadataManager.getLock().releaseUserLock(owner); + if (acquiredLock) { + metadataManager.getLock().releaseMultiUserLock(owner, actualOwner); + } + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } } @@ -300,7 +314,7 @@ private void setOwnerCommitToDB(VolumeList oldOwnerVolumeList, @Override public OmVolumeArgs setQuota(String volume, long quota) throws IOException { Preconditions.checkNotNull(volume); - metadataManager.getLock().acquireVolumeLock(volume); + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = @@ -326,7 +340,7 @@ public OmVolumeArgs setQuota(String volume, long quota) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } } @@ -352,7 +366,7 @@ public void applySetQuota(OmVolumeArgs omVolumeArgs) throws IOException { @Override public OmVolumeArgs getVolumeInfo(String volume) throws IOException { Preconditions.checkNotNull(volume); - metadataManager.getLock().acquireVolumeLock(volume); + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = @@ -370,7 +384,7 @@ public OmVolumeArgs getVolumeInfo(String volume) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } } @@ -385,16 +399,13 @@ public OmVolumeArgs getVolumeInfo(String volume) throws IOException { @Override public OmDeleteVolumeResponse deleteVolume(String volume) throws IOException { Preconditions.checkNotNull(volume); - String owner; - metadataManager.getLock().acquireVolumeLock(volume); + String owner = null; + boolean acquiredLock = false; + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { owner = getVolumeInfo(volume).getOwnerName(); - } finally { - metadataManager.getLock().releaseVolumeLock(volume); - } - metadataManager.getLock().acquireUserLock(owner); - metadataManager.getLock().acquireVolumeLock(volume); - try { + metadataManager.getLock().acquireLock(USER_LOCK, owner); + acquiredLock = true; String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = metadataManager.getVolumeTable().get(dbVolumeKey); @@ -425,8 +436,11 @@ public OmDeleteVolumeResponse deleteVolume(String volume) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); - metadataManager.getLock().releaseUserLock(owner); + if (acquiredLock) { + metadataManager.getLock().releaseLock(USER_LOCK, owner); + } + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); + } } @@ -472,7 +486,7 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) throws IOException { Preconditions.checkNotNull(volume); Preconditions.checkNotNull(userAcl); - metadataManager.getLock().acquireVolumeLock(volume); + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = @@ -493,7 +507,7 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } } @@ -503,12 +517,12 @@ public boolean checkVolumeAccess(String volume, OzoneAclInfo userAcl) @Override public List listVolumes(String userName, String prefix, String startKey, int maxKeys) throws IOException { - metadataManager.getLock().acquireUserLock(userName); + metadataManager.getLock().acquireLock(USER_LOCK, userName); try { return metadataManager.listVolumes( userName, prefix, startKey, maxKeys); } finally { - metadataManager.getLock().releaseUserLock(userName); + metadataManager.getLock().releaseLock(USER_LOCK, userName); } } @@ -529,7 +543,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { "VolumeManager. OzoneObj type:" + obj.getResourceType()); } String volume = obj.getVolumeName(); - metadataManager.getLock().acquireVolumeLock(volume); + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = @@ -556,7 +570,7 @@ public boolean addAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } return true; @@ -579,7 +593,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { "VolumeManager. OzoneObj type:" + obj.getResourceType()); } String volume = obj.getVolumeName(); - metadataManager.getLock().acquireVolumeLock(volume); + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = @@ -606,7 +620,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } return true; @@ -630,7 +644,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { "VolumeManager. OzoneObj type:" + obj.getResourceType()); } String volume = obj.getVolumeName(); - metadataManager.getLock().acquireVolumeLock(volume); + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = @@ -652,7 +666,7 @@ public boolean setAcl(OzoneObj obj, List acls) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } return true; @@ -673,7 +687,7 @@ public List getAcl(OzoneObj obj) throws IOException { "VolumeManager. OzoneObj type:" + obj.getResourceType()); } String volume = obj.getVolumeName(); - metadataManager.getLock().acquireVolumeLock(volume); + metadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = metadataManager.getVolumeKey(volume); OmVolumeArgs volumeArgs = @@ -692,7 +706,7 @@ public List getAcl(OzoneObj obj) throws IOException { } throw ex; } finally { - metadataManager.getLock().releaseVolumeLock(volume); + metadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java index c4e72c6d67acd..15f258cd7f3c6 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java @@ -58,6 +58,8 @@ import org.apache.hadoop.utils.db.cache.CacheKey; import org.apache.hadoop.utils.db.cache.CacheValue; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos .CryptoProtocolVersionProto.ENCRYPTION_ZONES; @@ -144,10 +146,12 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, String bucketKey = metadataManager.getBucketKey(volumeName, bucketName); IOException exception = null; - metadataManager.getLock().acquireVolumeLock(volumeName); - metadataManager.getLock().acquireBucketLock(volumeName, bucketName); - try { + boolean acquiredBucketLock = false; + metadataManager.getLock().acquireLock(VOLUME_LOCK, volumeName); + try { + metadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); + acquiredBucketLock = true; //Check if the volume exists if (metadataManager.getVolumeTable().get(volumeKey) == null) { LOG.debug("volume: {} not found ", volumeName); @@ -169,8 +173,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } catch (IOException ex) { exception = ex; } finally { - metadataManager.getLock().releaseBucketLock(volumeName, bucketName); - metadataManager.getLock().releaseVolumeLock(volumeName); + if (acquiredBucketLock) { + metadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); + } + metadataManager.getLock().releaseLock(VOLUME_LOCK, volumeName); } // Performing audit logging outside of the lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java index 853bb7293b6c6..6de8d2b258ed5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketDeleteRequest.java @@ -48,6 +48,8 @@ import org.apache.hadoop.utils.db.cache.CacheKey; import org.apache.hadoop.utils.db.cache.CacheValue; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; + /** * Handles DeleteBucket Request. */ @@ -102,7 +104,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, IOException exception = null; // acquire lock - omMetadataManager.getLock().acquireBucketLock(volumeName, bucketName); + omMetadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { // No need to check volume exists here, as bucket cannot be created // with out volume creation. @@ -131,7 +133,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } catch (IOException ex) { exception = ex; } finally { - omMetadataManager.getLock().releaseBucketLock(volumeName, bucketName); + omMetadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } // Performing audit logging outside of the lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java index fa8c939d2f952..2e1a6ad4ab14a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java @@ -58,6 +58,8 @@ import org.apache.hadoop.utils.db.cache.CacheKey; import org.apache.hadoop.utils.db.cache.CacheValue; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK; + /** * Handle SetBucketProperty Request. */ @@ -120,7 +122,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, IOException exception = null; // acquire lock - omMetadataManager.getLock().acquireBucketLock(volumeName, bucketName); + omMetadataManager.getLock().acquireLock(BUCKET_LOCK, volumeName, bucketName); try { @@ -182,7 +184,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } catch (IOException ex) { exception = ex; } finally { - omMetadataManager.getLock().releaseBucketLock(volumeName, bucketName); + omMetadataManager.getLock().releaseLock(BUCKET_LOCK, volumeName, bucketName); } // Performing audit logging outside of the lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java index 993cd9d88a011..2faac127b1ac8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeCreateRequest.java @@ -54,6 +54,9 @@ .VolumeList; import org.apache.hadoop.util.Time; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK; + /** * Handles volume create request. */ @@ -134,13 +137,14 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, String dbUserKey = omMetadataManager.getUserKey(owner); String dbVolumeKey = omMetadataManager.getVolumeKey(volume); VolumeList volumeList = null; + boolean acquiredUserLock = false; + IOException exception = null; // acquire lock. - omMetadataManager.getLock().acquireUserLock(owner); - omMetadataManager.getLock().acquireVolumeLock(volume); - - IOException exception = null; + omMetadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { + omMetadataManager.getLock().acquireLock(USER_LOCK, owner); + acquiredUserLock = true; OmVolumeArgs dbVolumeArgs = omMetadataManager.getVolumeTable().get(dbVolumeKey); @@ -166,8 +170,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } catch (IOException ex) { exception = ex; } finally { - omMetadataManager.getLock().releaseVolumeLock(volumeInfo.getVolume()); - omMetadataManager.getLock().releaseUserLock(dbUserKey); + if (acquiredUserLock) { + omMetadataManager.getLock().releaseLock(USER_LOCK, owner); + } + omMetadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } // Performing audit logging outside of the lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java index 65f7df51bf2b0..d07694bec2001 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeDeleteRequest.java @@ -50,7 +50,8 @@ import org.apache.hadoop.utils.db.cache.CacheKey; import org.apache.hadoop.utils.db.cache.CacheValue; - +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.USER_LOCK; /** * Handles volume delete request. */ @@ -104,37 +105,19 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OmVolumeArgs omVolumeArgs = null; String owner = null; + boolean acquiredUserLock = false; + IOException exception = null; + OzoneManagerProtocolProtos.VolumeList newVolumeList = null; - omMetadataManager.getLock().acquireVolumeLock(volume); + omMetadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { owner = getVolumeInfo(omMetadataManager, volume).getOwnerName(); - } catch (IOException ex) { - LOG.error("Volume deletion failed for volume:{}", volume, ex); - omMetrics.incNumVolumeDeleteFails(); - auditLog(auditLogger, buildAuditMessage(OMAction.DELETE_VOLUME, - buildVolumeAuditMap(volume), ex, userInfo)); - return new OMVolumeDeleteResponse(null, null, null, - createErrorOMResponse(omResponse, ex)); - } finally { - omMetadataManager.getLock().releaseVolumeLock(volume); - } - - // Release and reacquire lock for now it will not be a problem for now, as - // applyTransaction serializes the operation's. - // TODO: Revisit this logic once HDDS-1672 checks in. - - // We cannot acquire user lock holding volume lock, so released volume - // lock, and acquiring user and volume lock. + omMetadataManager.getLock().acquireLock(USER_LOCK, owner); + acquiredUserLock = true; - omMetadataManager.getLock().acquireUserLock(owner); - omMetadataManager.getLock().acquireVolumeLock(volume); - - String dbUserKey = omMetadataManager.getUserKey(owner); - String dbVolumeKey = omMetadataManager.getVolumeKey(volume); + String dbUserKey = omMetadataManager.getUserKey(owner); + String dbVolumeKey = omMetadataManager.getVolumeKey(volume); - IOException exception = null; - OzoneManagerProtocolProtos.VolumeList newVolumeList = null; - try { if (!omMetadataManager.isVolumeEmpty(volume)) { LOG.debug("volume:{} is not empty", volume); throw new OMException(OMException.ResultCodes.VOLUME_NOT_EMPTY); @@ -155,10 +138,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } catch (IOException ex) { exception = ex; - } finally { - omMetadataManager.getLock().releaseVolumeLock(volume); - omMetadataManager.getLock().releaseUserLock(owner); + if (acquiredUserLock) { + omMetadataManager.getLock().releaseLock(USER_LOCK, owner); + } + omMetadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } // Performing audit logging outside of the lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java index 0de2124b54ed9..021c797bcdc3e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetOwnerRequest.java @@ -53,6 +53,8 @@ import org.apache.hadoop.utils.db.cache.CacheKey; import org.apache.hadoop.utils.db.cache.CacheValue; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; + /** * Handle set owner request for volume. */ @@ -123,11 +125,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, OzoneManagerProtocolProtos.VolumeList newOwnerVolumeList = null; OmVolumeArgs omVolumeArgs = null; IOException exception = null; + boolean acquiredUserLocks = false; - omMetadataManager.getLock().acquireUserLock(newOwner); - omMetadataManager.getLock().acquireVolumeLock(volume); - - boolean needToreleaseOldOwnerLock = false; + omMetadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { omVolumeArgs = omMetadataManager.getVolumeTable().get(dbVolumeKey); @@ -140,25 +140,15 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, oldOwner = omVolumeArgs.getOwnerName(); + omMetadataManager.getLock().acquireMultiUserLock(newOwner, oldOwner); + acquiredUserLocks = true; - // Release and reacquire lock for now it will not be a problem, as - // applyTransaction serializes the operation's. - // TODO: Revisit this logic once HDDS-1672 checks in. - - // releasing volume lock, as to acquire user lock we need to release - // volume lock. - omMetadataManager.getLock().releaseVolumeLock(volume); - omMetadataManager.getLock().acquireUserLock(oldOwner); - omMetadataManager.getLock().acquireVolumeLock(volume); - - needToreleaseOldOwnerLock = true; oldOwnerVolumeList = omMetadataManager.getUserTable().get(oldOwner); oldOwnerVolumeList = delVolumeFromOwnerList( oldOwnerVolumeList, volume, oldOwner); - newOwnerVolumeList = omMetadataManager.getUserTable().get(newOwner); newOwnerVolumeList = addVolumeToOwnerList( newOwnerVolumeList, volume, newOwner, maxUserVolumeCount); @@ -182,11 +172,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } catch (IOException ex) { exception = ex; } finally { - omMetadataManager.getLock().releaseVolumeLock(volume); - omMetadataManager.getLock().releaseUserLock(newOwner); - if (needToreleaseOldOwnerLock) { - omMetadataManager.getLock().releaseUserLock(oldOwner); + if (acquiredUserLocks) { + omMetadataManager.getLock().releaseMultiUserLock(newOwner, oldOwner); } + omMetadataManager.getLock().acquireLock(VOLUME_LOCK, volume); } // Performing audit logging outside of the lock. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java index 1d7097fc297d9..a5430aa8890fb 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java @@ -52,6 +52,8 @@ import org.apache.hadoop.utils.db.cache.CacheKey; import org.apache.hadoop.utils.db.cache.CacheValue; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK; + /** * Handles set Quota request for volume. @@ -121,7 +123,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, IOException exception = null; OmVolumeArgs omVolumeArgs = null; - omMetadataManager.getLock().acquireVolumeLock(volume); + omMetadataManager.getLock().acquireLock(VOLUME_LOCK, volume); try { String dbVolumeKey = omMetadataManager.getVolumeKey(volume); omVolumeArgs = omMetadataManager.getVolumeTable().get(dbVolumeKey); @@ -141,7 +143,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, } catch (IOException ex) { exception = ex; } finally { - omMetadataManager.getLock().releaseVolumeLock(volume); + omMetadataManager.getLock().releaseLock(VOLUME_LOCK, volume); } // Performing audit logging outside of the lock. diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerLock.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerLock.java deleted file mode 100644 index 2def5c4213990..0000000000000 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerLock.java +++ /dev/null @@ -1,193 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with this - * work for additional information regarding copyright ownership. The ASF - * licenses this file to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS,WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ - -package org.apache.hadoop.ozone.om; - -import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.junit.Assert; -import org.junit.Test; - -import java.util.concurrent.atomic.AtomicBoolean; - -/** - * Contains test-cases to verify OzoneManagerLock. - */ -public class TestOzoneManagerLock { - - @Test(timeout = 1000) - public void testDifferentUserLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - lock.acquireUserLock("userTwo"); - lock.releaseUserLock("userOne"); - lock.releaseUserLock("userTwo"); - Assert.assertTrue(true); - } - - @Test - public void testSameUserLock() throws Exception { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - AtomicBoolean gotLock = new AtomicBoolean(false); - new Thread(() -> { - lock.acquireUserLock("userOne"); - gotLock.set(true); - lock.releaseUserLock("userOne"); - }).start(); - // Let's give some time for the new thread to run - Thread.sleep(100); - // Since the new thread is trying to get lock on same user, it will wait. - Assert.assertFalse(gotLock.get()); - lock.releaseUserLock("userOne"); - // Since we have released the lock, the new thread should have the lock - // now - // Let's give some time for the new thread to run - Thread.sleep(100); - Assert.assertTrue(gotLock.get()); - } - - @Test(timeout = 1000) - public void testDifferentVolumeLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - lock.acquireVolumeLock("volTwo"); - lock.releaseVolumeLock("volOne"); - lock.releaseVolumeLock("volTwo"); - Assert.assertTrue(true); - } - - @Test - public void testSameVolumeLock() throws Exception { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - AtomicBoolean gotLock = new AtomicBoolean(false); - new Thread(() -> { - lock.acquireVolumeLock("volOne"); - gotLock.set(true); - lock.releaseVolumeLock("volOne"); - }).start(); - // Let's give some time for the new thread to run - Thread.sleep(100); - // Since the new thread is trying to get lock on same user, it will wait. - Assert.assertFalse(gotLock.get()); - lock.releaseVolumeLock("volOne"); - // Since we have released the lock, the new thread should have the lock - // now - // Let's give some time for the new thread to run - Thread.sleep(100); - Assert.assertTrue(gotLock.get()); - } - - @Test(timeout = 1000) - public void testDifferentBucketLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireBucketLock("volOne", "bucketOne"); - lock.acquireBucketLock("volOne", "bucketTwo"); - lock.releaseBucketLock("volOne", "bucketTwo"); - lock.releaseBucketLock("volOne", "bucketOne"); - Assert.assertTrue(true); - } - - @Test - public void testSameBucketLock() throws Exception { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireBucketLock("volOne", "bucketOne"); - AtomicBoolean gotLock = new AtomicBoolean(false); - new Thread(() -> { - lock.acquireBucketLock("volOne", "bucketOne"); - gotLock.set(true); - lock.releaseBucketLock("volOne", "bucketOne"); - }).start(); - // Let's give some time for the new thread to run - Thread.sleep(100); - // Since the new thread is trying to get lock on same user, it will wait. - Assert.assertFalse(gotLock.get()); - lock.releaseBucketLock("volOne", "bucketOne"); - // Since we have released the lock, the new thread should have the lock - // now - // Let's give some time for the new thread to run - Thread.sleep(100); - Assert.assertTrue(gotLock.get()); - } - - @Test(timeout = 1000) - public void testVolumeLockAfterUserLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - lock.acquireVolumeLock("volOne"); - lock.releaseVolumeLock("volOne"); - lock.releaseUserLock("userOne"); - Assert.assertTrue(true); - } - - @Test(timeout = 1000) - public void testBucketLockAfterVolumeLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - lock.acquireBucketLock("volOne", "bucketOne"); - lock.releaseBucketLock("volOne", "bucketOne"); - lock.releaseVolumeLock("volOne"); - Assert.assertTrue(true); - } - - @Test(timeout = 1000) - public void testBucketLockAfterVolumeLockAfterUserLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - lock.acquireVolumeLock("volOne"); - lock.acquireBucketLock("volOne", "bucketOne"); - lock.releaseBucketLock("volOne", "bucketOne"); - lock.releaseVolumeLock("volOne"); - lock.releaseUserLock("userOne"); - Assert.assertTrue(true); - } - - @Test - public void testUserLockAfterVolumeLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - try { - lock.acquireUserLock("userOne"); - Assert.fail(); - } catch (RuntimeException ex) { - String msg = - "cannot acquire user lock while holding " + - "volume, bucket or S3 bucket lock(s)."; - Assert.assertTrue(ex.getMessage().contains(msg)); - } - lock.releaseVolumeLock("volOne"); - Assert.assertTrue(true); - } - - @Test - public void testVolumeLockAfterBucketLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireBucketLock("volOne", "bucketOne"); - try { - lock.acquireVolumeLock("volOne"); - Assert.fail(); - } catch (RuntimeException ex) { - String msg = - "cannot acquire volume lock while holding bucket lock(s)."; - Assert.assertTrue(ex.getMessage().contains(msg)); - } - lock.releaseBucketLock("volOne", "bucketOne"); - Assert.assertTrue(true); - } - - -} \ No newline at end of file
- * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS,WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ - -package org.apache.hadoop.ozone.om; - -import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.junit.Assert; -import org.junit.Test; - -import java.util.concurrent.atomic.AtomicBoolean; - -/** - * Contains test-cases to verify OzoneManagerLock. - */ -public class TestOzoneManagerLock { - - @Test(timeout = 1000) - public void testDifferentUserLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - lock.acquireUserLock("userTwo"); - lock.releaseUserLock("userOne"); - lock.releaseUserLock("userTwo"); - Assert.assertTrue(true); - } - - @Test - public void testSameUserLock() throws Exception { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - AtomicBoolean gotLock = new AtomicBoolean(false); - new Thread(() -> { - lock.acquireUserLock("userOne"); - gotLock.set(true); - lock.releaseUserLock("userOne"); - }).start(); - // Let's give some time for the new thread to run - Thread.sleep(100); - // Since the new thread is trying to get lock on same user, it will wait. - Assert.assertFalse(gotLock.get()); - lock.releaseUserLock("userOne"); - // Since we have released the lock, the new thread should have the lock - // now - // Let's give some time for the new thread to run - Thread.sleep(100); - Assert.assertTrue(gotLock.get()); - } - - @Test(timeout = 1000) - public void testDifferentVolumeLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - lock.acquireVolumeLock("volTwo"); - lock.releaseVolumeLock("volOne"); - lock.releaseVolumeLock("volTwo"); - Assert.assertTrue(true); - } - - @Test - public void testSameVolumeLock() throws Exception { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - AtomicBoolean gotLock = new AtomicBoolean(false); - new Thread(() -> { - lock.acquireVolumeLock("volOne"); - gotLock.set(true); - lock.releaseVolumeLock("volOne"); - }).start(); - // Let's give some time for the new thread to run - Thread.sleep(100); - // Since the new thread is trying to get lock on same user, it will wait. - Assert.assertFalse(gotLock.get()); - lock.releaseVolumeLock("volOne"); - // Since we have released the lock, the new thread should have the lock - // now - // Let's give some time for the new thread to run - Thread.sleep(100); - Assert.assertTrue(gotLock.get()); - } - - @Test(timeout = 1000) - public void testDifferentBucketLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireBucketLock("volOne", "bucketOne"); - lock.acquireBucketLock("volOne", "bucketTwo"); - lock.releaseBucketLock("volOne", "bucketTwo"); - lock.releaseBucketLock("volOne", "bucketOne"); - Assert.assertTrue(true); - } - - @Test - public void testSameBucketLock() throws Exception { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireBucketLock("volOne", "bucketOne"); - AtomicBoolean gotLock = new AtomicBoolean(false); - new Thread(() -> { - lock.acquireBucketLock("volOne", "bucketOne"); - gotLock.set(true); - lock.releaseBucketLock("volOne", "bucketOne"); - }).start(); - // Let's give some time for the new thread to run - Thread.sleep(100); - // Since the new thread is trying to get lock on same user, it will wait. - Assert.assertFalse(gotLock.get()); - lock.releaseBucketLock("volOne", "bucketOne"); - // Since we have released the lock, the new thread should have the lock - // now - // Let's give some time for the new thread to run - Thread.sleep(100); - Assert.assertTrue(gotLock.get()); - } - - @Test(timeout = 1000) - public void testVolumeLockAfterUserLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - lock.acquireVolumeLock("volOne"); - lock.releaseVolumeLock("volOne"); - lock.releaseUserLock("userOne"); - Assert.assertTrue(true); - } - - @Test(timeout = 1000) - public void testBucketLockAfterVolumeLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - lock.acquireBucketLock("volOne", "bucketOne"); - lock.releaseBucketLock("volOne", "bucketOne"); - lock.releaseVolumeLock("volOne"); - Assert.assertTrue(true); - } - - @Test(timeout = 1000) - public void testBucketLockAfterVolumeLockAfterUserLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireUserLock("userOne"); - lock.acquireVolumeLock("volOne"); - lock.acquireBucketLock("volOne", "bucketOne"); - lock.releaseBucketLock("volOne", "bucketOne"); - lock.releaseVolumeLock("volOne"); - lock.releaseUserLock("userOne"); - Assert.assertTrue(true); - } - - @Test - public void testUserLockAfterVolumeLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireVolumeLock("volOne"); - try { - lock.acquireUserLock("userOne"); - Assert.fail(); - } catch (RuntimeException ex) { - String msg = - "cannot acquire user lock while holding " + - "volume, bucket or S3 bucket lock(s)."; - Assert.assertTrue(ex.getMessage().contains(msg)); - } - lock.releaseVolumeLock("volOne"); - Assert.assertTrue(true); - } - - @Test - public void testVolumeLockAfterBucketLock() { - OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); - lock.acquireBucketLock("volOne", "bucketOne"); - try { - lock.acquireVolumeLock("volOne"); - Assert.fail(); - } catch (RuntimeException ex) { - String msg = - "cannot acquire volume lock while holding bucket lock(s)."; - Assert.assertTrue(ex.getMessage().contains(msg)); - } - lock.releaseBucketLock("volOne", "bucketOne"); - Assert.assertTrue(true); - } - - -} \ No newline at end of file