Apache9 commented on code in PR #7077:
URL: https://github.com/apache/hbase/pull/7077#discussion_r2135828873


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java:
##########
@@ -599,32 +593,47 @@ private void removeGlobalQueue(String globalId) {
   }
 
   private void tryCleanupGlobalQueue(String globalId, Procedure<?> procedure) {
+    tryCleanupQueue(globalId, procedure, () -> globalMap, 
GLOBAL_QUEUE_KEY_COMPARATOR,
+      locking::getGlobalLock, globalRunQueue, this::removeGlobalQueue);
+  }
+
+  private static boolean isGlobalProcedure(Procedure<?> proc) {
+    return proc instanceof GlobalProcedureInterface;
+  }
+
+  private static String getGlobalId(Procedure<?> proc) {
+    return ((GlobalProcedureInterface) proc).getGlobalId();
+  }
+
+  private <T extends Comparable<T>, TNode extends Queue<T>> boolean 
tryCleanupQueue(T id,
+    Procedure<?> proc, Supplier<TNode> getMap, AvlKeyComparator<TNode> 
comparator,
+    Function<T, LockAndQueue> getLock, FairQueue<T> runQueue, Consumer<T> 
removeQueue) {
     schedLock();
     try {
-      GlobalQueue queue = AvlTree.get(globalMap, globalId, 
GLOBAL_QUEUE_KEY_COMPARATOR);
+      Queue<T> queue = AvlTree.get(getMap.get(), id, comparator);
       if (queue == null) {
-        return;
+        return true;
       }
 
-      final LockAndQueue lock = locking.getGlobalLock(globalId);
-      if (queue.isEmpty() && lock.tryExclusiveLock(procedure)) {
-        removeFromRunQueue(globalRunQueue, queue,
-          () -> "clean up global queue after " + procedure + " completed");
-        removeGlobalQueue(globalId);
+      LockAndQueue lock = getLock.apply(id);
+      if (queue.isEmpty() && lock.isWaitingQueueEmpty() && !lock.isLocked()) {
+        // 1. the queue is empty
+        // 2. no procedure is in the lock's waiting queue
+        // 3. no other one holds the lock. It is possible that someone else 
holds the lock, usually
+        // our parent procedures
+        // If we can meet all the above conditions, it is safe for us to 
remove the queue
+        removeFromRunQueue(runQueue, queue, () -> "clean up queue after " + 
proc + " completed");
+        removeQueue.accept(id);
+        return true;

Review Comment:
   All the operation is protected by the scheLock, so this is not a problem.



-- 
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-unsubscr...@hbase.apache.org

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

Reply via email to