kevinrr888 commented on code in PR #5263:
URL: https://github.com/apache/accumulo/pull/5263#discussion_r1920460779


##########
core/src/main/java/org/apache/accumulo/core/fate/Fate.java:
##########
@@ -397,24 +443,27 @@ public Fate(T environment, FateStore<T> store, boolean 
runDeadResCleaner,
       Function<Repo<T>,String> toLogStrFunc, AccumuloConfiguration conf) {
     this.store = FateLogger.wrap(store, toLogStrFunc, false);
     this.environment = environment;
-    final ThreadPoolExecutor pool = 
ThreadPools.getServerThreadPools().createExecutorService(conf,
+    this.transactionExecutor = 
ThreadPools.getServerThreadPools().createExecutorService(conf,
         Property.MANAGER_FATE_THREADPOOL_SIZE, true);
     this.workQueue = new LinkedTransferQueue<>();
+    this.runningTxRunners = new ArrayList<>();
     this.fatePoolWatcher =
         
ThreadPools.getServerThreadPools().createGeneralScheduledExecutorService(conf);
     
ThreadPools.watchCriticalScheduledTask(fatePoolWatcher.scheduleWithFixedDelay(()
 -> {
       // resize the pool if the property changed
-      ThreadPools.resizePool(pool, conf, 
Property.MANAGER_FATE_THREADPOOL_SIZE);
-      // If the pool grew, then ensure that there is a TransactionRunner for 
each thread
+      ThreadPools.resizePool(transactionExecutor, conf, 
Property.MANAGER_FATE_THREADPOOL_SIZE);
       final int configured = 
conf.getCount(Property.MANAGER_FATE_THREADPOOL_SIZE);
-      final int needed = configured - pool.getActiveCount();
+      final int needed = configured - transactionExecutor.getActiveCount();

Review Comment:
   Yes good idea.
   Re the cached thread pool: this may be a good solution (I'm not sure about 
the implications of this yet), however would require quite a bit of 
refactoring, and wasn't the original intention of this PR. If that is a route 
we want to take, we could close this PR. This would also mean 
https://github.com/apache/accumulo/issues/5130 changes would be different. I 
assume each set of fate ops may have their own cached thread pool. Or this may 
make 5130 completely obsolete: all fate ops would just share one cached thread 
pool.



-- 
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