From 1d29d8c004af64891050345bcba324ed0018ae13 Mon Sep 17 00:00:00 2001 From: Matteo Bertozzi Date: Tue, 10 Nov 2015 13:15:25 -0800 Subject: [PATCH] HBASE-14786 TestProcedureAdmin hangs --- .../store/wal/WALProcedureStore.java | 10 +++--- .../MasterProcedureTestingUtility.java | 35 +++++++++++++++---- .../master/procedure/TestProcedureAdmin.java | 13 ++++--- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java index a3115f8577b..f724a3e5bfa 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/store/wal/WALProcedureStore.java +++ b/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()) { - LOG.debug("Remove log: " + log); + if (LOG.isTraceEnabled()) { + LOG.trace("Removing log=" + log); } log.removeFile(); logs.remove(log); - LOG.info("Remove log: " + log); - LOG.info("Removed logs: " + logs); - if (logs.size() == 0) { LOG.error("Expected at least one log"); } + if (LOG.isDebugEnabled()) { + LOG.info("Removed log=" + log + " activeLogs=" + logs); + } assert logs.size() > 0 : "expected at least one log"; } catch (IOException e) { LOG.error("Unable to remove log: " + log, e); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java index 4d5ed56f086..fa57b122011 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java +++ b/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 = - new MasterProcedureTestingUtility.InjectAbortOnLoadListener(procExec); + InjectAbortOnLoadListener abortListener = new 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 = - new MasterProcedureTestingUtility.InjectAbortOnLoadListener(procExec); + InjectAbortOnLoadListener abortListener = new 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 = - new MasterProcedureTestingUtility.InjectAbortOnLoadListener(procExec); + InjectAbortOnLoadListener abortListener = new 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 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 { HTableDescriptor htd = master.getTableDescriptors().get(tableName); @@ -435,13 +447,24 @@ public class MasterProcedureTestingUtility { public static class InjectAbortOnLoadListener implements ProcedureExecutor.ProcedureExecutorListener { private final ProcedureExecutor procExec; + private TreeSet procsToAbort = null; public InjectAbortOnLoadListener(final ProcedureExecutor procExec) { this.procExec = procExec; } + public void addProcId(long procId) { + if (procsToAbort == null) { + procsToAbort = new TreeSet(); + } + procsToAbort.add(procId); + } + @Override public void procedureLoaded(long procId) { + if (procsToAbort != null && !procsToAbort.contains(procId)) { + return; + } procExec.abort(procId); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java index a49c23c9b9a..8097ae5aa9f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestProcedureAdmin.java @@ -84,6 +84,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()); @@ -102,12 +103,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); - ProcedureTestingUtility.restart(procExec); + MasterProcedureTestingUtility.testRestartWithAbort(procExec, procId); ProcedureTestingUtility.waitNoProcedureRunning(procExec); // Validate the disable table procedure was aborted successfully MasterProcedureTestingUtility.validateTableIsEnabled( @@ -128,12 +130,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); - ProcedureTestingUtility.restart(procExec); + MasterProcedureTestingUtility.testRestartWithAbort(procExec, procId); ProcedureTestingUtility.waitNoProcedureRunning(procExec); ProcedureTestingUtility.assertProcNotFailed(procExec, procId); // Validate the delete table procedure was not aborted @@ -194,6 +197,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 listProcedures = procExec.listProcedures(); assertTrue(listProcedures.size() >= 1);