dlmarion commented on code in PR #4535:
URL: https://github.com/apache/accumulo/pull/4535#discussion_r1621047732


##########
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java:
##########
@@ -306,25 +311,25 @@ public TabletServerResourceManager(ServerContext context, 
TabletHostingServer ts
         Property.TSERV_MINC_MAXCONCURRENT, true);
     modifyThreadPoolSizesAtRuntime(
         () -> 
context.getConfiguration().getCount(Property.TSERV_MINC_MAXCONCURRENT),
-        "minor compactor", minorCompactionThreadPool);
+        METRICS_TSERVER_MINOR_COMPACTOR_POOL, minorCompactionThreadPool);
 
-    splitThreadPool = 
ThreadPools.getServerThreadPools().getPoolBuilder("splitter")
-        .numCoreThreads(0).numMaxThreads(1).withTimeOut(1, SECONDS)
-        .enableThreadPoolMetrics(enableMetrics).build();
+    splitThreadPool = ThreadPools.getServerThreadPools()
+        .getPoolBuilder(METRICS_POOL_PREFIX + 
"tserver.minor.compactor").numCoreThreads(0)

Review Comment:
   I think putting `tserver` in the name is not a good choice. When merged to 
elasticity this will need to change as splits are done in the manager.



##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java:
##########
@@ -672,11 +672,11 @@ private class MutationWriter {
     public MutationWriter(int numSendThreads) {
       serversMutations = new HashMap<>();
       queued = new HashSet<>();
-      sendThreadPool = 
context.threadPools().getPoolBuilder(this.getClass().getName())
+      sendThreadPool = 
context.threadPools().getPoolBuilder("tserver.batch.writer.send.pool")

Review Comment:
   `tserver` might be confusing, could probably just get away with 
`batch.writer...`. Also, probably don't need  `pool` in the name as I think 
it's already being added by the prefix



##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchWriter.java:
##########
@@ -672,11 +672,11 @@ private class MutationWriter {
     public MutationWriter(int numSendThreads) {
       serversMutations = new HashMap<>();
       queued = new HashSet<>();
-      sendThreadPool = 
context.threadPools().getPoolBuilder(this.getClass().getName())
+      sendThreadPool = 
context.threadPools().getPoolBuilder("tserver.batch.writer.send.pool")
           .numCoreThreads(numSendThreads).build();
       locators = new HashMap<>();
-      binningThreadPool = 
context.threadPools().getPoolBuilder("BinMutations").numCoreThreads(1)
-          .withQueue(new SynchronousQueue<>()).build();
+      binningThreadPool = 
context.threadPools().getPoolBuilder("tserver.batch.writer.bin.mutations")

Review Comment:
   `tserver` might be confusing, could probably just get away with 
`batch.writer...`



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to