[GitHub] [hbase] wchevreuil commented on a diff in pull request #5267: HBASE-27905 Directly schedule procedures that do not need to acquire …

["wchevreuil (via GitHub)" <gi...@apache.org>]

wchevreuil commented on code in PR #5267:
URL: https://github.com/apache/hbase/pull/5267#discussion_r1223090669


##########
hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java:
##########
@@ -272,6 +272,10 @@ protected boolean waitInitialized(TEnvironment env) {
     return false;
   }
 
+  public boolean needLock() {

Review Comment:
   nit: needsLock



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java:
##########
@@ -368,6 +374,13 @@ private static <T extends Comparable<T>> void addToRunQueue(FairQueue<T> fairq,
 
   private static <T extends Comparable<T>> void removeFromRunQueue(FairQueue<T> fairq,
     Queue<T> queue, Supplier<String> reason) {
+    if (hasNoLockNeededProcedure(queue)) {
+      if (LOG.isTraceEnabled()) {
+        LOG.trace("DO NOT remove {} from run queue because There are still procedures in the "
+          + "queue that do not need to acquire locks", queue);
+      }
+      return;
+    }

Review Comment:
   Mind explain why we do this here? I see `removeFromRunQueue` is called from several places, some apparently expecting the call to effectively remove the queue. 



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