ctubbsii commented on code in PR #3542:
URL: https://github.com/apache/accumulo/pull/3542#discussion_r1242789779
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -1101,7 +1107,7 @@ public void run() {
log.error("Error initializing metrics, metrics will not be emitted.",
e1);
}
- recoveryManager = new RecoveryManager(this,
TIME_TO_CACHE_RECOVERY_WAL_EXISTENCE);
+ recoveryManager = new RecoveryManager(this,
timeToCacheRecoveryWalExistence);
Review Comment:
This should pass a LongSupplier to compute the value dynamically from the
current value of the configurable interval time.
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -197,7 +197,7 @@ public void run() {
}
if (currentTServers.isEmpty()) {
- eventListener.waitForEvents(Manager.TIME_TO_WAIT_BETWEEN_SCANS);
+ eventListener.waitForEvents(manager.getWaitTimeBetweenScans());
Review Comment:
Using the Supplier idea I suggested, the value can be obtained once at the
top of this loop, and reused for the rest of the loop iteration, using a final
local variable, so it doesn't change in the middle of the iteration.
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -449,6 +448,13 @@ public static void main(String[] args) throws Exception {
log.info("SASL is not enabled, delegation tokens will not be available");
delegationTokensAvailable = false;
}
+ this.waitTimeBetweenScans =
+ aconf.getTimeInMillis(Property.MANAGER_TABLET_GROUP_WATCHER_INTERVAL);
Review Comment:
Computing it once at the time of the Manager startup makes this a fixed
property, only changeable at restarts. You can do that, of course, but should
add it to the `Property.fixedProperties`. However, a better idea is to have the
field be a LongSupplier, and compute it from the config dynamically. You can
use Guava's `Suppliers.memoize` to create the supplier. You can configure the
memoizing supplier to cache the value for a certain amount of time, if you
don't want it to do the lookup from the config too frequently.
--
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]
