cshannon commented on code in PR #4133:
URL: https://github.com/apache/accumulo/pull/4133#discussion_r1450445005


##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -1068,17 +1073,12 @@ boolean canSuspendTablets() {
     }
 
     try {
-      final AgeOffStore<Manager> store = new AgeOffStore<>(
-          new org.apache.accumulo.core.fate.ZooStore<>(getZooKeeperRoot() + 
Constants.ZFATE,
-              context.getZooReaderWriter()),
-          HOURS.toMillis(8), System::currentTimeMillis);
+      initializeFateInstance(context, FateInstanceType.META,
+          new ZooStore<>(getZooKeeperRoot() + Constants.ZFATE, 
context.getZooReaderWriter()));
+      initializeFateInstance(context, FateInstanceType.USER,
+          new AccumuloStore<>(context, FateTable.NAME));

Review Comment:
   @keith-turner - I went ahead and decided to do this but used an 
`AtomicReference` instead of volatile. I went with that approach because I 
could make that reference final still and then ensure things are expected by 
adding so state checks when 
[initializing](https://github.com/apache/accumulo/pull/4133/files#diff-07984c23da1745b22dd4fde5c4b8877dfb190ea19e5e60ab79d8a93e752031c7R1083)
 and also 
[retrieving](https://github.com/apache/accumulo/pull/4133/files#diff-07984c23da1745b22dd4fde5c4b8877dfb190ea19e5e60ab79d8a93e752031c7R1683)
 the reference to make sure on initialization it's null and on retrieval it 
exists. 
   
   It's all private code so in theory we shouldn't need to check the expected 
state each time but I thought it might be good in case future changes causes 
those assumptions to change. Let me know what you think.



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