keith-turner commented on code in PR #5817:
URL: https://github.com/apache/accumulo/pull/5817#discussion_r2294029178
##########
core/src/main/java/org/apache/accumulo/core/fate/FateExecutor.java:
##########
@@ -105,15 +105,22 @@ 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 var runningTxRunners = getRunningTxRunners();
+ final var runningTxRunnersCopy = getRunningTxRunners();
final int configured = poolConfigs.get(fateOps);
ThreadPools.resizePool(pool, () -> configured, poolName);
- final int needed = configured - runningTxRunners.size();
+ final int needed = configured - runningTxRunnersCopy.size();
if (needed > 0) {
// If the pool grew, then ensure that there is a TransactionRunner for
each thread
for (int i = 0; i < needed; i++) {
try {
- pool.execute(new TransactionRunner());
+ final TransactionRunner tr = new TransactionRunner();
+ synchronized (runningTxRunners) {
+ if (pool.isShutdown() || pool.isTerminating()) {
+ return;
+ }
+ runningTxRunners.add(tr);
+ pool.execute(tr);
+ }
Review Comment:
Seems like it may be good to get 5813 merged and then make this change to
avoid the copy change.
Checking the pool for shutdown here has a race condition like in the
following.
```java
if (pool.isShutdown() || pool.isTerminating()) {
return;
}
runningTxRunners.add(tr);
// pool could be shutdown here and a rejected excecution
exception thrown
pool.execute(tr);
```
So maybe could do something like the following and not check for shutdown
here.
```java
try {
runningTxRunners.add(tr);
pool.execute(tr);
}catch (RejectedExecutionException e){
runningTxRunners.remove(tr);
}
```
--
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
