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