[GitHub] [hbase] apurtell commented on a diff in pull request #4468: HBASE-27064 CME in TestRegionNormalizerWorkQueue

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

apurtell commented on code in PR #4468:
URL: https://github.com/apache/hbase/pull/4468#discussion_r886007490


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorkQueue.java:
##########
@@ -39,62 +39,22 @@
  * {@link BlockingQueue}.</li>
  * <li>Allows a producer to insert an item at the head of the queue, if desired.</li>
  * </ul>
- * Assumes low-frequency and low-parallelism concurrent access, so protects state using a simplistic
- * synchronization strategy.
  */
 @InterfaceAudience.Private
 class RegionNormalizerWorkQueue<E> {
 
   /** Underlying storage structure that gives us the Set behavior and FIFO retrieval policy. */
   private LinkedHashSet<E> delegate;
 
-  // the locking structure used here follows the example found in LinkedBlockingQueue. The
-  // difference is that our locks guard access to `delegate` rather than the head node.
-
-  /** Lock held by take, poll, etc */
-  private final ReentrantLock takeLock;
-
+  /** Lock for puts and takes **/
+  private final ReentrantReadWriteLock lock;
   /** Wait queue for waiting takes */
   private final Condition notEmpty;
 
-  /** Lock held by put, offer, etc */
-  private final ReentrantLock putLock;
-
   RegionNormalizerWorkQueue() {
     delegate = new LinkedHashSet<>();
-    takeLock = new ReentrantLock();
-    notEmpty = takeLock.newCondition();
-    putLock = new ReentrantLock();
-  }
-
-  /**
-   * Signals a waiting take. Called only from put/offer (which do not otherwise ordinarily lock
-   * takeLock.)
-   */
-  private void signalNotEmpty() {
-    final ReentrantLock takeLock = this.takeLock;
-    takeLock.lock();
-    try {
-      notEmpty.signal();
-    } finally {
-      takeLock.unlock();
-    }
-  }
-
-  /**
-   * Locks to prevent both puts and takes.
-   */
-  private void fullyLock() {
-    putLock.lock();
-    takeLock.lock();
-  }
-
-  /**
-   * Unlocks to allow both puts and takes.
-   */
-  private void fullyUnlock() {
-    takeLock.unlock();
-    putLock.unlock();
+    lock = new ReentrantReadWriteLock();
+    notEmpty = lock.writeLock().newCondition();

Review Comment:
   ReentrantReadLock does not support Conditions on the readLock. 



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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org