HBASE-14786 TestProcedureAdmin hangs

hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java

@@ -813,14 +813,14 @@ public class WALProcedureStore extends ProcedureStoreBase {
 
   private boolean removeLogFile(final ProcedureWALFile log) {
     try {
-      if (LOG.isDebugEnabled()) {
+      if (LOG.isTraceEnabled()) {
-        LOG.debug("Remove log: " + log);
+        LOG.trace("Removing log=" + log);
       }
       log.removeFile();
       logs.remove(log);
-      LOG.info("Remove log: " + log);
+      if (LOG.isDebugEnabled()) {
-      LOG.info("Removed logs: " + logs);
+        LOG.info("Removed log=" + log + " activeLogs=" + logs);
-      if (logs.size() == 0) { LOG.error("Expected at least one log"); }
+      }
       assert logs.size() > 0 : "expected at least one log";
     } catch (IOException e) {
       LOG.error("Unable to remove log: " + log, e);

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

@@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master.procedure;
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.List;
+import java.util.TreeSet;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -270,8 +271,7 @@ public class MasterProcedureTestingUtility {
     //   rollback step N - kill before store update
     //   restart executor/store
     //   rollback step N - save on store
-    MasterProcedureTestingUtility.InjectAbortOnLoadListener abortListener =
+    InjectAbortOnLoadListener abortListener = new InjectAbortOnLoadListener(procExec);
-      new MasterProcedureTestingUtility.InjectAbortOnLoadListener(procExec);
     procExec.registerListener(abortListener);
     try {
       for (int i = lastStep + 1; i >= 0; --i) {
@@ -305,8 +305,7 @@ public class MasterProcedureTestingUtility {
 
     // try to inject the abort
     ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false);
-    MasterProcedureTestingUtility.InjectAbortOnLoadListener abortListener =
+    InjectAbortOnLoadListener abortListener = new InjectAbortOnLoadListener(procExec);
-      new MasterProcedureTestingUtility.InjectAbortOnLoadListener(procExec);
     procExec.registerListener(abortListener);
     try {
       ProcedureTestingUtility.assertProcNotYetCompleted(procExec, procId);
@@ -339,8 +338,7 @@ public class MasterProcedureTestingUtility {
 
     // execute the rollback
     ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false);
-    MasterProcedureTestingUtility.InjectAbortOnLoadListener abortListener =
+    InjectAbortOnLoadListener abortListener = new InjectAbortOnLoadListener(procExec);
-      new MasterProcedureTestingUtility.InjectAbortOnLoadListener(procExec);
     procExec.registerListener(abortListener);
     try {
       ProcedureTestingUtility.assertProcNotYetCompleted(procExec, procId);
@@ -354,6 +352,20 @@ public class MasterProcedureTestingUtility {
     ProcedureTestingUtility.assertIsAbortException(procExec.getResult(procId));
   }
 
+  public static void testRestartWithAbort(ProcedureExecutor<MasterProcedureEnv> procExec,
+      long procId) throws Exception {
+    InjectAbortOnLoadListener abortListener = new InjectAbortOnLoadListener(procExec);
+    abortListener.addProcId(procId);
+    procExec.registerListener(abortListener);
+    try {
+      ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false);
+      ProcedureTestingUtility.restart(procExec);
+      ProcedureTestingUtility.waitProcedure(procExec, procId);
+    } finally {
+      assertTrue(procExec.unregisterListener(abortListener));
+    }
+  }
+
   public static void validateColumnFamilyAddition(final HMaster master, final TableName tableName,
       final String family) throws IOException {
     TableDescriptor htd = master.getTableDescriptors().getDescriptor(tableName);
@@ -436,13 +448,24 @@ public class MasterProcedureTestingUtility {
   public static class InjectAbortOnLoadListener
       implements ProcedureExecutor.ProcedureExecutorListener {
     private final ProcedureExecutor<MasterProcedureEnv> procExec;
+    private TreeSet<Long> procsToAbort = null;
 
     public InjectAbortOnLoadListener(final ProcedureExecutor<MasterProcedureEnv> procExec) {
       this.procExec = procExec;
     }
 
+    public void addProcId(long procId) {
+      if (procsToAbort == null) {
+        procsToAbort = new TreeSet<Long>();
+      }
+      procsToAbort.add(procId);
+    }
+
     @Override
     public void procedureLoaded(long procId) {
+      if (procsToAbort != null && !procsToAbort.contains(procId)) {
+        return;
+      }
       procExec.abort(procId);
     }
 

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

@@ -85,6 +85,7 @@ public class TestProcedureAdmin {
 
   @After
   public void tearDown() throws Exception {
+    assertTrue("expected executor to be running", getMasterProcedureExecutor().isRunning());
     ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(getMasterProcedureExecutor(), false);
     for (HTableDescriptor htd: UTIL.getHBaseAdmin().listTables()) {
       LOG.info("Tear down, remove table=" + htd.getTableName());
@@ -103,12 +104,13 @@ public class TestProcedureAdmin {
     // Submit an abortable procedure
     long procId = procExec.submitProcedure(
         new DisableTableProcedure(procExec.getEnvironment(), tableName, false), nonceGroup, nonce);
+    // Wait for one step to complete
+    ProcedureTestingUtility.waitProcedure(procExec, procId);
 
     boolean abortResult = procExec.abort(procId, true);
     assertTrue(abortResult);
 
-    ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false);
+    MasterProcedureTestingUtility.testRestartWithAbort(procExec, procId);
-    ProcedureTestingUtility.restart(procExec);
     ProcedureTestingUtility.waitNoProcedureRunning(procExec);
     // Validate the disable table procedure was aborted successfully
     MasterProcedureTestingUtility.validateTableIsEnabled(
@@ -129,12 +131,13 @@ public class TestProcedureAdmin {
     // Submit an un-abortable procedure
     long procId = procExec.submitProcedure(
         new DeleteTableProcedure(procExec.getEnvironment(), tableName), nonceGroup, nonce);
+    // Wait for one step to complete
+    ProcedureTestingUtility.waitProcedure(procExec, procId);
 
     boolean abortResult = procExec.abort(procId, true);
     assertFalse(abortResult);
 
-    ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false);
+    MasterProcedureTestingUtility.testRestartWithAbort(procExec, procId);
-    ProcedureTestingUtility.restart(procExec);
     ProcedureTestingUtility.waitNoProcedureRunning(procExec);
     ProcedureTestingUtility.assertProcNotFailed(procExec, procId);
     // Validate the delete table procedure was not aborted
@@ -195,6 +198,8 @@ public class TestProcedureAdmin {
 
     long procId = procExec.submitProcedure(
       new DisableTableProcedure(procExec.getEnvironment(), tableName, false), nonceGroup, nonce);
+    // Wait for one step to complete
+    ProcedureTestingUtility.waitProcedure(procExec, procId);
 
     List<ProcedureInfo> listProcedures = procExec.listProcedures();
     assertTrue(listProcedures.size() >= 1);