Fixing TestRemoteTable and TestHRegionServerFileSystemFailure broken by D336813
Summary:
In D336813 I broke two unit tests, which was hard to notice because they failed
with time-outs. TestHRegionServerFileSystemFailure re-starts a master in the end
if it had crashed so that regionservers can shut down properly, but the
condition it uses to identify whether the master is still alive was no longer
valid, because there is now a thread object wrapping HMaster, and
master.isAlive() is meaningless. I replaced this with master.isClosed() and
removed hard-coded master class from the unit test.
TestRemoteTable started failing because of a bug in JVMClusterUtil.shutdown()
-- it did not call join() on master threads.
Also calling Thread.currentThread().interrupt() when handling
InterruptedException instead of simply ignoring it. This is recommended by Java
Concurrency in Practice as well as in other articles online as the proper way to
handle InterruptedException, but I must admit I don't fully understand why.
Test Plan: All 621 unit tests pass (compared to 614 without the patch).
Reviewers: pritam, kranganathan, pkhemani
Reviewed By: pritam
CC: hbase-eng@lists, pkhemani, pritam, mbautin
Differential Revision: 340934
Revert Plan: OK
git-svn-id: https://svn.apache.org/repos/asf/hbase/branches/0.89@1182040 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java b/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java
index ffacfbd..843dafc 100644
--- a/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java
+++ b/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java
@@ -210,17 +210,38 @@
}
}
}
- // regionServerThreads can never be null because they are initialized when
- // the class is constructed.
- for(Thread t: regionservers) {
- if (t.isAlive()) {
- try {
- t.join();
- } catch (InterruptedException e) {
- // continue
+
+ boolean interrupted = false;
+ try {
+ // regionServerThreads can never be null because they are initialized when
+ // the class is constructed.
+ for (Thread t : regionservers) {
+ if (t.isAlive()) {
+ try {
+ t.join();
+ } catch (InterruptedException e) {
+ interrupted = true;
+ }
}
}
+
+ if (masters != null) {
+ for (JVMClusterUtil.MasterThread t : masters) {
+ while (t.isAlive()) {
+ try {
+ t.join();
+ } catch (InterruptedException e) {
+ interrupted = true;
+ }
+ }
+ }
+ }
+ } finally {
+ if (interrupted) {
+ Thread.currentThread().interrupt();
+ }
}
+
LOG.info("Shutdown of " +
((masters != null) ? masters.size() : "0") + " master(s) and " +
((regionservers != null) ? regionservers.size() : "0") +
diff --git a/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java b/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
index f3e0403..442e185 100644
--- a/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
+++ b/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
@@ -288,6 +288,14 @@
}
/**
+ * Adds a new master to the cluster and starts the master thread. Useful if
+ * the existing master dies and a live master is needed for cleanup.
+ */
+ public void startNewMaster() throws IOException {
+ hbaseCluster.addMaster().start();
+ }
+
+ /**
* Cause a region server to exit doing basic clean up only on its way out.
* @param serverNumber Used as index into a list.
*/
diff --git a/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerFileSystemFailure.java b/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerFileSystemFailure.java
index f7449e4..a96e507 100644
--- a/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerFileSystemFailure.java
+++ b/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerFileSystemFailure.java
@@ -96,9 +96,9 @@
// Bring namenode, hbasemaster back up so we cleanup properly.
TEST_UTIL.getDFSCluster().restartNameNode();
HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
- if (!master.isAlive()) {
- master = HMaster.constructMaster(MiniHBaseCluster.MiniHBaseClusterMaster.class, conf);
- master.start();
+ if (master.isClosed()) {
+ // Master died, we need to re-start it.
+ TEST_UTIL.getMiniHBaseCluster().startNewMaster();
}
}
}
