Apache9 commented on a change in pull request #3712:
URL: https://github.com/apache/hbase/pull/3712#discussion_r724895963
##########
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/master/region/MasterRegionTestBase.java
##########
@@ -48,7 +48,9 @@
protected ChoreService choreService;
- protected DirScanPool cleanerPool;
+ protected DirScanPool archiveCleanerPool;
Review comment:
hfileCleanerPool
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/DirScanPool.java
##########
@@ -39,21 +39,41 @@
private final ThreadPoolExecutor pool;
private int cleanerLatch;
private boolean reconfigNotification;
+ private Type dirScanPoolType;
+ private final String name;
- public DirScanPool(Configuration conf) {
- String poolSize = conf.get(CleanerChore.CHORE_POOL_SIZE,
CleanerChore.DEFAULT_CHORE_POOL_SIZE);
+ private enum Type {
+ LOG_CLEANER(CleanerChore.LOG_CLEANER_CHORE_SIZE,
+ CleanerChore.DEFAULT_LOG_CLEANER_CHORE_POOL_SIZE),
+ HFILE_CLEANER(CleanerChore.CHORE_POOL_SIZE,
CleanerChore.DEFAULT_CHORE_POOL_SIZE);
+
+ private final String cleanerPoolSizeConfigName;
+ private final String cleanerPoolSizeConfigDefault;
+
+ private Type(String cleanerPoolSizeConfigName, String
cleanerPoolSizeConfigDefault) {
+ this.cleanerPoolSizeConfigName = cleanerPoolSizeConfigName;
+ this.cleanerPoolSizeConfigDefault = cleanerPoolSizeConfigDefault;
+ }
+ }
+
+ private DirScanPool(Configuration conf, Type dirScanPoolType) {
+ this.dirScanPoolType = dirScanPoolType;
+ this.name = dirScanPoolType.name();
+ String poolSize = conf.get(dirScanPoolType.cleanerPoolSizeConfigName,
+ dirScanPoolType.cleanerPoolSizeConfigDefault);
size = CleanerChore.calculatePoolSize(poolSize);
// poolSize may be 0 or 0.0 from a careless configuration,
// double check to make sure.
- size = size == 0 ?
CleanerChore.calculatePoolSize(CleanerChore.DEFAULT_CHORE_POOL_SIZE) : size;
- pool = initializePool(size);
- LOG.info("Cleaner pool size is {}", size);
+ size = size == 0 ?
+
CleanerChore.calculatePoolSize(dirScanPoolType.cleanerPoolSizeConfigDefault) :
size;
+ pool = initializePool(size, name);
+ LOG.info("{} Cleaner pool size is {}", name, size);
cleanerLatch = 0;
}
- private static ThreadPoolExecutor initializePool(int size) {
+ private static ThreadPoolExecutor initializePool(int size, String name) {
return Threads.getBoundedCachedThreadPool(size, 1, TimeUnit.MINUTES,
- new
ThreadFactoryBuilder().setNameFormat("dir-scan-pool-%d").setDaemon(true)
+ new ThreadFactoryBuilder().setNameFormat(name +
"dir-scan-pool-%d").setDaemon(true)
Review comment:
Add a '-' between name and the suffix? I think the current
implementation will have thread named `LOG_CLEANERdir-scan-pool-0`.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/DirScanPool.java
##########
@@ -109,11 +131,19 @@ synchronized void tryUpdatePoolSize(long timeout) {
break;
}
}
- LOG.info("Update chore's pool size from {} to {}", pool.getPoolSize(),
size);
+ LOG.info("Update {} chore's pool size from {} to {}", name,
pool.getPoolSize(), size);
pool.setCorePoolSize(size);
}
public int getSize() {
return size;
}
+
+ public static DirScanPool getHFileScanPool(Configuration conf) {
Review comment:
Please align these two methods. We do not have cleaner in name for this
method but have a cleaner in name for the method below.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/DirScanPool.java
##########
@@ -39,21 +39,41 @@
private final ThreadPoolExecutor pool;
private int cleanerLatch;
private boolean reconfigNotification;
+ private Type dirScanPoolType;
+ private final String name;
- public DirScanPool(Configuration conf) {
- String poolSize = conf.get(CleanerChore.CHORE_POOL_SIZE,
CleanerChore.DEFAULT_CHORE_POOL_SIZE);
+ private enum Type {
+ LOG_CLEANER(CleanerChore.LOG_CLEANER_CHORE_SIZE,
+ CleanerChore.DEFAULT_LOG_CLEANER_CHORE_POOL_SIZE),
+ HFILE_CLEANER(CleanerChore.CHORE_POOL_SIZE,
CleanerChore.DEFAULT_CHORE_POOL_SIZE);
+
+ private final String cleanerPoolSizeConfigName;
+ private final String cleanerPoolSizeConfigDefault;
+
+ private Type(String cleanerPoolSizeConfigName, String
cleanerPoolSizeConfigDefault) {
+ this.cleanerPoolSizeConfigName = cleanerPoolSizeConfigName;
+ this.cleanerPoolSizeConfigDefault = cleanerPoolSizeConfigDefault;
+ }
+ }
+
+ private DirScanPool(Configuration conf, Type dirScanPoolType) {
+ this.dirScanPoolType = dirScanPoolType;
+ this.name = dirScanPoolType.name();
Review comment:
Better use toLowerCase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
