[GitHub] [accumulo] ivakegg commented on a change in pull request #1622: fixes #1621: The ClientPool thread pool allows all core threads to time out

[GitBox <gi...@apache.org>]

ivakegg commented on a change in pull request #1622:
URL: https://github.com/apache/accumulo/pull/1622#discussion_r448386243



##########
File path: core/src/main/java/org/apache/accumulo/core/util/SimpleThreadPool.java
##########
@@ -28,15 +28,16 @@
  */
 public class SimpleThreadPool extends ThreadPoolExecutor {
 
-  public SimpleThreadPool(int max, final String name) {
-    super(max, max, 4L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(),
+  public SimpleThreadPool(int coreAndMax, boolean allowCoreThreadTimeOut, final String name) {
+    super(coreAndMax, coreAndMax, 4L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(),
         new NamingThreadFactory(name));
-    allowCoreThreadTimeOut(true);
+    allowCoreThreadTimeOut(allowCoreThreadTimeOut);
   }
 
-  public SimpleThreadPool(int max, final String name, BlockingQueue<Runnable> queue) {
-    super(max, max, 4L, TimeUnit.SECONDS, queue, new NamingThreadFactory(name));
-    allowCoreThreadTimeOut(true);
+  public SimpleThreadPool(int coreAndMax, boolean allowCoreThreadTimeOut, final String name,
+      BlockingQueue<Runnable> queue) {
+    super(coreAndMax, coreAndMax, 4L, TimeUnit.SECONDS, queue, new NamingThreadFactory(name));

Review comment:
       Your idea is starting to grow on me.  Instead of ALLOW_TIMEOUT properties I will create a THREAD_TIMEOUT properties which default to 0.  I will work that through and update unless I find any issues.




----------------------------------------------------------------
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:
users@infra.apache.org