keith-turner commented on code in PR #4535:
URL: https://github.com/apache/accumulo/pull/4535#discussion_r1613900751
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/bulk/BulkImport.java:
##########
@@ -482,12 +484,16 @@ private SortedMap<KeyExtent,Bulk.Files>
computeMappingFromFiles(FileSystem fs, T
if (this.executor != null) {
executor = this.executor;
} else if (numThreads > 0) {
- executor = service =
context.threadPools().getPoolBuilder("BulkImportThread")
- .numCoreThreads(numThreads).build();
+ executor = service =
+ context.threadPools().getPoolBuilder(METRICS_POOL_PREFIX +
"bulk.import.service")
Review Comment:
This is a client side thread pool for bulk load, could use the following name
```suggestion
context.threadPools().getPoolBuilder(METRICS_POOL_PREFIX +
"client.bulk.load")
```
##########
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 this is misnamed, should indicate split in the name and not minor
compaction.
##########
server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java:
##########
@@ -362,8 +364,8 @@ private Map<Path,List<AssignmentInfo>> estimateSizes(final
VolumeManager vm,
final Map<Path,List<AssignmentInfo>> ais = Collections.synchronizedMap(new
TreeMap<>());
- ExecutorService threadPool =
ThreadPools.getServerThreadPools().getPoolBuilder("estimateSizes")
- .numCoreThreads(numThreads).build();
+ ExecutorService threadPool = ThreadPools.getServerThreadPools()
+
.getPoolBuilder("bulk.import.estimate.sizes.pool").numCoreThreads(numThreads).build();
Review Comment:
Should this use the METRICS_POOL_PREFIX?
##########
server/base/src/main/java/org/apache/accumulo/server/conf/store/impl/PropCacheCaffeineImpl.java:
##########
@@ -45,8 +46,8 @@ public class PropCacheCaffeineImpl implements PropCache {
public static final int EXPIRE_MIN = 60;
private static final Logger log =
LoggerFactory.getLogger(PropCacheCaffeineImpl.class);
private static final Executor executor =
-
ThreadPools.getServerThreadPools().getPoolBuilder("caffeine-tasks").numCoreThreads(1)
- .numMaxThreads(20).withTimeOut(60L, SECONDS).build();
+ ThreadPools.getServerThreadPools().getPoolBuilder(METRICS_POOL_PREFIX +
"caffeine.task")
Review Comment:
Would it be useful to include something in the name that indicates the pool
is used for a cache that is related to properties?
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReader.java:
##########
@@ -71,9 +72,9 @@ protected TabletServerBatchReader(ClientContext context,
Class<?> scopeClass, Ta
this.tableName = tableName;
this.numThreads = numQueryThreads;
- queryThreadPool =
- context.threadPools().getPoolBuilder("batch scanner " +
batchReaderInstance + "-")
- .numCoreThreads(numQueryThreads).build();
+ queryThreadPool = context.threadPools()
+ .getPoolBuilder(METRICS_POOL_PREFIX + "tserver.batch.reader.scanner" +
batchReaderInstance)
Review Comment:
Could drop `tserver` in the name as these threads run in client code,
usually outside if tserver. Could use `client` instead to indicate a client
side thread pool.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java:
##########
@@ -335,31 +340,31 @@ public TabletServerResourceManager(ServerContext context,
TabletHostingServer ts
Property.TSERV_ASSIGNMENT_MAXCONCURRENT, enableMetrics);
modifyThreadPoolSizesAtRuntime(
() ->
context.getConfiguration().getCount(Property.TSERV_ASSIGNMENT_MAXCONCURRENT),
- "tablet assignment", assignmentPool);
+ "tserver.tablet.assignment.pool", assignmentPool);
Review Comment:
Why was this change made? Getting lost in the diffs trying to understand
the bigger picture.
--
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]
