From d9417738f00a7250d10c13919c9a40871d911592 Mon Sep 17 00:00:00 2001 From: Reid Chan Date: Thu, 1 Mar 2018 15:51:31 +0800 Subject: [PATCH] HBASE-20095 Redesign single instance pool in CleanerChore --- .../org/apache/hadoop/hbase/master/HMaster.java | 5 ++- .../hadoop/hbase/master/cleaner/CleanerChore.java | 48 ++++++++++++---------- .../hbase/master/cleaner/TestCleanerChore.java | 6 +++ .../hbase/master/cleaner/TestHFileCleaner.java | 1 + .../hbase/master/cleaner/TestLogsCleaner.java | 1 + 5 files changed, 38 insertions(+), 23 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 b0dd0b40e9..0205742947 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 @@ -107,6 +107,7 @@ import org.apache.hadoop.hbase.master.balancer.BalancerChore; import org.apache.hadoop.hbase.master.balancer.BaseLoadBalancer; import org.apache.hadoop.hbase.master.balancer.ClusterStatusChore; import org.apache.hadoop.hbase.master.balancer.LoadBalancerFactory; +import org.apache.hadoop.hbase.master.cleaner.CleanerChore; import org.apache.hadoop.hbase.master.cleaner.HFileCleaner; import org.apache.hadoop.hbase.master.cleaner.LogCleaner; import org.apache.hadoop.hbase.master.locking.LockManager; @@ -1144,7 +1145,9 @@ public class HMaster extends HRegionServer implements MasterServices { this.executorService.startExecutorService(ExecutorType.MASTER_TABLE_OPERATIONS, 1); startProcedureExecutor(); - // Start log cleaner thread + // Initial cleaner chore + CleanerChore.initChorePool(conf); + // Start log cleaner thread. int cleanerInterval = conf.getInt("hbase.master.cleaner.interval", 600 * 1000); this.logCleaner = new LogCleaner(cleanerInterval, 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 fdf5141734..ace31c0cb6 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 @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master.cleaner; import org.apache.hadoop.hbase.conf.ConfigurationObserver; import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; +import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; import org.apache.hbase.thirdparty.com.google.common.base.Predicate; import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet; import org.apache.hbase.thirdparty.com.google.common.collect.Iterables; @@ -69,8 +70,8 @@ public abstract class CleanerChore extends Schedu // It may be waste resources for each cleaner chore own its pool, // so let's make pool for all cleaner chores. - private static volatile ForkJoinPool CHOREPOOL; - private static volatile int CHOREPOOLSIZE; + private static volatile ForkJoinPool pool; + private static volatile int poolSize; protected final FileSystem fs; private final Path oldFileDir; @@ -80,6 +81,16 @@ public abstract class CleanerChore extends Schedu private final AtomicBoolean reconfig = new AtomicBoolean(false); protected List cleanersChain; + public static void initChorePool(Configuration conf) { + String size = conf.get(CHORE_POOL_SIZE, DEFAULT_CHORE_POOL_SIZE); + poolSize = calculatePoolSize(size); + // poolSize may be 0 or 0.0 from a careless configuration, + // double check to make sure. + poolSize = poolSize == 0 ? calculatePoolSize(DEFAULT_CHORE_POOL_SIZE) : poolSize; + pool = new ForkJoinPool(poolSize); + LOG.info("Cleaner pool size is {}", poolSize); + } + 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); @@ -98,21 +109,14 @@ public abstract class CleanerChore extends Schedu public CleanerChore(String name, final int sleepPeriod, final Stoppable s, Configuration conf, FileSystem fs, Path oldFileDir, String confKey, Map params) { super(name, s, sleepPeriod); + + Preconditions.checkNotNull(pool, "Chore's pool doesn't initialized, please call" + + "CleanerChore.initChorePool(Configuration) before new a cleaner chore."); this.fs = fs; this.oldFileDir = oldFileDir; this.conf = conf; this.params = params; initCleanerChain(confKey); - - if (CHOREPOOL == null) { - String poolSize = conf.get(CHORE_POOL_SIZE, DEFAULT_CHORE_POOL_SIZE); - CHOREPOOLSIZE = calculatePoolSize(poolSize); - // poolSize may be 0 or 0.0 from a careless configuration, - // double check to make sure. - CHOREPOOLSIZE = CHOREPOOLSIZE == 0? calculatePoolSize(DEFAULT_CHORE_POOL_SIZE): CHOREPOOLSIZE; - this.CHOREPOOL = new ForkJoinPool(CHOREPOOLSIZE); - LOG.info("Cleaner pool size is {}", CHOREPOOLSIZE); - } } /** @@ -174,12 +178,12 @@ public abstract class CleanerChore extends Schedu @Override public void onConfigurationChange(Configuration conf) { int updatedSize = calculatePoolSize(conf.get(CHORE_POOL_SIZE, DEFAULT_CHORE_POOL_SIZE)); - if (updatedSize == CHOREPOOLSIZE) { + if (updatedSize == poolSize) { LOG.trace("Size from configuration is same as previous={}, no need to update.", updatedSize); return; } - CHOREPOOLSIZE = updatedSize; - if (CHOREPOOL.getPoolSize() == 0) { + poolSize = updatedSize; + if (pool.getPoolSize() == 0) { // Chore does not work now, update it directly. updateChorePoolSize(updatedSize); return; @@ -188,10 +192,10 @@ public abstract class CleanerChore extends Schedu reconfig.set(true); } - private void updateChorePoolSize(int updatedSize) { - CHOREPOOL.shutdownNow(); - LOG.info("Update chore's pool size from {} to {}", CHOREPOOL.getParallelism(), updatedSize); - CHOREPOOL = new ForkJoinPool(updatedSize); + private static void updateChorePoolSize(int updatedSize) { + pool.shutdownNow(); + LOG.info("Update chore's pool size from {} to {}", pool.getParallelism(), updatedSize); + pool = new ForkJoinPool(updatedSize); } /** @@ -227,7 +231,7 @@ public abstract class CleanerChore extends Schedu } // After each clean chore, checks if receives reconfigure notification while cleaning if (reconfig.compareAndSet(true, false)) { - updateChorePoolSize(CHOREPOOLSIZE); + updateChorePoolSize(poolSize); } } else { LOG.debug("Cleaner chore disabled! Not cleaning."); @@ -241,7 +245,7 @@ public abstract class CleanerChore extends Schedu public Boolean runCleaner() { preRunCleaner(); CleanerTask task = new CleanerTask(this.oldFileDir, true); - CHOREPOOL.submit(task); + pool.submit(task); return task.join(); } @@ -373,7 +377,7 @@ public abstract class CleanerChore extends Schedu @VisibleForTesting int getChorePoolSize() { - return CHOREPOOLSIZE; + return poolSize; } /** 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 22fa292a21..9438319187 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 @@ -36,6 +36,7 @@ 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.Before; import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -55,6 +56,11 @@ 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 { + CleanerChore.initChorePool(UTIL.getConfiguration()); + } + @After public void cleanup() throws Exception { // delete and recreate the test directory, ensuring a clean test dir between tests diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileCleaner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileCleaner.java index 32480ea1bb..465e1932a9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileCleaner.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileCleaner.java @@ -68,6 +68,7 @@ public class TestHFileCleaner { public static void setupCluster() throws Exception { // have to use a minidfs cluster because the localfs doesn't modify file times correctly UTIL.startMiniDFSCluster(1); + CleanerChore.initChorePool(UTIL.getConfiguration()); } @AfterClass diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestLogsCleaner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestLogsCleaner.java index 7423d26933..0263085aec 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestLogsCleaner.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestLogsCleaner.java @@ -85,6 +85,7 @@ public class TestLogsCleaner { public static void setUpBeforeClass() throws Exception { TEST_UTIL.startMiniZKCluster(); TEST_UTIL.startMiniDFSCluster(1); + CleanerChore.initChorePool(TEST_UTIL.getConfiguration()); } @AfterClass -- 2.15.0