HBASE-27905 Directly schedule procedures that do not need to acquire …

hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java

@@ -272,6 +272,17 @@ protected boolean waitInitialized(TEnvironment env) {
     return false;
   }
 
+  /**
+   * If the procedure does not require any lock protection during execution, you can declare that
+   * the procedure does not require locks through the following method, which will help speed up the
+   * scheduling of the procedure. It's a little dangerous to override this method, do not change its
+   * behavior unless you know what you are doing. See HBASE-27905 for details.
+   * @return true if procedure requires lock protection during execution, otherwise false
+   */
+  public boolean needLock() {
+    return true;
+  }
+
   /**
    * The user should override this method if they need a lock on an Entity. A lock can be anything,
    * and it is up to the implementor. The Procedure Framework will call this method just before it
@@ -974,6 +985,9 @@ final LockState doAcquireLock(TEnvironment env, ProcedureStore store) {
     if (waitInitialized(env)) {
       return LockState.LOCK_EVENT_WAIT;
     }
+    if (!needLock()) {
+      return LockState.LOCK_ACQUIRED;
+    }
     if (lockedWhenLoading) {
       // reset it so we will not consider it anymore
       lockedWhenLoading = false;
@@ -1000,6 +1014,10 @@ final LockState doAcquireLock(TEnvironment env, ProcedureStore store) {
    * Internal method called by the ProcedureExecutor that starts the user-level code releaseLock().
    */
   final void doReleaseLock(TEnvironment env, ProcedureStore store) {
+    if (!needLock()) {
+      return;
+    }
+
     locked = false;
     // persist that we have released the lock. This must be done before we actually release the
     // lock. Another procedure may take this lock immediately after we release the lock, and if we

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java

@@ -149,19 +149,22 @@ private <T extends Comparable<T>> void doAdd(FairQueue<T> fairq, Queue<T> queue,
     Procedure<?> proc, boolean addFront) {
     queue.add(proc, addFront);
     // For the following conditions, we will put the queue back into execution
-    // 1. The procedure has already held the lock, or the lock has been restored when restarting,
+    // 1. The procedure does not need any lock at all.
+    // 2. The procedure has already held the lock, or the lock has been restored when restarting,
     // which means it can be executed immediately.
-    // 2. The exclusive lock for this queue has not been held.
-    // 3. The given procedure has the exclusive lock permission for this queue.
+    // 3. The exclusive lock for this queue has not been held.
+    // 4. The given procedure has the exclusive lock permission for this queue.
     Supplier<String> reason = null;
-    if (proc.hasLock()) {
+    if (!proc.needLock()) {
+      reason = () -> proc + " does not need any lock";
+    } else if (proc.needLock() && proc.hasLock()) {
       reason = () -> proc + " has lock";
     } else if (proc.isLockedWhenLoading()) {
       reason = () -> proc + " restores lock when restarting";
     } else if (!queue.getLockStatus().hasExclusiveLock()) {
       reason = () -> "the exclusive lock is not held by anyone when adding " + proc;
     } else if (queue.getLockStatus().hasLockAccess(proc)) {
-      reason = () -> proc + " has the excusive lock access";
+      reason = () -> proc + " has the exclusive lock access";
     }
     if (reason != null) {
       addToRunQueue(fairq, queue, reason);
@@ -219,6 +222,9 @@ private <T extends Comparable<T>> Procedure<?> doPoll(final FairQueue<T> fairq)
     // procedures, then we give up and remove the queue from run queue.
     for (int i = 0, n = rq.size(); i < n; i++) {
       Procedure<?> proc = rq.poll();
+      if (!proc.needLock()) {
+        return proc;
+      }
       if (isLockReady(proc, rq)) {
         // the queue is empty, remove from run queue
         if (rq.isEmpty()) {
@@ -229,8 +235,15 @@ private <T extends Comparable<T>> Procedure<?> doPoll(final FairQueue<T> fairq)
       // we are not ready to run, add back and try the next procedure
       rq.add(proc, false);
     }
-    // no procedure is ready for execution, remove from run queue
-    removeFromRunQueue(fairq, rq, () -> "no procedure can be executed");
+    if (hasNoLockNeededProcedure(rq)) {
+      if (LOG.isTraceEnabled()) {
+        LOG.trace("DO NOT remove {} from run queue because there are still procedures that do "
+          + "not need to acquire locks in the queue", rq);
+      }
+    } else {
+      // no procedure is ready for execution, remove from run queue
+      removeFromRunQueue(fairq, rq, () -> "no procedure can be executed");
+    }
     return null;
   }
 
@@ -376,6 +389,19 @@ private static <T extends Comparable<T>> void removeFromRunQueue(FairQueue<T> fa
     }
   }
 
+  private static <T extends Comparable<T>> boolean hasNoLockNeededProcedure(Queue<T> q) {
+    boolean ret = false;
+    // TODO: Iterate Queue in a more efficient way ?
+    for (int i = 0, n = q.size(); i < n; i++) {
+      Procedure<?> proc = q.poll();
+      if (!proc.needLock()) {
+        ret = true;
+      }
+      q.add(proc, false);
+    }
+    return ret;
+  }
+
   // ============================================================================
   // Table Queue Lookup Helpers
   // ============================================================================
@@ -616,8 +642,15 @@ public boolean waitTableExclusiveLock(final Procedure<?> procedure, final TableN
         logLockedResource(LockedResourceType.TABLE, table.getNameAsString());
         return true;
       }
-      removeFromRunQueue(tableRunQueue, getTableQueue(table),
-        () -> procedure + " held the exclusive lock");
+      TableQueue queue = getTableQueue(table);
+      if (hasNoLockNeededProcedure(queue)) {
+        if (LOG.isTraceEnabled()) {
+          LOG.trace("DO NOT remove {} from run queue because there are still procedures that do "
+            + "not need to acquire locks in the queue", queue);
+        }
+      } else {
+        removeFromRunQueue(tableRunQueue, queue, () -> procedure + " held the exclusive lock");
+      }
       return false;
     } finally {
       schedUnlock();
@@ -932,12 +965,18 @@ public boolean waitServerExclusiveLock(final Procedure<?> procedure,
       if (lock.tryExclusiveLock(procedure)) {
         // In tests we may pass procedures other than ServerProcedureInterface, just pass null if
         // so.
-        removeFromRunQueue(serverRunQueue,
-          getServerQueue(serverName,
-            procedure instanceof ServerProcedureInterface
-              ? (ServerProcedureInterface) procedure
-              : null),
-          () -> procedure + " held exclusive lock");
+        ServerQueue queue = getServerQueue(serverName,
+          procedure instanceof ServerProcedureInterface
+            ? (ServerProcedureInterface) procedure
+            : null);
+        if (hasNoLockNeededProcedure(queue)) {
+          if (LOG.isTraceEnabled()) {
+            LOG.trace("DO NOT remove {} from run queue because there are still procedures that do "
+              + "not need to acquire locks in the queue", queue);
+          }
+        } else {
+          removeFromRunQueue(serverRunQueue, queue, () -> procedure + " held exclusive lock");
+        }
         return false;
       }
       waitProcedure(lock, procedure);
@@ -990,8 +1029,15 @@ public boolean waitPeerExclusiveLock(Procedure<?> procedure, String peerId) {
     try {
       final LockAndQueue lock = locking.getPeerLock(peerId);
       if (lock.tryExclusiveLock(procedure)) {
-        removeFromRunQueue(peerRunQueue, getPeerQueue(peerId),
-          () -> procedure + " held exclusive lock");
+        PeerQueue queue = getPeerQueue(peerId);
+        if (hasNoLockNeededProcedure(queue)) {
+          if (LOG.isTraceEnabled()) {
+            LOG.trace("DO NOT remove {} from run queue because there are still procedures that do "
+              + "not need to acquire locks in the queue", queue);
+          }
+        } else {
+          removeFromRunQueue(peerRunQueue, queue, () -> procedure + " held exclusive lock");
+        }
         return false;
       }
       waitProcedure(lock, procedure);
@@ -1040,7 +1086,15 @@ public boolean waitMetaExclusiveLock(Procedure<?> procedure) {
     try {
       final LockAndQueue lock = locking.getMetaLock();
       if (lock.tryExclusiveLock(procedure)) {
-        removeFromRunQueue(metaRunQueue, getMetaQueue(), () -> procedure + " held exclusive lock");
+        MetaQueue queue = getMetaQueue();
+        if (hasNoLockNeededProcedure(queue)) {
+          if (LOG.isTraceEnabled()) {
+            LOG.trace("DO NOT remove {} from run queue because there are still procedures that do "
+              + "not need to acquire locks in the queue", queue);
+          }
+        } else {
+          removeFromRunQueue(metaRunQueue, queue, () -> procedure + " held exclusive lock");
+        }
         return false;
       }
       waitProcedure(lock, procedure);
@@ -1086,8 +1140,15 @@ public boolean waitGlobalExclusiveLock(Procedure<?> procedure, String globalId)
     try {
       final LockAndQueue lock = locking.getGlobalLock(globalId);
       if (lock.tryExclusiveLock(procedure)) {
-        removeFromRunQueue(globalRunQueue, getGlobalQueue(globalId),
-          () -> procedure + " held shared lock");
+        GlobalQueue queue = getGlobalQueue(globalId);
+        if (hasNoLockNeededProcedure(queue)) {
+          if (LOG.isTraceEnabled()) {
+            LOG.trace("DO NOT remove {} from run queue because there are still procedures that do "
+              + "not need to acquire locks in the queue", queue);
+          }
+        } else {
+          removeFromRunQueue(globalRunQueue, queue, () -> procedure + " held shared lock");
+        }
         return false;
       }
       waitProcedure(lock, procedure);

hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java

@@ -1200,4 +1200,28 @@ public void testAcquireSharedLockWhileParentHoldingExclusiveLock() {
     queue.wakeRegion(proc, regionInfo);
     queue.wakeTableExclusiveLock(parentProc, tableName);
   }
+
+  @Test
+  public void testDirectlyScheduleProcedureThatDoesNotNeedLock() {
+    TableName tableName = TableName.valueOf(name.getMethodName());
+    TestTableProcedure xLockNeededProc =
+      new TestTableProcedure(1, tableName, TableOperationType.DELETE);
+    TestTableProcedure noLockNeededProc =
+      new TestTableProcedure(2, tableName, TableOperationType.READ) {
+        @Override
+        public boolean needLock() {
+          return false;
+        }
+      };
+    queue.addBack(xLockNeededProc);
+    queue.addBack(noLockNeededProc);
+
+    assertSame(xLockNeededProc, queue.poll());
+    assertEquals(1, queue.size());
+
+    // now the table exclusive lock has been acquired
+    assertFalse(queue.waitTableExclusiveLock(xLockNeededProc, tableName));
+
+    assertSame(noLockNeededProc, queue.poll());
+  }
 }