wchevreuil commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r570169987



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, 
TimeUnit unit) {
     return ret;
   }
 
+  /**
+   * Configuration wrapper for {@link Executor}.
+   */
+  public static class ExecutorConfig {
+    // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+    public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+    private int corePoolSize = -1;
+    private int maxPoolSize = -1;
+    private boolean allowCoreThreadTimeout = false;
+    private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+    private String name;
+
+    public int getMaxPoolSize() {
+      Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + 
maxPoolSize);

Review comment:
       Should we check this upfront, in the related setter (or maybe at a non 
default constructor)?

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, 
TimeUnit unit) {
     return ret;
   }
 
+  /**
+   * Configuration wrapper for {@link Executor}.
+   */
+  public static class ExecutorConfig {
+    // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+    public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+    private int corePoolSize = -1;
+    private int maxPoolSize = -1;
+    private boolean allowCoreThreadTimeout = false;
+    private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+    private String name;
+
+    public int getMaxPoolSize() {
+      Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + 
maxPoolSize);
+      return maxPoolSize;
+    }
+
+    public ExecutorConfig setMaxPoolSize(int maxPoolSize) {

Review comment:
       What if the maxPoolSize is set to less than the corePoolSize? Would it 
be worth to check and automatically set to same as corePoolSize? 

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, 
TimeUnit unit) {
     return ret;
   }
 
+  /**
+   * Configuration wrapper for {@link Executor}.
+   */
+  public static class ExecutorConfig {
+    // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+    public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+    private int corePoolSize = -1;
+    private int maxPoolSize = -1;

Review comment:
       Could we define minimal valid defaults for this, since we are enforcing 
checks later? Or maybe require setting these at construction time.




----------------------------------------------------------------
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]


Reply via email to