keith-turner commented on code in PR #5817:
URL: https://github.com/apache/accumulo/pull/5817#discussion_r2298594273
##########
core/src/main/java/org/apache/accumulo/core/fate/Fate.java:
##########
@@ -187,8 +189,8 @@ public void run() {
+ "the FateExecutor.", getFateConfigProp(), fateExecutor);
fateExecutor.initiateShutdown();
} else if (fateExecutor.isShutdown() && fateExecutor.isAlive()) {
- log.debug("{} has been shutdown, but is still actively working
on transactions.",
- fateExecutor);
+ log.debug("{} has been shutdown, but is still actively working
on {} transactions.",
+ fateExecutor, fateExecutor.getNumRunningTxRunners());
Review Comment:
The toString for FateExecutor will include the count of the number of
running thread, so do not need to add it again here.
##########
core/src/main/java/org/apache/accumulo/core/fate/FateExecutor.java:
##########
@@ -107,23 +106,27 @@ public FateExecutor(Fate<T> fate, T environment,
Set<Fate.FateOperation> fateOps
*/
protected void resizeFateExecutor(Map<Set<Fate.FateOperation>,Integer>
poolConfigs,
long idleCheckIntervalMillis) {
- final var pool = transactionExecutor;
final int configured = poolConfigs.get(fateOps);
- ThreadPools.resizePool(pool, () -> configured, poolName);
+ ThreadPools.resizePool(transactionExecutor, () -> configured, poolName);
synchronized (runningTxRunners) {
final int running = runningTxRunners.size();
final int needed = configured - running;
log.trace("resizing pools configured:{} running:{} needed:{}
fateOps:{}", configured, running,
needed, fateOps);
-
if (needed > 0) {
// If the pool grew, then ensure that there is a TransactionRunner for
each thread
for (int i = 0; i < needed; i++) {
+ if (transactionExecutor.isShutdown()) {
+ log.trace("Not adding TransactionRunner, FateExecutor is
shutdown.");
+ break;
+ }
try {
- pool.execute(new TransactionRunner());
+ final TransactionRunner tr = new TransactionRunner();
+ runningTxRunners.add(tr);
+ transactionExecutor.execute(tr);
} catch (RejectedExecutionException e) {
// RejectedExecutionException could be shutting down
- if (pool.isShutdown()) {
+ if (transactionExecutor.isShutdown()) {
Review Comment:
Need to remove the runner when execution rejected because the finally block
in the background thread that removes it will never run.
```suggestion
runningTxRunners.remove(tr);
if (transactionExecutor.isShutdown()) {
```
##########
core/src/main/java/org/apache/accumulo/core/fate/Fate.java:
##########
@@ -207,6 +209,7 @@ public void run() {
synchronized (fateExecutors) {
if (fateExecutors.stream().map(FateExecutor::getFateOps)
.noneMatch(fo -> fo.equals(configFateOps))) {
+ log.debug("Adding FateExecutor for {}", configFateOps);
Review Comment:
Would be useful to also include the store type here prior to the ops. Then
we know if its meta or user fate that is adding a FateExecutor.
--
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
