DomGarguilo commented on code in PR #4740:
URL: https://github.com/apache/accumulo/pull/4740#discussion_r1678164615
##########
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java:
##########
@@ -702,16 +701,11 @@ public void run() {
try {
final AtomicReference<Throwable> err = new AtomicReference<>();
- final AtomicLong timeSinceLastCompletion = new AtomicLong(0L);
+ final AtomicBoolean isIdle = new AtomicBoolean(true);
while (!shutdown) {
- idleProcessCheck(() -> {
- return timeSinceLastCompletion.get() == 0
- /* Never started a compaction */ ||
(timeSinceLastCompletion.get() > 0
- && (System.nanoTime() - timeSinceLastCompletion.get())
- > idleReportingPeriodNanos);
- });
+ idleProcessCheck(isIdle::get);
Review Comment:
I figured out what was going on. In order to have the metric be set to idle,
two consecutive calls to `AbstractServer.idleProcessCheck()` need to be made
passing in true. Depending on if I used the `idleProcessCheck(isIdle::get);` or
`idleProcessCheck(()->true);`, the metric value would get stuck since the
compactor code loops while a compaction is happening and the value was not
updated. By adding `idleProcessCheck(() -> true);` in the main loop, we ensure
the metric is correctly marked idle when not compacting. The nested compaction
loop handles setting the metric to busy and back to idle after compaction
completes.
Edit: rereading your comments now I see that you came to the same conclusion
before I did. I just didn't understand it yet lol
--
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]
