apache /
kudu /
e8f459284164b0d82d6120684b17d2f30b4ddb4a [logging] simplify and improve LogThrottler
I've noticed that among all the usages of LogThrottler throughout the
current codebase, the alternating tags functionality was used only in
a single place. That single usage was extremely confusing since the
throttling interval for the former KLOG_EVERY_N_SECS_THROTTLER macro was
different while using it with the same LogThrottler instance, so the
throttling frequency was unpredictable and didn't make much sense,
especially when trying to analyze the generated logs post-mortem.
Getting rid of the tag alterations allows for simpler logic and cleaner
code in the LogThrottler::ShouldLog() method as well.
I also noticed that thousands of messages might be suppressed and go
unreported for the rest of a Kudu server's life-cycle, but for some
scenarios it could be beneficial to report on the number of suppressions
when the server shuts down, so there were a way to assess how massive
the very last surge in throttled log messages had been.
This patch contains the following updates on LogThrottler:
* The alternating tags for the LogThrottler are gone along with the
corresponding macro KLOG_EVERY_N_SECS_THROTTLER(), and that's
replaced with the new KLOG_THROTTLER() macro.
* LogThrottler::ShouldLog() is rewritten using std::atomic for
all the fields that require atomicity. There is no need having
the ANNOTATE_BENIGN_RACE_SIZED annotation anymore to cover up
the sloppiness of the former code.
* Throttling interval is now a parameter of the LogThrottler's
constructor, and is an invariant of a LogThrottler class instance.
* LogThrottler's destructor now logs about the number of suppressed
but not reported messages since logging about them last time.
* ostream& operator<<(ostream, const PRIVATE_ThrottleMsg&) is
optimized to not perform dynamic casting in release builds: if
there is a call site that uses not LogMessage::LogStream-derived
output stream, that should have been caught during the code review
process or by pre-commit tests running against DEBUG binaries.
Change-Id: I6c26c4fb11c40bfba3e668d210bdaa1d497c51d0
Reviewed-on: http://gerrit.cloudera.org:8080/20820
Reviewed-by: Abhishek Chennaka <achennaka@cloudera.com>
Tested-by: Alexey Serbin <alexey@apache.org>
6 files changed
