keith-turner commented on code in PR #5605:
URL: https://github.com/apache/accumulo/pull/5605#discussion_r2116595258
##########
core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java:
##########
@@ -74,7 +74,7 @@ private static void
setContextConfig(Supplier<Map<String,String>> contextConfigS
private static void startCleanupThread(final AccumuloConfiguration conf,
final Supplier<Map<String,String>> contextConfigSupplier) {
ScheduledFuture<?> future =
- ((ScheduledThreadPoolExecutor) ThreadPools.getClientThreadPools((t, e)
-> {
+ ((ScheduledThreadPoolExecutor) ThreadPools.getClientThreadPools(conf,
(t, e) -> {
Review Comment:
This code could call `context.threadPools()` if the context were passed down
instead of a configuration object. Not sure if that is possible though. Was
wondering if this code does things a bit differently than all other code, like
maybe all other code will get the thread pools object from the context?
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -242,11 +242,11 @@ public ClientContext(SingletonReservation reservation,
ClientInfo info,
} else {
// Provide a default UEH that just logs the error
if (ueh == null) {
- clientThreadPools = ThreadPools.getClientThreadPools((t, e) -> {
+ clientThreadPools = ThreadPools.getClientThreadPools(serverConf, (t,
e) -> {
Review Comment:
Wondering if this should call `getConfiguration()` because ServerContext
overrides this method.
```suggestion
clientThreadPools =
ThreadPools.getClientThreadPools(getConfiguration(), (t, e) -> {
```
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -242,11 +242,11 @@ public ClientContext(SingletonReservation reservation,
ClientInfo info,
} else {
// Provide a default UEH that just logs the error
if (ueh == null) {
- clientThreadPools = ThreadPools.getClientThreadPools((t, e) -> {
+ clientThreadPools = ThreadPools.getClientThreadPools(serverConf, (t,
e) -> {
log.error("Caught an Exception in client background thread: {}.
Thread is dead.", t, e);
});
} else {
- clientThreadPools = ThreadPools.getClientThreadPools(ueh);
+ clientThreadPools = ThreadPools.getClientThreadPools(serverConf, ueh);
Review Comment:
Same question here.
```suggestion
clientThreadPools =
ThreadPools.getClientThreadPools(getConfiguration(), ueh);
```
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
