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 42ce374e64..28f8907140 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/lock/OzoneManagerLock.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java index 7d62bc5af9..4267577cc4 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 @@ -99,10 +99,14 @@ public OzoneManagerLock(Configuration conf) { * * Special Note for UserLock: 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, 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) { + return OzoneManagerLockUtil.generateResourceLockName(resource, + resources[0]); + } else if (resources.length == 2 && resource == Resource.BUCKET) { + 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)) { @@ -141,6 +162,9 @@ private List getCurrentLocks() { */ public void acquireMultiUserLock(String firstUser, String secondUser) { Resource resource = Resource.USER; + firstUser = generateResourceName(resource, firstUser); + secondUser = generateResourceName(resource, secondUser); + if (!resource.canLock(lockSet.get())) { String errorMessage = getErrorMessage(resource); LOG.error(errorMessage); @@ -199,10 +223,12 @@ public void acquireMultiUserLock(String firstUser, String secondUser) { */ public void releaseMultiUserLock(String firstUser, String secondUser) { Resource resource = Resource.USER; + 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, 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. 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 27f28f0e06..f26256c0d6 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() { @@ -50,7 +51,7 @@ public static String generateResourceLockName( } else if (resource == OzoneManagerLock.Resource.S3_SECRET) { return OM_S3_SECRET + resourceName; } else if (resource == OzoneManagerLock.Resource.PREFIX) { - return OM_S3_PREFIX + resourceName; + 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 0df6cf2281..bdc3d19c91 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,33 +39,33 @@ 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()); @@ -73,22 +73,22 @@ private void testResourceReacquireLock(String resourceName, if (resource == OzoneManagerLock.Resource.USER || resource == OzoneManagerLock.Resource.S3_SECRET || resource == OzoneManagerLock.Resource.PREFIX){ - lock.acquireLock(resourceName, resource); + 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, "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.getName() == OzoneManagerLock.Resource.BUCKET.getName()) { + 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)."; 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, "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)."; Assert.assertTrue(ex.getMessage(), ex.getMessage().contains(message)); } - lock.releaseLock(userLock, OzoneManagerLock.Resource.USER); + lock.releaseLock(OzoneManagerLock.Resource.USER, "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, "user3"); fail("acquireUserLockAfterMultiUserLock failed"); } catch (RuntimeException ex) { String message = "cannot acquire USER lock while holding [USER] 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