accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

sets compaction coordinator status update log to trace

Open keith-turner opened this issue 1 year ago • 5 comments

The log message set to trace in this PR was consuming 20% to 30% of the logs for the compaction coordinator.

keith-turner avatar Apr 23 '24 20:04 keith-turner

The change itself looks fine.

How fast were the messages generated? Wondering if this is another case where it would help to somehow track and generate messages at a lower rate (interval or every N).

The tipping point would be, without seeing these messages, can the compaction progress still be seen while its running to show that its likely working and making progress? Basically issue #4485 seems to be calling for better status / logging of compaction progress - so with this at trace, would it cause the same type of hurdle to troubleshoot?

(This PR will not address the other issue, just pointing it out for consideration.)

EdColeman avatar Apr 23 '24 21:04 EdColeman

@dlmarion Would the extra insights be more suitable as metrics, or are these logs specifically useful because they contain something that isn't possible to communicate via metrics? I like the idea of a dedicated compaction status logger that could be configured to emit to a separate place, if one wished to do so. Maybe all that's needed here is to use a dedicated logger instead of the same one as the rest of the coordinator?

ctubbsii avatar Apr 24 '24 17:04 ctubbsii

I think this status update information is useful. It provides insight that we never had with internal compactions. I would rather see a different logger being used, that gets directed to a different file.

A named logger would be nice. That strategy could also be useful for something else I wanted to do. I wanted to add some more detailed trace logging for when the coordinator collects info from tservers. Having a named logger for that would be nice because then could turn on just that trace logging.

keith-turner avatar Apr 24 '24 17:04 keith-turner

@ctubbsii - I don't think these log messages make sense as metrics because they are information about a specific transient event. The event is bound by the start and stop time, and you would need to add a tag to the metric with the external compaction id to find it. This would cause an issue because this would generate a ton of unique metrics.

I think it's sufficient to add a different logger in the CompactionCoordinator for these messages, such as:

  private static final Logger LOG = LoggerFactory.getLogger("CompactionStatusLogger");

dlmarion avatar Apr 24 '24 17:04 dlmarion

Looking again at the Compactor, it calls updateCompactionStatus with STARTED, SUCCEEDED, IN_PROGRESS, CANCELED, and FAILED. I wonder if only that method needs the different logger.

@dlmarion made that change in 4aaaa18

keith-turner avatar Apr 24 '24 19:04 keith-turner