Repository: hbase Updated Branches: refs/heads/branch-2.1 5309c3389 -> e71c05707
HBASE-21384 Procedure with holdlock=false should not be restored lock when restarts Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e71c0570 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e71c0570 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e71c0570 Branch: refs/heads/branch-2.1 Commit: e71c05707e0717a1f1487b648e830fe7fd90d2eb Parents: 5309c33 Author: Allan Yang <[email protected]> Authored: Thu Oct 25 13:58:50 2018 +0800 Committer: Allan Yang <[email protected]> Committed: Thu Oct 25 13:58:50 2018 +0800 ---------------------------------------------------------------------- .../apache/hadoop/hbase/procedure2/LockAndQueue.java | 3 ++- .../apache/hadoop/hbase/procedure2/Procedure.java | 8 +++++++- .../hadoop/hbase/procedure2/ProcedureExecutor.java | 15 ++++++++++++++- 3 files changed, 23 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/e71c0570/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/LockAndQueue.java ---------------------------------------------------------------------- diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/LockAndQueue.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/LockAndQueue.java index ae8daa2..4365a2c 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/LockAndQueue.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/LockAndQueue.java @@ -73,7 +73,8 @@ public class LockAndQueue implements LockStatus { @Override public boolean hasParentLock(Procedure<?> proc) { - // TODO: need to check all the ancestors + // TODO: need to check all the ancestors. need to passed in the procedures + // to find the ancestors. return proc.hasParent() && (isLockOwner(proc.getParentProcId()) || isLockOwner(proc.getRootProcId())); } http://git-wip-us.apache.org/repos/asf/hbase/blob/e71c0570/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java ---------------------------------------------------------------------- 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 472a0d1..74082bf 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 @@ -992,7 +992,13 @@ public abstract class Procedure<TEnvironment> implements Comparable<Procedure<TE LOG.debug("{} is already bypassed, skip acquiring lock.", this); return; } - + // this can happen if the parent stores the sub procedures but before it can + // release its lock, the master restarts + if (getState() == ProcedureState.WAITING && !holdLock(env)) { + LOG.debug("{} is in WAITING STATE, and holdLock= false, skip acquiring lock.", this); + lockedWhenLoading = false; + return; + } LOG.debug("{} held the lock before restarting, call acquireLock to restore it.", this); LockState state = acquireLock(env); assert state == LockState.LOCK_ACQUIRED; http://git-wip-us.apache.org/repos/asf/hbase/blob/e71c0570/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java ---------------------------------------------------------------------- diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java index c6c34df..ede2c90 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java @@ -653,7 +653,20 @@ public class ProcedureExecutor<TEnvironment> { sendProcedureLoadedNotification(p.getProcId()); } // If the procedure holds the lock, put the procedure in front - if (p.isLockedWhenLoading()) { + // If its parent holds the lock, put the procedure in front + // TODO. Is that possible that its ancestor holds the lock? + // For now, the deepest procedure hierarchy is: + // ModifyTableProcedure -> ReopenTableProcedure -> + // MoveTableProcedure -> Unassign/AssignProcedure + // But ModifyTableProcedure and ReopenTableProcedure won't hold the lock + // So, check parent lock is enough(a tricky case is resovled by HBASE-21384). + // If some one change or add new procedures making 'grandpa' procedure + // holds the lock, but parent procedure don't hold the lock, there will + // be a problem here. We have to check one procedure's ancestors. + // And we need to change LockAndQueue.hasParentLock(Procedure<?> proc) method + // to check all ancestors too. + if (p.isLockedWhenLoading() || (p.hasParent() && procedures + .get(p.getParentProcId()).isLockedWhenLoading())) { scheduler.addFront(p, false); } else { // if it was not, it can wait.
