kevinrr888 commented on code in PR #4524:
URL: https://github.com/apache/accumulo/pull/4524#discussion_r1672826477
##########
core/src/main/java/org/apache/accumulo/core/fate/Fate.java:
##########
@@ -73,9 +75,12 @@ public class Fate<T> {
private final FateStore<T> store;
private final T environment;
private final ScheduledThreadPoolExecutor fatePoolWatcher;
- private final ExecutorService executor;
+ private final ExecutorService transactionExecutor;
+ private final ExecutorService deadResCleanerExecutor;
private static final EnumSet<TStatus> FINISHED_STATES = EnumSet.of(FAILED,
SUCCESSFUL, UNKNOWN);
+ private static boolean userDeadReservationCleanerRunning = false;
+ private static boolean metaDeadReservationCleanerRunning = false;
Review Comment:
> Should these be Atomic? Or is it fine to keep them as-is?
Right now, there is only one Manager which runs, so these do not need to be
Atomic, but the goal is to have multiple managers in the future, so not sure if
this should be changed now or in the future.
Edit for further clarification: Only one thread will be accessing these at a
time (the Manager). The only places these are accessed or used are in the
constructor for Fate, and in Fate.shutdown(), neither of which have multiple
threads executing. This will change when/if more than 1 Manager is running in
the future.
> Can these static variables could be removed? Do these help testing in
someway? In the manager we can assume that only a single instance of each type
is created. If we wanted to guard against multiple instances per type, then a
more general check could be done that is not limited to dead reservation
cleaners.
The purpose of these was to avoid creating more than 1
DeadReservationCleaner running per store type. As I currently have this, the
first Fate object to be created per store type will be running a
DeadReservationCleaner (in the future, this probably should be changed as
running on the first Fate object created may not be the best approach). In the
Manager, only one Fate object will be created per store type, so this isn't a
necessary check now, but isn't the plan in the future to have multiple Fate
instance running per store type, so in that case we'd only ever want 1
DeadReservationCleaner running? Also, in my tests I create more than 1 Fate per
store type, so without a check I'd be running more than 1 here which I don't
really want. What do you have in mind regarding a more general check?
--
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]
