Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20820
Change subject: [logging] simplify and improve LogThrottler ...................................................................... [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 --- M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/server/server_base.cc M src/kudu/util/logging-test.cc M src/kudu/util/logging.cc M src/kudu/util/logging.h 6 files changed, 130 insertions(+), 109 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/20820/1 -- To view, visit http://gerrit.cloudera.org:8080/20820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6c26c4fb11c40bfba3e668d210bdaa1d497c51d0 Gerrit-Change-Number: 20820 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <ale...@apache.org>