bharathv commented on a change in pull request #2909:
URL: https://github.com/apache/hbase/pull/2909#discussion_r567465600
##########
File path:
hbase-client/src/main/java/org/apache/hadoop/hbase/executor/EventType.java
##########
@@ -149,7 +149,7 @@
* C_M_MERGE_REGION<br>
* Client asking Master to merge regions.
*/
- C_M_MERGE_REGION (30, ExecutorType.MASTER_TABLE_OPERATIONS),
+ C_M_MERGE_REGION (30, ExecutorType.MASTER_MERGE_OPERATIONS),
Review comment:
@saintstack It looks like upping the thread-pool backed by
MASTER_TABLE_OPERATIONS has other implications, please see the following
comment in HMaster. Seems critical that we only have one one thread there,
haven't dug deep into it (the comment is decade old, not sure if it still
applies).
```
// We depend on there being only one instance of this executor running
// at a time. To do concurrency, would need fencing of enable/disable of
// tables.
// Any time changing this maxThreads to > 1, pls see the comment at
// AccessController#postCompletedCreateTableAction
this.executorService.startExecutorService(ExecutorType.MASTER_TABLE_OPERATIONS,
1);
```
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1269,6 +1269,9 @@ private void startServiceThreads() throws IOException {
HConstants.MASTER_LOG_REPLAY_OPS_THREADS,
HConstants.MASTER_LOG_REPLAY_OPS_THREADS_DEFAULT));
this.service.startExecutorService(ExecutorType.MASTER_SNAPSHOT_OPERATIONS,
conf.getInt(
SnapshotManager.SNAPSHOT_POOL_THREADS_KEY,
SnapshotManager.SNAPSHOT_POOL_THREADS_DEFAULT));
+ this.service.startExecutorService(ExecutorType.MASTER_MERGE_OPERATIONS,
conf.getInt(
Review comment:
Normally it does, but it looks like we are using corePoolSize =
maxThreads [1]. Per
[documentation](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ThreadPoolExecutor.html),
eviction of unused threads (see keepAliveTime doc) only happens if
corePoolSize < maxThreads. It looks like we can force core threads too time out
too with
[allowCoreThreadTimeout](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ThreadPoolExecutor.html#allowCoreThreadTimeOut(boolean))
Mind if I submit a follow up patch that makes these configurable along with
the appropriate defaults that apply to all executors?
[1]
https://github.com/apache/hbase/blame/master/hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java#L203
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
