keith-turner commented on code in PR #4737:
URL: https://github.com/apache/accumulo/pull/4737#discussion_r1672744418
##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -71,28 +73,28 @@ protected AbstractServer(String appName, ConfigOpts opts,
String[] args) {
lmd.getIntervalMillis(context.getConfiguration()),
TimeUnit.MILLISECONDS);
ThreadPools.watchNonCriticalScheduledTask(future);
processMetrics = new ProcessMetrics(context);
- idleReportingPeriodNanos = TimeUnit.MILLISECONDS.toNanos(
+ idleReportingPeriod = Duration.ofMillis(
Review Comment:
Not a change for this PR, but looking at this I suspect adding a `Duration
getDuration(Property)` to Accumulo configuration would improve this and other
code.
##########
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java:
##########
@@ -694,15 +693,17 @@ public void run() {
try {
final AtomicReference<Throwable> err = new AtomicReference<>();
- final AtomicLong timeSinceLastCompletion = new AtomicLong(0L);
+ final AtomicReference<NanoTime> lastCompletionTime = new
AtomicReference<>(null);
while (!shutdown) {
idleProcessCheck(() -> {
- return timeSinceLastCompletion.get() == 0
- /* Never started a compaction */ ||
(timeSinceLastCompletion.get() > 0
Review Comment:
These changes fix a bug in the existing code. Seems like the existing code
was assuming that nano times would be positive which I do not think is safe to
assume. This change is being made in main, if the bug exists in 2.1 then may
want to fix it in another PR w/o the NanoTime changes.
--
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]
