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.
   
   > 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? I was premtively handling this case. What do 
you have in mind regarding a more general check? Or should I not be handling 
this case yet?



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