virajjasani commented on a change in pull request #2196:
URL: https://github.com/apache/hbase/pull/2196#discussion_r467785338
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/util/Threads.java
##########
@@ -197,83 +198,14 @@ public static ThreadPoolExecutor
getBoundedCachedThreadPool(
return boundedCachedThreadPool;
}
- public static ThreadPoolExecutor getBoundedCachedThreadPool(int
maxCachedThread, long timeout,
- TimeUnit unit, String prefix) {
- return getBoundedCachedThreadPool(maxCachedThread, timeout, unit,
- newDaemonThreadFactory(prefix));
- }
-
- /**
- * Returns a {@link java.util.concurrent.ThreadFactory} that names each
created thread uniquely,
- * with a common prefix.
- * @param prefix The prefix of every created Thread's name
- * @return a {@link java.util.concurrent.ThreadFactory} that names threads
- */
- public static ThreadFactory getNamedThreadFactory(final String prefix) {
- SecurityManager s = System.getSecurityManager();
- final ThreadGroup threadGroup = (s != null) ? s.getThreadGroup() :
Thread.currentThread()
- .getThreadGroup();
-
- return new ThreadFactory() {
- final AtomicInteger threadNumber = new AtomicInteger(1);
- private final int poolNumber = Threads.poolNumber.getAndIncrement();
- final ThreadGroup group = threadGroup;
-
- @Override
- public Thread newThread(Runnable r) {
- final String name = prefix + "-pool" + poolNumber + "-t" +
threadNumber.getAndIncrement();
- return new Thread(group, r, name);
- }
- };
- }
-
- /**
- * Same as {#newDaemonThreadFactory(String, UncaughtExceptionHandler)},
- * without setting the exception handler.
- */
- public static ThreadFactory newDaemonThreadFactory(final String prefix) {
- return newDaemonThreadFactory(prefix, null);
- }
-
- /**
- * Get a named {@link ThreadFactory} that just builds daemon threads.
- * @param prefix name prefix for all threads created from the factory
- * @param handler unhandles exception handler to set for all threads
- * @return a thread factory that creates named, daemon threads with
- * the supplied exception handler and normal priority
- */
- public static ThreadFactory newDaemonThreadFactory(final String prefix,
- final UncaughtExceptionHandler handler) {
- final ThreadFactory namedFactory = getNamedThreadFactory(prefix);
- return new ThreadFactory() {
- @Override
- public Thread newThread(Runnable r) {
- Thread t = namedFactory.newThread(r);
- if (handler != null) {
- t.setUncaughtExceptionHandler(handler);
- } else {
Review comment:
I see, you are correct. We should not see issue from end result
viewpoint if we are just executing some tasks and some exception interrupts
executor thread specifically when `ThreadGroup` takes care of it:
```
public void uncaughtException(Thread t, Throwable e) {
if (parent != null) {
parent.uncaughtException(t, e);
} else {
Thread.UncaughtExceptionHandler ueh =
Thread.getDefaultUncaughtExceptionHandler();
if (ueh != null) {
ueh.uncaughtException(t, e);
} else if (!(e instanceof ThreadDeath)) {
System.err.print("Exception in thread \""
+ t.getName() + "\" ");
e.printStackTrace(System.err);
}
}
}
```
However, if we want to maintain the same log present in
`LOGGING_EXCEPTION_HANDLER` as default (which is what used to happen before
this patch) behaviour, we should update the corresponding usages with uncaught
exception handler as `LOGGING_EXCEPTION_HANDLER` in all places.
Let me raise a PR for this.
----------------------------------------------------------------
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]
