diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/IOzoneManagerLock.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/IOzoneManagerLock.java index c19d9955b98d..808b9a4321ab 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/IOzoneManagerLock.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/IOzoneManagerLock.java @@ -25,46 +25,128 @@ */ public interface IOzoneManagerLock { - OMLockDetails acquireReadLock(OzoneManagerLock.LeveledResource resource, + OMLockDetails acquireReadLock(Resource resource, String... resources); - OMLockDetails acquireReadLocks(OzoneManagerLock.LeveledResource resource, Collection resources); + OMLockDetails acquireReadLocks(Resource resource, Collection resources); - OMLockDetails acquireWriteLock(OzoneManagerLock.LeveledResource resource, + OMLockDetails acquireWriteLock(Resource resource, String... resources); - OMLockDetails acquireWriteLocks(OzoneManagerLock.LeveledResource resource, + OMLockDetails acquireWriteLocks(Resource resource, Collection resources); boolean acquireMultiUserLock(String firstUser, String secondUser); void releaseMultiUserLock(String firstUser, String secondUser); - OMLockDetails releaseWriteLock(OzoneManagerLock.LeveledResource resource, + OMLockDetails releaseWriteLock(Resource resource, String... resources); - OMLockDetails releaseWriteLocks(OzoneManagerLock.LeveledResource resource, + OMLockDetails releaseWriteLocks(Resource resource, Collection resources); - OMLockDetails releaseReadLock(OzoneManagerLock.LeveledResource resource, + OMLockDetails releaseReadLock(Resource resource, String... resources); - OMLockDetails releaseReadLocks(OzoneManagerLock.LeveledResource resource, + OMLockDetails releaseReadLocks(Resource resource, Collection resources); @VisibleForTesting - int getReadHoldCount(OzoneManagerLock.LeveledResource resource, + int getReadHoldCount(Resource resource, String... resources); @VisibleForTesting - int getWriteHoldCount(OzoneManagerLock.LeveledResource resource, + int getWriteHoldCount(Resource resource, String... resources); @VisibleForTesting - boolean isWriteLockedByCurrentThread(OzoneManagerLock.LeveledResource resource, + boolean isWriteLockedByCurrentThread(Resource resource, String... resources); void cleanup(); OMLockMetrics getOMLockMetrics(); + + /** + * Defines a resource interface used to represent entities that can be + * associated with locks in the Ozone Manager Lock mechanism. A resource + * implementation provides a name and an associated {@link ResourceManager} + * to manage its locking behavior. + */ + interface Resource { + + String getName(); + + ResourceManager getResourceManager(); + } + + /** + * The ResourceManager class provides functionality for managing + * information about resource read and write lock usage. It tracks the time of + * read and write locks acquired and held by individual threads, enabling + * more granular lock usage metrics. + */ + class ResourceManager { + // This helps in maintaining read lock related variables locally confined + // to a given thread. + private final ThreadLocal readLockTimeStampNanos = + ThreadLocal.withInitial(LockUsageInfo::new); + + // This helps in maintaining write lock related variables locally confined + // to a given thread. + private final ThreadLocal writeLockTimeStampNanos = + ThreadLocal.withInitial(LockUsageInfo::new); + + ResourceManager() { + } + + /** + * Sets the time (ns) when the read lock holding period begins specific to a + * thread. + * + * @param startReadHeldTimeNanos read lock held start time (ns) + */ + void setStartReadHeldTimeNanos(long startReadHeldTimeNanos) { + readLockTimeStampNanos.get() + .setStartReadHeldTimeNanos(startReadHeldTimeNanos); + } + + /** + * Sets the time (ns) when the write lock holding period begins specific to + * a thread. + * + * @param startWriteHeldTimeNanos write lock held start time (ns) + */ + void setStartWriteHeldTimeNanos(long startWriteHeldTimeNanos) { + writeLockTimeStampNanos.get() + .setStartWriteHeldTimeNanos(startWriteHeldTimeNanos); + } + + /** + * Returns the time (ns) when the read lock holding period began specific to + * a thread. + * + * @return read lock held start time (ns) + */ + long getStartReadHeldTimeNanos() { + long startReadHeldTimeNanos = + readLockTimeStampNanos.get().getStartReadHeldTimeNanos(); + readLockTimeStampNanos.remove(); + return startReadHeldTimeNanos; + } + + /** + * Returns the time (ns) when the write lock holding period began specific + * to a thread. + * + * @return write lock held start time (ns) + */ + long getStartWriteHeldTimeNanos() { + long startWriteHeldTimeNanos = + writeLockTimeStampNanos.get().getStartWriteHeldTimeNanos(); + writeLockTimeStampNanos.remove(); + return startWriteHeldTimeNanos; + } + } } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java index 17f5e2f04644..0c289cb18890 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java @@ -21,7 +21,6 @@ import static org.apache.hadoop.ozone.om.lock.OMLockDetails.EMPTY_DETAILS_LOCK_NOT_ACQUIRED; import java.util.Collection; -import org.apache.hadoop.ozone.om.lock.OzoneManagerLock.LeveledResource; /** * Read only "lock" for snapshots @@ -31,23 +30,23 @@ public class OmReadOnlyLock implements IOzoneManagerLock { @Override - public OMLockDetails acquireReadLock(LeveledResource resource, String... resources) { + public OMLockDetails acquireReadLock(Resource resource, String... resources) { return EMPTY_DETAILS_LOCK_ACQUIRED; } @Override - public OMLockDetails acquireReadLocks(LeveledResource resource, Collection resources) { + public OMLockDetails acquireReadLocks(Resource resource, Collection resources) { return EMPTY_DETAILS_LOCK_ACQUIRED; } @Override - public OMLockDetails acquireWriteLock(LeveledResource resource, + public OMLockDetails acquireWriteLock(Resource resource, String... resources) { return EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } @Override - public OMLockDetails acquireWriteLocks(LeveledResource resource, Collection resources) { + public OMLockDetails acquireWriteLocks(Resource resource, Collection resources) { return EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } @@ -62,38 +61,38 @@ public void releaseMultiUserLock(String firstUser, String secondUser) { } @Override - public OMLockDetails releaseWriteLock(LeveledResource resource, + public OMLockDetails releaseWriteLock(Resource resource, String... resources) { return EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } @Override - public OMLockDetails releaseWriteLocks(LeveledResource resource, Collection resources) { + public OMLockDetails releaseWriteLocks(Resource resource, Collection resources) { return EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } @Override - public OMLockDetails releaseReadLock(LeveledResource resource, String... resources) { + public OMLockDetails releaseReadLock(Resource resource, String... resources) { return EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } @Override - public OMLockDetails releaseReadLocks(LeveledResource resource, Collection resources) { + public OMLockDetails releaseReadLocks(Resource resource, Collection resources) { return EMPTY_DETAILS_LOCK_NOT_ACQUIRED; } @Override - public int getReadHoldCount(LeveledResource resource, String... resources) { + public int getReadHoldCount(Resource resource, String... resources) { return 0; } @Override - public int getWriteHoldCount(LeveledResource resource, String... resources) { + public int getWriteHoldCount(Resource resource, String... resources) { return 0; } @Override - public boolean isWriteLockedByCurrentThread(LeveledResource resource, + public boolean isWriteLockedByCurrentThread(Resource resource, String... resources) { return false; } 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 eb0df99a0a58..2a62836bed9b 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 @@ -24,6 +24,7 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_MANAGER_STRIPED_LOCK_SIZE_PREFIX; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.Striped; import java.util.ArrayList; import java.util.Arrays; @@ -37,7 +38,9 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; +import java.util.stream.Stream; import java.util.stream.StreamSupport; +import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.utils.CompositeKey; import org.apache.hadoop.hdds.utils.SimpleStriped; @@ -93,14 +96,10 @@ public class OzoneManagerLock implements IOzoneManagerLock { private static final Logger LOG = LoggerFactory.getLogger(OzoneManagerLock.class); - private final Map> stripedLockByResource; + private final Map, + Pair>, ResourceLockManager>> resourcelockMap; private OMLockMetrics omLockMetrics; - private final ThreadLocal lockSet = ThreadLocal.withInitial( - () -> Short.valueOf((short)0)); - - private ThreadLocal omLockDetails = - ThreadLocal.withInitial(OMLockDetails::new); /** * Creates new OzoneManagerLock instance. @@ -108,15 +107,29 @@ public class OzoneManagerLock implements IOzoneManagerLock { */ public OzoneManagerLock(ConfigurationSource conf) { omLockMetrics = OMLockMetrics.create(); - Map> stripedLockMap = - new EnumMap<>(LeveledResource.class); + this.resourcelockMap = ImmutableMap.of(LeveledResource.class, getLeveledLocks(conf), FlatResource.class, + getFlatLocks(conf)); + } + + private Pair>, ResourceLockManager> getLeveledLocks( + ConfigurationSource conf) { + Map> stripedLockMap = new EnumMap<>(LeveledResource.class); for (LeveledResource r : LeveledResource.values()) { stripedLockMap.put(r, createStripeLock(r, conf)); } - this.stripedLockByResource = Collections.unmodifiableMap(stripedLockMap); + return Pair.of(Collections.unmodifiableMap(stripedLockMap), new LeveledResourceLockManager()); + } + + private Pair>, ResourceLockManager> getFlatLocks( + ConfigurationSource conf) { + Map> stripedLockMap = new EnumMap<>(FlatResource.class); + for (FlatResource r : FlatResource.values()) { + stripedLockMap.put(r, createStripeLock(r, conf)); + } + return Pair.of(Collections.unmodifiableMap(stripedLockMap), new FlatResourceLockManager()); } - private Striped createStripeLock(LeveledResource r, + private Striped createStripeLock(Resource r, ConfigurationSource conf) { boolean fair = conf.getBoolean(OZONE_MANAGER_FAIR_LOCK, OZONE_MANAGER_FAIR_LOCK_DEFAULT); @@ -127,8 +140,9 @@ private Striped createStripeLock(LeveledResource r, return SimpleStriped.readWriteLock(size, fair); } - private Iterable bulkGetLock(LeveledResource resource, Collection keys) { - Striped striped = stripedLockByResource.get(resource); + private Iterable bulkGetLock(Map> lockMap, Resource resource, + Collection keys) { + Striped striped = lockMap.get(resource); List lockKeys = new ArrayList<>(keys.size()); for (String[] key : keys) { if (Objects.nonNull(key)) { @@ -138,8 +152,9 @@ private Iterable bulkGetLock(LeveledResource resource, Collection return striped.bulkGet(lockKeys); } - private ReentrantReadWriteLock getLock(LeveledResource resource, String... keys) { - Striped striped = stripedLockByResource.get(resource); + private ReentrantReadWriteLock getLock(Map> lockMap, Resource resource, + String... keys) { + Striped striped = lockMap.get(resource); Object key = combineKeys(keys); return (ReentrantReadWriteLock) striped.get(key); } @@ -162,7 +177,7 @@ private ReentrantReadWriteLock getLock(LeveledResource resource, String... keys) * be passed. */ @Override - public OMLockDetails acquireReadLock(LeveledResource resource, String... keys) { + public OMLockDetails acquireReadLock(Resource resource, String... keys) { return acquireLock(resource, true, keys); } @@ -184,7 +199,7 @@ public OMLockDetails acquireReadLock(LeveledResource resource, String... keys) { * be passed. */ @Override - public OMLockDetails acquireReadLocks(LeveledResource resource, Collection keys) { + public OMLockDetails acquireReadLocks(Resource resource, Collection keys) { return acquireLocks(resource, true, keys); } @@ -206,7 +221,7 @@ public OMLockDetails acquireReadLocks(LeveledResource resource, Collection keys) { + public OMLockDetails acquireWriteLocks(Resource resource, Collection keys) { return acquireLocks(resource, false, keys); } - private void acquireLock(LeveledResource resource, boolean isReadLock, ReadWriteLock lock, + private void acquireLock(Resource resource, boolean isReadLock, ReadWriteLock lock, long startWaitingTimeNanos) { if (isReadLock) { lock.readLock().lock(); @@ -243,10 +258,12 @@ private void acquireLock(LeveledResource resource, boolean isReadLock, ReadWrite } } - private OMLockDetails acquireLocks(LeveledResource resource, boolean isReadLock, - Collection keys) { - omLockDetails.get().clear(); - if (!resource.canLock(lockSet.get())) { + private OMLockDetails acquireLocks(Resource resource, boolean isReadLock, Collection keys) { + Pair>, ResourceLockManager> resourceLockPair = + resourcelockMap.get(resource.getClass()); + ResourceLockManager resourceLockManager = resourceLockPair.getRight(); + resourceLockManager.clearLockDetails(); + if (!resourceLockManager.canLockResource(resource)) { String errorMessage = getErrorMessage(resource); LOG.error(errorMessage); throw new RuntimeException(errorMessage); @@ -254,19 +271,18 @@ private OMLockDetails acquireLocks(LeveledResource resource, boolean isReadLock, long startWaitingTimeNanos = Time.monotonicNowNanos(); - for (ReadWriteLock lock : bulkGetLock(resource, keys)) { + for (ReadWriteLock lock : bulkGetLock(resourceLockPair.getKey(), resource, keys)) { acquireLock(resource, isReadLock, lock, startWaitingTimeNanos); } - - lockSet.set(resource.setLock(lockSet.get())); - omLockDetails.get().setLockAcquired(true); - return omLockDetails.get(); + return resourceLockManager.lockResource(resource); } - private OMLockDetails acquireLock(LeveledResource resource, boolean isReadLock, - String... keys) { - omLockDetails.get().clear(); - if (!resource.canLock(lockSet.get())) { + private OMLockDetails acquireLock(Resource resource, boolean isReadLock, String... keys) { + Pair>, ResourceLockManager> resourceLockPair = + resourcelockMap.get(resource.getClass()); + ResourceLockManager resourceLockManager = resourceLockPair.getRight(); + resourceLockManager.clearLockDetails(); + if (!resourceLockManager.canLockResource(resource)) { String errorMessage = getErrorMessage(resource); LOG.error(errorMessage); throw new RuntimeException(errorMessage); @@ -274,15 +290,12 @@ private OMLockDetails acquireLock(LeveledResource resource, boolean isReadLock, long startWaitingTimeNanos = Time.monotonicNowNanos(); - ReentrantReadWriteLock lock = getLock(resource, keys); + ReentrantReadWriteLock lock = getLock(resourceLockPair.getKey(), resource, keys); acquireLock(resource, isReadLock, lock, startWaitingTimeNanos); - - lockSet.set(resource.setLock(lockSet.get())); - omLockDetails.get().setLockAcquired(true); - return omLockDetails.get(); + return resourceLockManager.lockResource(resource); } - private void updateReadLockMetrics(LeveledResource resource, + private void updateReadLockMetrics(Resource resource, ReentrantReadWriteLock lock, long startWaitingTimeNanos) { /* @@ -296,13 +309,14 @@ private void updateReadLockMetrics(LeveledResource resource, // Adds a snapshot to the metric readLockWaitingTimeMsStat. omLockMetrics.setReadLockWaitingTimeMsStat( TimeUnit.NANOSECONDS.toMillis(readLockWaitingTimeNanos)); - updateProcessingDetails(Timing.LOCKWAIT, readLockWaitingTimeNanos); + updateProcessingDetails(resourcelockMap.get(resource.getClass()).getValue(), + Timing.LOCKWAIT, readLockWaitingTimeNanos); - resource.setStartReadHeldTimeNanos(Time.monotonicNowNanos()); + resource.getResourceManager().setStartReadHeldTimeNanos(Time.monotonicNowNanos()); } } - private void updateWriteLockMetrics(LeveledResource resource, + private void updateWriteLockMetrics(Resource resource, ReentrantReadWriteLock lock, long startWaitingTimeNanos) { /* * writeHoldCount helps in metrics updation only once in case @@ -317,29 +331,26 @@ private void updateWriteLockMetrics(LeveledResource resource, // Adds a snapshot to the metric writeLockWaitingTimeMsStat. omLockMetrics.setWriteLockWaitingTimeMsStat( TimeUnit.NANOSECONDS.toMillis(writeLockWaitingTimeNanos)); - updateProcessingDetails(Timing.LOCKWAIT, writeLockWaitingTimeNanos); + updateProcessingDetails(resourcelockMap.get(resource.getClass()).getValue(), Timing.LOCKWAIT, + writeLockWaitingTimeNanos); - resource.setStartWriteHeldTimeNanos(Time.monotonicNowNanos()); + resource.getResourceManager().setStartWriteHeldTimeNanos(Time.monotonicNowNanos()); } } - private String getErrorMessage(LeveledResource resource) { + private String getErrorMessage(Resource resource) { return "Thread '" + Thread.currentThread().getName() + "' cannot " + - "acquire " + resource.name + " lock while holding " + + "acquire " + resource.getName() + " lock while holding " + getCurrentLocks().toString() + " lock(s)."; } @VisibleForTesting List getCurrentLocks() { - List currentLocks = new ArrayList<>(); - short lockSetVal = lockSet.get(); - for (LeveledResource value : LeveledResource.values()) { - if (value.isLevelLocked(lockSetVal)) { - currentLocks.add(value.getName()); - } - } - return currentLocks; + return resourcelockMap.values().stream().map(Pair::getValue) + .flatMap(rlm -> ((ResourceLockManager)rlm).getCurrentLockedResources()) + .map(Resource::getName) + .collect(Collectors.toList()); } /** @@ -347,29 +358,10 @@ List getCurrentLocks() { */ @Override public boolean acquireMultiUserLock(String firstUser, String secondUser) { - LeveledResource resource = LeveledResource.USER_LOCK; - - if (!resource.canLock(lockSet.get())) { - String errorMessage = getErrorMessage(resource); - LOG.error(errorMessage); - throw new RuntimeException(errorMessage); - } else { - Striped striped = - stripedLockByResource.get(LeveledResource.USER_LOCK); - // The result of bulkGet is always sorted in a consistent order. - // This prevents deadlocks. - Iterable locks = - striped.bulkGet(Arrays.asList(firstUser, secondUser)); - for (ReadWriteLock lock : locks) { - lock.writeLock().lock(); - } - - lockSet.set(resource.setLock(lockSet.get())); - return true; - } + return acquireWriteLocks(LeveledResource.USER_LOCK, + Arrays.asList(new String[] {firstUser}, new String[] {secondUser})).isLockAcquired(); } - /** * Release lock on multiple users. * @param firstUser @@ -377,15 +369,8 @@ public boolean acquireMultiUserLock(String firstUser, String secondUser) { */ @Override public void releaseMultiUserLock(String firstUser, String secondUser) { - Striped striped = - stripedLockByResource.get(LeveledResource.USER_LOCK); - Iterable locks = - striped.bulkGet(Arrays.asList(firstUser, secondUser)); - for (ReadWriteLock lock : locks) { - lock.writeLock().unlock(); - } - - lockSet.set(LeveledResource.USER_LOCK.clearLock(lockSet.get())); + releaseWriteLocks(LeveledResource.USER_LOCK, + Arrays.asList(new String[] {firstUser}, new String[] {secondUser})); } @@ -398,7 +383,7 @@ public void releaseMultiUserLock(String firstUser, String secondUser) { * be passed. */ @Override - public OMLockDetails releaseWriteLock(LeveledResource resource, String... keys) { + public OMLockDetails releaseWriteLock(Resource resource, String... keys) { return releaseLock(resource, false, keys); } @@ -411,7 +396,7 @@ public OMLockDetails releaseWriteLock(LeveledResource resource, String... keys) * be passed. */ @Override - public OMLockDetails releaseWriteLocks(LeveledResource resource, Collection keys) { + public OMLockDetails releaseWriteLocks(Resource resource, Collection keys) { return releaseLocks(resource, false, keys); } @@ -424,7 +409,7 @@ public OMLockDetails releaseWriteLocks(LeveledResource resource, Collection keys) { + public OMLockDetails releaseReadLocks(Resource resource, Collection keys) { return releaseLocks(resource, true, keys); } - private OMLockDetails releaseLock(LeveledResource resource, boolean isReadLock, + private OMLockDetails releaseLock(Resource resource, boolean isReadLock, String... keys) { - omLockDetails.get().clear(); - ReentrantReadWriteLock lock = getLock(resource, keys); + Pair>, ResourceLockManager> resourceLockPair = + resourcelockMap.get(resource.getClass()); + ResourceLockManager resourceLockManager = resourceLockPair.getRight(); + resourceLockManager.clearLockDetails(); + ReentrantReadWriteLock lock = getLock(resourceLockPair.getKey(), resource, keys); if (isReadLock) { lock.readLock().unlock(); updateReadUnlockMetrics(resource, lock); @@ -453,16 +441,17 @@ private OMLockDetails releaseLock(LeveledResource resource, boolean isReadLock, lock.writeLock().unlock(); updateWriteUnlockMetrics(resource, lock, isWriteLocked); } - - lockSet.set(resource.clearLock(lockSet.get())); - return omLockDetails.get(); + return resourceLockManager.unlockResource(resource); } - private OMLockDetails releaseLocks(LeveledResource resource, boolean isReadLock, - Collection keys) { - omLockDetails.get().clear(); - List locks = - StreamSupport.stream(bulkGetLock(resource, keys).spliterator(), false).collect(Collectors.toList()); + private OMLockDetails releaseLocks(Resource resource, boolean isReadLock, + Collection keys) { + Pair>, ResourceLockManager> resourceLockPair = + resourcelockMap.get(resource.getClass()); + ResourceLockManager resourceLockManager = resourceLockPair.getRight(); + resourceLockManager.clearLockDetails(); + List locks = StreamSupport.stream(bulkGetLock(resourceLockPair.getKey(), resource, keys) + .spliterator(), false).collect(Collectors.toList()); // Release locks in reverse order. Collections.reverse(locks); for (ReadWriteLock lock : locks) { @@ -475,12 +464,10 @@ private OMLockDetails releaseLocks(LeveledResource resource, boolean isReadLock, updateWriteUnlockMetrics(resource, (ReentrantReadWriteLock) lock, isWriteLocked); } } - - lockSet.set(resource.clearLock(lockSet.get())); - return omLockDetails.get(); + return resourceLockManager.unlockResource(resource); } - private void updateReadUnlockMetrics(LeveledResource resource, + private void updateReadUnlockMetrics(Resource resource, ReentrantReadWriteLock lock) { /* * readHoldCount helps in metrics updation only once in case @@ -488,16 +475,17 @@ private void updateReadUnlockMetrics(LeveledResource resource, */ if (lock.getReadHoldCount() == 0) { long readLockHeldTimeNanos = - Time.monotonicNowNanos() - resource.getStartReadHeldTimeNanos(); + Time.monotonicNowNanos() - resource.getResourceManager().getStartReadHeldTimeNanos(); // Adds a snapshot to the metric readLockHeldTimeMsStat. omLockMetrics.setReadLockHeldTimeMsStat( TimeUnit.NANOSECONDS.toMillis(readLockHeldTimeNanos)); - updateProcessingDetails(Timing.LOCKSHARED, readLockHeldTimeNanos); + updateProcessingDetails(resourcelockMap.get(resource.getClass()).getValue(), Timing.LOCKSHARED, + readLockHeldTimeNanos); } } - private void updateWriteUnlockMetrics(LeveledResource resource, + private void updateWriteUnlockMetrics(Resource resource, ReentrantReadWriteLock lock, boolean isWriteLocked) { /* * writeHoldCount helps in metrics updation only once in case @@ -506,12 +494,13 @@ private void updateWriteUnlockMetrics(LeveledResource resource, */ if ((lock.getWriteHoldCount() == 0) && isWriteLocked) { long writeLockHeldTimeNanos = - Time.monotonicNowNanos() - resource.getStartWriteHeldTimeNanos(); + Time.monotonicNowNanos() - resource.getResourceManager().getStartWriteHeldTimeNanos(); // Adds a snapshot to the metric writeLockHeldTimeMsStat. omLockMetrics.setWriteLockHeldTimeMsStat( TimeUnit.NANOSECONDS.toMillis(writeLockHeldTimeNanos)); - updateProcessingDetails(Timing.LOCKEXCLUSIVE, writeLockHeldTimeNanos); + updateProcessingDetails(resourcelockMap.get(resource.getClass()).getValue(), Timing.LOCKEXCLUSIVE, + writeLockHeldTimeNanos); } } @@ -522,8 +511,8 @@ private void updateWriteUnlockMetrics(LeveledResource resource, */ @Override @VisibleForTesting - public int getReadHoldCount(LeveledResource resource, String... keys) { - return getLock(resource, keys).getReadHoldCount(); + public int getReadHoldCount(Resource resource, String... keys) { + return getLock(resourcelockMap.get(resource.getClass()).getKey(), resource, keys).getReadHoldCount(); } @@ -534,8 +523,8 @@ public int getReadHoldCount(LeveledResource resource, String... keys) { */ @Override @VisibleForTesting - public int getWriteHoldCount(LeveledResource resource, String... keys) { - return getLock(resource, keys).getWriteHoldCount(); + public int getWriteHoldCount(Resource resource, String... keys) { + return getLock(resourcelockMap.get(resource.getClass()).getKey(), resource, keys).getWriteHoldCount(); } /** @@ -547,9 +536,9 @@ public int getWriteHoldCount(LeveledResource resource, String... keys) { */ @Override @VisibleForTesting - public boolean isWriteLockedByCurrentThread(LeveledResource resource, + public boolean isWriteLockedByCurrentThread(Resource resource, String... keys) { - return getLock(resource, keys).isWriteLockedByCurrentThread(); + return getLock(resourcelockMap.get(resource.getClass()).getKey(), resource, keys).isWriteLockedByCurrentThread(); } /** @@ -566,9 +555,120 @@ public OMLockMetrics getOMLockMetrics() { } /** - * Resource defined in Ozone. + * Flat Resource defined in Ozone. Locks can be acquired on a resource independent of one another. + */ + public enum FlatResource implements Resource { + SNAPSHOT_GC_LOCK("SNAPSHOT_GC_LOCK"); + + private String name; + private ResourceManager resourceManager; + + FlatResource(String name) { + this.name = name; + this.resourceManager = new ResourceManager(); + } + + @Override + public String getName() { + return name; + } + + @Override + public ResourceManager getResourceManager() { + return resourceManager; + } + } + + private abstract static class ResourceLockManager { + + private final ThreadLocal omLockDetails = ThreadLocal.withInitial(OMLockDetails::new); + + abstract boolean canLockResource(T resource); + + abstract Stream getCurrentLockedResources(); + + OMLockDetails clearLockDetails() { + omLockDetails.get().clear(); + return omLockDetails.get(); + } + + OMLockDetails unlockResource(T resource) { + return omLockDetails.get(); + } + + OMLockDetails lockResource(T resource) { + omLockDetails.get().setLockAcquired(true); + return omLockDetails.get(); + } + } + + private static final class FlatResourceLockManager extends ResourceLockManager { + + private EnumMap> acquiredLocksMap = new EnumMap<>(FlatResource.class); + + private FlatResourceLockManager() { + for (FlatResource flatResource : FlatResource.values()) { + acquiredLocksMap.put(flatResource, ThreadLocal.withInitial(() -> Boolean.FALSE)); + } + } + + @Override + OMLockDetails lockResource(FlatResource resource) { + acquiredLocksMap.get(resource).set(Boolean.TRUE); + return super.lockResource(resource); + } + + @Override + OMLockDetails unlockResource(FlatResource resource) { + acquiredLocksMap.get(resource).set(Boolean.FALSE); + return super.unlockResource(resource); + } + + @Override + public boolean canLockResource(FlatResource resource) { + return true; + } + + @Override + Stream getCurrentLockedResources() { + return acquiredLocksMap.keySet().stream().filter(flatResource -> acquiredLocksMap.get(flatResource).get()); + } + } + + private static final class LeveledResourceLockManager extends ResourceLockManager { + private final ThreadLocal lockSet = ThreadLocal.withInitial(() -> Short.valueOf((short)0)); + + @Override + public boolean canLockResource(LeveledResource resource) { + return resource.canLock(lockSet.get()); + } + + @Override + Stream getCurrentLockedResources() { + short lockSetVal = lockSet.get(); + return Arrays.stream(LeveledResource.values()) + .filter(leveledResource -> leveledResource.isLevelLocked(lockSetVal)); + } + + @Override + public OMLockDetails unlockResource(LeveledResource resource) { + lockSet.set(resource.clearLock(lockSet.get())); + return super.unlockResource(resource); + } + + @Override + public OMLockDetails lockResource(LeveledResource resource) { + lockSet.set(resource.setLock(lockSet.get())); + return super.lockResource(resource); + } + } + + /** + * Leveled Resource defined in Ozone. + * Enforces lock acquisition ordering based on the resource level. A resource at lower level cannot be acquired + * after a higher level lock is already acquired. */ - public enum LeveledResource { + public enum LeveledResource implements Resource { // For S3 Bucket need to allow only for S3, that should be means only 1. S3_BUCKET_LOCK((byte) 0, "S3_BUCKET_LOCK"), // = 1 @@ -586,8 +686,7 @@ public enum LeveledResource { S3_SECRET_LOCK((byte) 4, "S3_SECRET_LOCK"), // 31 KEY_PATH_LOCK((byte) 5, "KEY_PATH_LOCK"), //63 PREFIX_LOCK((byte) 6, "PREFIX_LOCK"), //127 - SNAPSHOT_LOCK((byte) 7, "SNAPSHOT_LOCK"), // = 255 - SNAPSHOT_GC_LOCK((byte) 8, "SNAPSHOT_GC_LOCK"); + SNAPSHOT_LOCK((byte) 7, "SNAPSHOT_LOCK"); // = 255 // level of the resource private byte lockLevel; @@ -602,69 +701,14 @@ public enum LeveledResource { // Name of the resource. private String name; - // This helps in maintaining read lock related variables locally confined - // to a given thread. - private final ThreadLocal readLockTimeStampNanos = - ThreadLocal.withInitial(LockUsageInfo::new); - - // This helps in maintaining write lock related variables locally confined - // to a given thread. - private final ThreadLocal writeLockTimeStampNanos = - ThreadLocal.withInitial(LockUsageInfo::new); - - /** - * Sets the time (ns) when the read lock holding period begins specific to a - * thread. - * - * @param startReadHeldTimeNanos read lock held start time (ns) - */ - void setStartReadHeldTimeNanos(long startReadHeldTimeNanos) { - readLockTimeStampNanos.get() - .setStartReadHeldTimeNanos(startReadHeldTimeNanos); - } - - /** - * Sets the time (ns) when the write lock holding period begins specific to - * a thread. - * - * @param startWriteHeldTimeNanos write lock held start time (ns) - */ - void setStartWriteHeldTimeNanos(long startWriteHeldTimeNanos) { - writeLockTimeStampNanos.get() - .setStartWriteHeldTimeNanos(startWriteHeldTimeNanos); - } - - /** - * Returns the time (ns) when the read lock holding period began specific to - * a thread. - * - * @return read lock held start time (ns) - */ - long getStartReadHeldTimeNanos() { - long startReadHeldTimeNanos = - readLockTimeStampNanos.get().getStartReadHeldTimeNanos(); - readLockTimeStampNanos.remove(); - return startReadHeldTimeNanos; - } - - /** - * Returns the time (ns) when the write lock holding period began specific - * to a thread. - * - * @return write lock held start time (ns) - */ - long getStartWriteHeldTimeNanos() { - long startWriteHeldTimeNanos = - writeLockTimeStampNanos.get().getStartWriteHeldTimeNanos(); - writeLockTimeStampNanos.remove(); - return startWriteHeldTimeNanos; - } + private ResourceManager resourceManager; LeveledResource(byte pos, String name) { this.lockLevel = pos; this.mask = (short) (Math.pow(2, lockLevel + 1) - 1); this.setMask = (short) Math.pow(2, lockLevel); this.name = name; + this.resourceManager = new ResourceManager(); } boolean canLock(short lockSetVal) { @@ -720,10 +764,16 @@ boolean isLevelLocked(short lockSetVal) { return (lockSetVal & setMask) == setMask; } - String getName() { + @Override + public String getName() { return name; } + @Override + public ResourceManager getResourceManager() { + return resourceManager; + } + short getMask() { return mask; } @@ -739,20 +789,21 @@ short getMask() { * @param type IPC Timing types * @param deltaNanos consumed time */ - private void updateProcessingDetails(Timing type, long deltaNanos) { + private void updateProcessingDetails(ResourceLockManager resourceLockManager, Timing type, + long deltaNanos) { Server.Call call = Server.getCurCall().get(); if (call != null) { call.getProcessingDetails().add(type, deltaNanos, TimeUnit.NANOSECONDS); } else { switch (type) { case LOCKWAIT: - omLockDetails.get().add(deltaNanos, OMLockDetails.LockOpType.WAIT); + resourceLockManager.omLockDetails.get().add(deltaNanos, OMLockDetails.LockOpType.WAIT); break; case LOCKSHARED: - omLockDetails.get().add(deltaNanos, OMLockDetails.LockOpType.READ); + resourceLockManager.omLockDetails.get().add(deltaNanos, OMLockDetails.LockOpType.READ); break; case LOCKEXCLUSIVE: - omLockDetails.get().add(deltaNanos, OMLockDetails.LockOpType.WRITE); + resourceLockManager.omLockDetails.get().add(deltaNanos, OMLockDetails.LockOpType.WRITE); break; default: LOG.error("Unsupported Timing type {}", type); 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 a3d3a40eda3a..500a96e29a46 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 @@ -30,9 +30,12 @@ import java.util.Stack; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.metrics2.MetricsRecord; import org.apache.hadoop.metrics2.impl.MetricsCollectorImpl; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock.Resource; +import org.apache.hadoop.ozone.om.lock.OzoneManagerLock.FlatResource; import org.apache.hadoop.ozone.om.lock.OzoneManagerLock.LeveledResource; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -115,6 +118,26 @@ void testLockingOrder() { } } + @ParameterizedTest + @EnumSource + public void testFlatLockWithParallelResource(FlatResource flatResource) { + OzoneManagerLock lock = new OzoneManagerLock(new OzoneConfiguration()); + List resources = new ArrayList<>(); + resources.addAll(Arrays.stream(LeveledResource.values()).collect(Collectors.toList())); + resources.addAll(Arrays.stream(FlatResource.values()).collect(Collectors.toList())); + for (Resource otherResource : resources) { + String[] otherResourceName = generateResourceName(otherResource); + String[] flatResourceName = generateResourceName(flatResource); + lock.acquireWriteLock(otherResource, otherResourceName); + try { + lock.acquireWriteLock(flatResource, flatResourceName); + } finally { + lock.releaseWriteLock(otherResource, otherResourceName); + lock.releaseWriteLock(flatResource, flatResourceName); + } + } + } + @ParameterizedTest @EnumSource void testLockViolationsWithOneHigherLevelLock(LeveledResource resource) { @@ -181,7 +204,7 @@ void releaseLockWithOutAcquiringLock() { () -> lock.releaseWriteLock(LeveledResource.USER_LOCK, "user3")); } - private String[] generateResourceName(LeveledResource resource) { + private String[] generateResourceName(Resource resource) { if (resource == LeveledResource.BUCKET_LOCK) { return new String[]{UUID.randomUUID().toString(), UUID.randomUUID().toString()}; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java index fc92499c44ff..525877306965 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java @@ -26,8 +26,8 @@ import java.util.stream.Collectors; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; +import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock.Resource; import org.apache.hadoop.ozone.om.lock.OMLockDetails; -import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; /** * Class to take multiple locks on multiple snapshots. @@ -35,11 +35,11 @@ public class MultiSnapshotLocks { private final List objectLocks; private final IOzoneManagerLock lock; - private final OzoneManagerLock.LeveledResource resource; + private final Resource resource; private final boolean writeLock; private OMLockDetails lockDetails; - public MultiSnapshotLocks(IOzoneManagerLock lock, OzoneManagerLock.LeveledResource resource, boolean writeLock) { + public MultiSnapshotLocks(IOzoneManagerLock lock, Resource resource, boolean writeLock) { this.writeLock = writeLock; this.resource = resource; this.lock = lock; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java index d67159b1a569..1f97fe10d806 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java @@ -17,6 +17,8 @@ package org.apache.hadoop.ozone.om.snapshot.filter; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.FlatResource.SNAPSHOT_GC_LOCK; + import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; @@ -34,7 +36,6 @@ import org.apache.hadoop.ozone.om.helpers.OmBucketInfo; import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; -import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; import org.apache.hadoop.ozone.om.snapshot.MultiSnapshotLocks; import org.apache.hadoop.ozone.om.snapshot.ReferenceCounted; import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils; @@ -88,7 +89,7 @@ public ReclaimableFilter( this.omSnapshotManager = omSnapshotManager; this.currentSnapshotInfo = currentSnapshotInfo; this.snapshotChainManager = snapshotChainManager; - this.snapshotIdLocks = new MultiSnapshotLocks(lock, OzoneManagerLock.LeveledResource.SNAPSHOT_GC_LOCK, false); + this.snapshotIdLocks = new MultiSnapshotLocks(lock, SNAPSHOT_GC_LOCK, false); this.keyManager = keyManager; this.numberOfPreviousSnapshotsFromChain = numberOfPreviousSnapshotsFromChain; this.previousOmSnapshots = new ArrayList<>(numberOfPreviousSnapshotsFromChain); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java index 2453487e5c91..4f133868f949 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java @@ -19,6 +19,7 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS; import static org.apache.hadoop.ozone.OzoneConsts.TRANSACTION_INFO_KEY; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.FlatResource.SNAPSHOT_GC_LOCK; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyList; @@ -58,7 +59,6 @@ import org.apache.hadoop.ozone.om.helpers.SnapshotInfo; import org.apache.hadoop.ozone.om.lock.IOzoneManagerLock; import org.apache.hadoop.ozone.om.lock.OMLockDetails; -import org.apache.hadoop.ozone.om.lock.OzoneManagerLock; import org.apache.hadoop.ozone.om.snapshot.ReferenceCounted; import org.apache.hadoop.ozone.om.snapshot.SnapshotCache; import org.apache.hadoop.ozone.om.snapshot.SnapshotDiffManager; @@ -125,14 +125,14 @@ protected SnapshotInfo setup( this.snapshotChainManager = mock(SnapshotChainManager.class); this.keyManager = mock(KeyManager.class); IOzoneManagerLock ozoneManagerLock = mock(IOzoneManagerLock.class); - when(ozoneManagerLock.acquireReadLocks(eq(OzoneManagerLock.LeveledResource.SNAPSHOT_GC_LOCK), anyList())) + when(ozoneManagerLock.acquireReadLocks(eq(SNAPSHOT_GC_LOCK), anyList())) .thenAnswer(i -> { lockIds.set( (List) i.getArgument(1, List.class).stream().map(val -> UUID.fromString(((String[]) val)[0])) .collect(Collectors.toList())); return OMLockDetails.EMPTY_DETAILS_LOCK_ACQUIRED; }); - when(ozoneManagerLock.releaseReadLocks(eq(OzoneManagerLock.LeveledResource.SNAPSHOT_GC_LOCK), anyList())) + when(ozoneManagerLock.releaseReadLocks(eq(SNAPSHOT_GC_LOCK), anyList())) .thenAnswer(i -> { Assertions.assertEquals(lockIds.get(), i.getArgument(1, List.class).stream().map(val -> UUID.fromString(((String[]) val)[0]))