diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java index 43adba2bc21a..04975b72b3ae 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java +++ b/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 diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java index fbf0eb8abf32..e3008b5c0e0a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureScheduler.java @@ -149,19 +149,22 @@ private > void doAdd(FairQueue fairq, Queue 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 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 > Procedure doPoll(final FairQueue 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 > Procedure doPoll(final FairQueue 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 > void removeFromRunQueue(FairQueue fa } } + private static > boolean hasNoLockNeededProcedure(Queue 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); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java index 0cf34126a945..f9ebb9504b4f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java +++ b/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()); + } }