From 4b0555503fa0cc90fa8f434c191b87b6b0627ed6 Mon Sep 17 00:00:00 2001 From: Reid Chan Date: Sun, 17 Jun 2018 01:17:39 +0800 Subject: [PATCH] HBASE-20732 Shutdown scan pool when master is stopped --- .../org/apache/hadoop/hbase/master/HMaster.java | 1 + .../hadoop/hbase/master/cleaner/CleanerChore.java | 13 +++++- .../hbase/master/cleaner/TestCleanerChore.java | 46 +++++++++++++++++++--- 3 files changed, 53 insertions(+), 7 deletions(-) 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 883bb4ff52..2d8e530f0a 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 @@ -1240,6 +1240,7 @@ public class HMaster extends HRegionServer implements MasterServices { } super.stopServiceThreads(); stopChores(); + CleanerChore.shutDownChorePool(); LOG.debug("Stopping service threads"); 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 bcb23ac29e..c4e1e379e3 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 @@ -118,7 +118,7 @@ public abstract class CleanerChore extends Schedu break; } } - pool.shutdownNow(); + shutDownNow(); LOG.info("Update chore's pool size from {} to {}", pool.getParallelism(), size); pool = new ForkJoinPool(size); } @@ -136,6 +136,13 @@ public abstract class CleanerChore extends Schedu synchronized void submit(ForkJoinTask task) { pool.submit(task); } + + synchronized void shutDownNow() { + if (pool == null || pool.isShutdown()) { + return; + } + pool.shutdownNow(); + } } // It may be waste resources for each cleaner chore own its pool, // so let's make pool for all cleaner chores. @@ -154,6 +161,10 @@ public abstract class CleanerChore extends Schedu } } + public static void shutDownChorePool() { + POOL.shutDownNow(); + } + public CleanerChore(String name, final int sleepPeriod, final Stoppable s, Configuration conf, FileSystem fs, Path oldFileDir, String confKey) { this(name, sleepPeriod, s, conf, fs, oldFileDir, confKey, null); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java index 6ce941f029..8ba747a1c0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestCleanerChore.java @@ -38,7 +38,9 @@ import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.StoppableImplementation; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -58,15 +60,16 @@ public class TestCleanerChore { private static final Logger LOG = LoggerFactory.getLogger(TestCleanerChore.class); private static final HBaseTestingUtility UTIL = new HBaseTestingUtility(); - @Before - public void setup() throws Exception { + @BeforeClass + public static void setup() { CleanerChore.initChorePool(UTIL.getConfiguration()); } - @After - public void cleanup() throws Exception { + @AfterClass + public static void cleanup() throws Exception { // delete and recreate the test directory, ensuring a clean test dir between tests UTIL.cleanupTestDir(); + CleanerChore.shutDownChorePool(); } @Test @@ -94,6 +97,9 @@ public class TestCleanerChore { // verify all the files were preserved assertTrue("File shouldn't have been deleted", fs.exists(file)); assertTrue("directory shouldn't have been deleted", fs.exists(parent)); + + // cancel chore + chore.cancel(); } @Test @@ -140,6 +146,9 @@ public class TestCleanerChore { assertFalse("directory should have been destroyed.", fs.exists(child)); // and verify that it accurately reported success. assertTrue("chore should claim it succeeded.", result); + + // cancel chore + chore.cancel(); } @Test @@ -176,6 +185,9 @@ public class TestCleanerChore { assertFalse("File didn't get deleted", fs.exists(file)); assertFalse("Empty directory didn't get deleted", fs.exists(child)); assertFalse("Empty directory didn't get deleted", fs.exists(parent)); + + // cancel chore + chore.cancel(); } /** @@ -212,6 +224,9 @@ public class TestCleanerChore { // make sure we never checked the directory Mockito.verify(spy, Mockito.never()).isFileDeletable(fStat); Mockito.reset(spy); + + // cancel chore + chore.cancel(); } @Test @@ -238,6 +253,9 @@ public class TestCleanerChore { // test that the file still exists assertTrue("File got deleted while chore was stopped", fs.exists(topFile)); + + // cancel chore + chore.cancel(); } /** @@ -288,6 +306,9 @@ public class TestCleanerChore { assertFalse("Original file unexpectedly retained", fs.exists(file)); Mockito.verify(spy, Mockito.times(1)).isFileDeletable(Mockito.any()); Mockito.reset(spy); + + // cancel chore + chore.cancel(); } /** @@ -345,6 +366,9 @@ public class TestCleanerChore { assertTrue("Parent directory deleted unexpectedly", fs.exists(parent)); assertFalse("Original file unexpectedly retained", fs.exists(file)); Mockito.verify(spy, Mockito.times(1)).isFileDeletable(Mockito.any()); + + // cancel chore + chore.cancel(); } @Test @@ -378,6 +402,9 @@ public class TestCleanerChore { assertFalse("File didn't get deleted", fs.exists(file)); assertFalse("Empty directory didn't get deleted", fs.exists(child)); assertFalse("Empty directory didn't get deleted", fs.exists(parent)); + + // cancel chore + chore.cancel(); } @Test @@ -411,6 +438,9 @@ public class TestCleanerChore { assertTrue("File got deleted with cleaner disabled", fs.exists(file)); assertTrue("Directory got deleted", fs.exists(child)); assertTrue("Directory got deleted", fs.exists(parent)); + + // cancel chore + chore.cancel(); } @Test @@ -452,8 +482,9 @@ public class TestCleanerChore { conf.set(CleanerChore.CHORE_POOL_SIZE, String.valueOf(changedPoolSize)); chore.onConfigurationChange(conf); assertEquals(changedPoolSize, chore.getChorePoolSize()); - // Stop chore - t.join(); + + // cancel chore + chore.cancel(); } @Test @@ -473,6 +504,9 @@ public class TestCleanerChore { assertEquals(numProcs, chore.calculatePoolSize(Integer.toString(numProcs + 2))); // Force us into the branch that is multiplying 0.0 against the number of processors assertEquals(1, chore.calculatePoolSize("0.0")); + + // cancel chore + chore.cancel(); } private void createFiles(FileSystem fs, Path parentDir, int numOfFiles) throws IOException { -- 2.15.0