HBASE-17699 Fix TestLockProcedure.

Earlier when queues had locks, clearQueue() also cleaned up old locks when AbstractProcedureScheduler.clear() was called to reset scheduler for testing failure and recovery.
Now with locks decoupled from queues, they need to be separately cleaned up.
We can't have clearLocks() as abstract method in AbstractProcedureScheduler because at that level, a procedure scheduler is just a queue. It's only in MasterProcedureScheduler that locks come into picture. So directly overriding clear() method in MPS.

Earlier when queues had locks, clearQueue() also cleaned up old locks when AbstractProcedureScheduler.clear() was called.
Now with locks decoupled from queues, they need to be separately cleaned up.
We can't have clearLocks() as abstract method in AbstractProcedureScheduler because at that level, a procedure scheduler is just a queue. It's only in MasterProcedureScheduler that locks come into picture. So directly overriding clear() method in MPS.

Change-Id: If1a0acb418a79f98ce6155541edb0c1e621638e3

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

@@ -161,12 +161,6 @@ public abstract class AbstractProcedureScheduler implements ProcedureScheduler {
   // ==========================================================================
   //  Utils
   // ==========================================================================
-  /**
-   * Removes all of the elements from the queue
-   * NOTE: this method is called with the sched lock held.
-   */
-  protected abstract void clearQueue();
-
   /**
    * Returns the number of elements in this queue.
    * NOTE: this method is called with the sched lock held.
@@ -181,17 +175,6 @@ public abstract class AbstractProcedureScheduler implements ProcedureScheduler {
    */
   protected abstract boolean queueHasRunnables();
 
-  @Override
-  public void clear() {
-    // NOTE: USED ONLY FOR TESTING
-    schedLock();
-    try {
-      clearQueue();
-    } finally {
-      schedUnlock();
-    }
-  }
-
   @Override
   public int size() {
     schedLock();

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

@@ -550,6 +550,7 @@ public class ProcedureExecutor<TEnvironment> {
     timeoutExecutor.sendStopSignal();
   }
 
+  @VisibleForTesting
   public void join() {
     assert !isRunning() : "expected not running";
 

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

@@ -130,7 +130,9 @@ public interface ProcedureScheduler {
   int size();
 
   /**
-   * Removes all of the elements from the queue
+   * Clear current state of scheduler such that it is equivalent to newly created scheduler.
+   * Used for testing failure and recovery. To emulate server crash/restart,
+   * {@link ProcedureExecutor} resets its own state and calls clear() on scheduler.
    */
   @VisibleForTesting
   void clear();

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

@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.hbase.procedure2;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.classification.InterfaceStability;
 
@@ -43,9 +44,15 @@ public class SimpleProcedureScheduler extends AbstractProcedureScheduler {
     return runnables.poll();
   }
 
+  @VisibleForTesting
   @Override
-  protected void clearQueue() {
-    runnables.clear();
+  public void clear() {
+    schedLock();
+    try {
+      runnables.clear();
+    } finally {
+      schedUnlock();
+    }
   }
 
   @Override

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

@@ -227,8 +227,19 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler {
     return pollResult;
   }
 
+  @VisibleForTesting
   @Override
-  protected void clearQueue() {
+  public void clear() {
+    schedLock();
+    try {
+      clearQueue();
+      locking.clear();
+    } finally {
+      schedUnlock();
+    }
+  }
+
+  private void clearQueue() {
     // Remove Servers
     for (int i = 0; i < serverBuckets.length; ++i) {
       clear(serverBuckets[i], serverRunQueue, SERVER_QUEUE_KEY_COMPARATOR);
@@ -922,30 +933,42 @@ public class MasterProcedureScheduler extends AbstractProcedureScheduler {
       return lock;
     }
 
-    public LockAndQueue getTableLock(TableName tableName) {
+    LockAndQueue getTableLock(TableName tableName) {
       return getLock(tableLocks, tableName);
     }
 
-    public LockAndQueue removeTableLock(TableName tableName) {
+    LockAndQueue removeTableLock(TableName tableName) {
       return tableLocks.remove(tableName);
     }
 
-    public LockAndQueue getNamespaceLock(String namespace) {
+    LockAndQueue getNamespaceLock(String namespace) {
       return getLock(namespaceLocks, namespace);
     }
 
-    public LockAndQueue getRegionLock(String encodedRegionName) {
+    LockAndQueue getRegionLock(String encodedRegionName) {
       return getLock(regionLocks, encodedRegionName);
     }
 
-    public LockAndQueue removeRegionLock(String encodedRegionName) {
+    LockAndQueue removeRegionLock(String encodedRegionName) {
       return regionLocks.remove(encodedRegionName);
     }
 
-    public LockAndQueue getServerLock(ServerName serverName) {
+    LockAndQueue getServerLock(ServerName serverName) {
       return getLock(serverLocks, serverName);
     }
 
+    /**
+     * Removes all locks by clearing the maps.
+     * Used when procedure executor is stopped for failure and recovery testing.
+     */
+    @VisibleForTesting
+    void clear() {
+      serverLocks.clear();
+      namespaceLocks.clear();
+      tableLocks.clear();
+      regionLocks.clear();
+    }
+
     final Map<ServerName, LockAndQueue> serverLocks = new HashMap<>();
     final Map<String, LockAndQueue> namespaceLocks = new HashMap<>();
     final Map<TableName, LockAndQueue> tableLocks = new HashMap<>();