Adar Dembo has posted comments on this change. Change subject: KUDU-1442 log number of open log block containers ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/7111/1/src/kudu/fs/log_block_manager.cc File src/kudu/fs/log_block_manager.cc: Line 2019: // Log number of containers opened every 10 seconds Nit: if you do use KLOG_EVERY_N_SECS() you don't need this comment; it'll be obvious from the code. Line 2022: LOG(INFO) << Substitute("Opened $0 log block containers in $1", Nit: how about formatting it as: Substitute("$0: opened $1 log block containers", dir->dir(), container_count); That way the directories are all aligned with one another and it's a little easier to differentiate between them. PS1, Line 2020: MonoTime now = MonoTime::Now(); : if ((now - last_opened_container_log_time).ToSeconds() > 10) { : LOG(INFO) << Substitute("Opened $0 log block containers in $1", : local_report.stats.lbm_container_count, dir->dir()); : last_opened_container_log_time = now; : } > Would just Agreed, unless KLOG_EVERY_N_SECS() will log at the 0th second mark as well as every n seconds. It'd be nice to avoid the 0th second statement since that'll be emitted all the time, even for super fast (or empty) deployments. -- To view, visit http://gerrit.cloudera.org:8080/7111 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dd5eccd166ae81be26d0aa75758539f84d62656 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sam Okrent <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
