diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java index 85d6131b50..5017432ade 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/ChoreService.java @@ -191,6 +191,11 @@ public class ChoreService implements ChoreServicer { ScheduledFuture future = scheduledChores.get(chore); future.cancel(mayInterruptIfRunning); scheduledChores.remove(chore); + try { + chore.cleanup(); + } catch (Throwable t) { + LOG.error(chore + " cleanup failed; ignoring", t); + } // Removing a chore that was missing its start time means it may be possible // to reduce the number of threads diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java index 468b5d30c3..0c19980588 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/ScheduledChore.java @@ -70,6 +70,7 @@ public abstract class ScheduledChore implements Runnable { private long timeOfLastRun = -1; // system time millis private long timeOfThisRun = -1; // system time millis private boolean initialChoreComplete = false; + private boolean cleanupExecuted = false; /** * A means by which a ScheduledChore can be stopped. Once a chore recognizes that it has been @@ -348,10 +349,18 @@ public abstract class ScheduledChore implements Runnable { return true; } + public final synchronized void cleanup() { + if (this.cleanupExecuted) { + return; + } + this.cleanupExecuted = true; + cleanupImpl(); + } + /** - * Override to run cleanup tasks when the Chore encounters an error and must stop running + * Override to implement cleanup tasks when the Chore encounters an error and must stop running */ - protected synchronized void cleanup() { + protected void cleanupImpl() { } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ClusterStatusPublisher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ClusterStatusPublisher.java index af35ce4eb3..4aae01b818 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ClusterStatusPublisher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ClusterStatusPublisher.java @@ -165,7 +165,8 @@ public class ClusterStatusPublisher extends ScheduledChore { } @Override - protected synchronized void cleanup() { + protected void cleanupImpl() { + super.cleanupImpl(); connected = false; publisher.close(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index a5961da1c0..3376ca3d15 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -323,6 +323,8 @@ public class HMaster extends HRegionServer implements MasterServices { MetaLocationSyncer metaLocationSyncer; // Tracker for active master location, if any client ZK quorum specified MasterAddressSyncer masterAddressSyncer; + // ZK watches for the client ZK quorum. + private ZKWatcher clientZkWatcher; // Tracker for split and merge state private SplitOrMergeTracker splitOrMergeTracker; @@ -797,7 +799,7 @@ public class HMaster extends HRegionServer implements MasterServices { if (clientQuorumServers != null && !clientZkObserverMode) { // we need to take care of the ZK information synchronization // if given client ZK are not observer nodes - ZKWatcher clientZkWatcher = new ZKWatcher(conf, + this.clientZkWatcher = new ZKWatcher(conf, getProcessName() + ":" + rpcServices.getSocketAddress().getPort() + "-clientZK", this, false, true); this.metaLocationSyncer = new MetaLocationSyncer(zooKeeper, clientZkWatcher, this); @@ -2975,6 +2977,11 @@ public class HMaster extends HRegionServer implements MasterServices { return zooKeeper; } + @VisibleForTesting + public ZKWatcher getClientZooKeeper() { + return clientZkWatcher; + } + @Override public MasterCoprocessorHost getMasterCoprocessorHost() { return cpHost; @@ -3967,4 +3974,12 @@ public class HMaster extends HRegionServer implements MasterServices { public SyncReplicationReplayWALManager getSyncReplicationReplayWALManager() { return this.syncReplicationReplayWALManager; } + + @Override + protected void shutdownServiceFromRun() { + if (this.clientZkWatcher != null) { + this.clientZkWatcher.close(); + } + super.shutdownServiceFromRun(); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java index 6c5d677a86..f8c5b294a7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MobCompactionChore.java @@ -89,8 +89,8 @@ public class MobCompactionChore extends ScheduledChore { } @Override - protected synchronized void cleanup() { - super.cleanup(); + protected void cleanupImpl() { + super.cleanupImpl(); pool.shutdown(); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java index 19a7a693ef..305881dc78 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/CleanerChore.java @@ -435,7 +435,8 @@ public abstract class CleanerChore extends Schedu } @Override - public synchronized void cleanup() { + public void cleanupImpl() { + super.cleanupImpl(); for (T lc : this.cleanersChain) { try { lc.stop("Exiting"); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java index 7ad6177764..0198f67f3a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileCleaner.java @@ -203,8 +203,8 @@ public class HFileCleaner extends CleanerChore { } @Override - public synchronized void cleanup() { - super.cleanup(); + public void cleanupImpl() { + super.cleanupImpl(); stopHFileDeleteThreads(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/LogCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/LogCleaner.java index a7338c0698..f2409fbbfe 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/LogCleaner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/LogCleaner.java @@ -123,8 +123,8 @@ public class LogCleaner extends CleanerChore { } @Override - public synchronized void cleanup() { - super.cleanup(); + public void cleanupImpl() { + super.cleanupImpl(); interruptOldWALsCleaner(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index e407285f9f..0e65b779ae 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -1041,6 +1041,13 @@ public class HRegionServer extends HasThread implements } } + shutdownServiceFromRun(); + + this.shutDown = true; + LOG.info("Exiting; stopping=" + this.serverName + "; zookeeper connection closed."); + } + + protected void shutdownServiceFromRun() { if (abortRequested) { Timer abortMonitor = new Timer("Abort regionserver monitor", true); TimerTask abortTimeoutTask = null; @@ -1184,8 +1191,6 @@ public class HRegionServer extends HasThread implements if (this.zooKeeper != null) { this.zooKeeper.close(); } - this.shutDown = true; - LOG.info("Exiting; stopping=" + this.serverName + "; zookeeper connection closed."); } private boolean containsMetaTableRegions() { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java index 026010d42a..5cc09aa406 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSeparateClientZKCluster.java @@ -33,6 +33,8 @@ import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.JVMClusterUtil.RegionServerThread; import org.apache.hadoop.hbase.zookeeper.MiniZooKeeperCluster; +import org.apache.hadoop.hbase.zookeeper.ZKWatcher; +import org.apache.zookeeper.ZooKeeper; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -120,6 +122,22 @@ public class TestSeparateClientZKCluster { } } + @Test + public void testZkConnectionClosed() throws Exception { + MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster(); + HMaster master = cluster.getMaster(); + ZKWatcher zk = master.getClientZooKeeper(); + master.stopMaster(); + while (!master.isShutDown()) { + Thread.sleep(200); + } + ZooKeeper.States state = zk.getRecoverableZooKeeper().getState(); + if (state != null && state != ZooKeeper.States.CLOSED + && state != ZooKeeper.States.NOT_CONNECTED) { + Assert.fail("State is " + state); + } + } + @Test public void testMasterSwitch() throws Exception { // get an admin instance and issue some request first