hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [hadoop] arp7 commented on a change in pull request #1564: HDDS-2223. Support ReadWrite lock in LockManager.
Date Tue, 01 Oct 2019 21:25:33 GMT
arp7 commented on a change in pull request #1564: HDDS-2223. Support ReadWrite lock in LockManager.
URL: https://github.com/apache/hadoop/pull/1564#discussion_r330283456
 
 

 ##########
 File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/lock/LockManager.java
 ##########
 @@ -25,42 +25,146 @@
 
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Consumer;
 
 /**
  * Manages the locks on a given resource. A new lock is created for each
  * and every unique resource. Uniqueness of resource depends on the
  * {@code equals} implementation of it.
  */
-public class LockManager<T> {
+public class LockManager<R> {
 
   private static final Logger LOG = LoggerFactory.getLogger(LockManager.class);
 
-  private final Map<T, ActiveLock> activeLocks = new ConcurrentHashMap<>();
+  private final Map<R, ActiveLock> activeLocks = new ConcurrentHashMap<>();
   private final GenericObjectPool<ActiveLock> lockPool =
       new GenericObjectPool<>(new PooledLockFactory());
 
   /**
-   * Creates new LockManager instance.
+   * Creates new LockManager instance with the given Configuration.
    *
    * @param conf Configuration object
    */
-  public LockManager(Configuration conf) {
-    int maxPoolSize = conf.getInt(HddsConfigKeys.HDDS_LOCK_MAX_CONCURRENCY,
+  public LockManager(final Configuration conf) {
+    final int maxPoolSize = conf.getInt(HddsConfigKeys.HDDS_LOCK_MAX_CONCURRENCY,
         HddsConfigKeys.HDDS_LOCK_MAX_CONCURRENCY_DEFAULT);
     lockPool.setMaxTotal(maxPoolSize);
   }
 
-
   /**
    * Acquires the lock on given resource.
    *
    * <p>If the lock is not available then the current thread becomes
    * disabled for thread scheduling purposes and lies dormant until the
    * lock has been acquired.
+   *
+   * @param resource on which the lock has to be acquired
+   * @deprecated Use {@link LockManager#writeLock} instead
+   */
+  public void lock(final R resource) {
+   writeLock(resource);
+  }
+
+  /**
+   * Releases the lock on given resource.
+   *
+   * @param resource for which the lock has to be released
+   * @deprecated Use {@link LockManager#writeUnlock} instead
+   */
+  public void unlock(final R resource) {
+   writeUnlock(resource);
+  }
+
+  /**
+   * Acquires the read lock on given resource.
+   *
+   * <p>Acquires the read lock on resource if the write lock is not held by
+   * another thread and returns immediately.
+   *
+   * <p>If the write lock on resource is held by another thread then
+   * the current thread becomes disabled for thread scheduling
+   * purposes and lies dormant until the read lock has been acquired.
+   *
+   * @param resource on which the read lock has to be acquired
+   */
+  public void readLock(final R resource) {
+    acquire(resource, ActiveLock::readLock);
+  }
+
+  /**
+   * Releases the read lock on given resource.
+   *
+   * @param resource for which the read lock has to be released
+   * @throws IllegalMonitorStateException if the current thread does not
+   *                                      hold this lock
+   */
+  public void readUnlock(final R resource) throws IllegalMonitorStateException {
+    release(resource, ActiveLock::readUnlock);
+  }
+
+  /**
+   * Acquires the write lock on given resource.
+   *
+   * <p>Acquires the write lock on resource if neither the read nor write lock
+   * are held by another thread and returns immediately.
+   *
+   * <p>If the current thread already holds the write lock then the
+   * hold count is incremented by one and the method returns
+   * immediately.
+   *
+   * <p>If the lock is held by another thread then the current
+   * thread becomes disabled for thread scheduling purposes and
+   * lies dormant until the write lock has been acquired.
+   *
+   * @param resource on which the lock has to be acquired
    */
-  public void lock(T resource) {
-    activeLocks.compute(resource, (k, v) -> {
-      ActiveLock lock;
+  public void writeLock(final R resource) {
+    acquire(resource, ActiveLock::writeLock);
+  }
+
+  /**
+   * Releases the write lock on given resource.
+   *
+   * @param resource for which the lock has to be released
+   * @throws IllegalMonitorStateException if the current thread does not
+   *                                      hold this lock
+   */
+  public void writeUnlock(final R resource) throws IllegalMonitorStateException {
+    release(resource, ActiveLock::writeUnlock);
+  }
+
+  /**
+   * Acquires the lock on given resource using the provided lock function.
+   *
+   * @param resource on which the lock has to be acquired
+   * @param lockFn function to acquire the lock
+   */
+  private void acquire(final R resource, final Consumer<ActiveLock> lockFn) {
+    lockFn.accept(getLockForLocking(resource));
+  }
+
+  /**
+   * Releases the lock on given resource using the provided release function.
+   *
+   * @param resource for which the lock has to be released
+   * @param releaseFn function to release the lock
+   */
+  private void release(final R resource, final Consumer<ActiveLock> releaseFn) {
+    final ActiveLock lock = getLockForReleasing(resource);
+    releaseFn.accept(lock);
+    decrementActiveLockCount(resource);
+  }
+
+  /**
+   * Returns {@link ActiveLock} instance for the given resource,
+   * on which the lock can be acquired.
+   *
+   * @param resource on which the lock has to be acquired
+   * @return {@link ActiveLock} instance
+   */
+  private ActiveLock getLockForLocking(final R resource) {
+    return activeLocks.compute(resource, (k, v) -> {
 
 Review comment:
   +1 on this change. This compute call gets a global lock, so it may be a potential bottleneck.
Not related to your patch.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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


Mime
View raw message