Adar Dembo has posted comments on this change. Change subject: [tools]: Keep the verbosity of CLI at FATAL and above ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4447/1/src/kudu/tools/tool_main.cc File src/kudu/tools/tool_main.cc: PS1, Line 215: // Set the verbosity of the commands to WARNING(2) and above. : // If the user had explicitly specified verbosity, then user's : // verbosity level is honored. Since '--v' depends on minloglevel : // specifying either of them on CLI will override this setting. : if (google::GetCommandLineFlagInfoOrDie("minloglevel").is_default && : google::GetCommandLineFlagInfoOrDie("v").is_default) { : google::SetCommandLineOption("minloglevel", : SimpleItoa(google::GLOG_WARNING).c_str()); : } > I'm arguing for more verbosity in anything that is "unexpected" or "abnorma "I'm arguing for more verbosity in anything that is "unexpected" or "abnormal" which is definitely the case for anything WARN or ERROR. For 'INFO' level, I'm also saying that if anything is 100% expected, we probably shouldn't bother with it." Thanks for clarifying, but is this your take on the CLI's verbosity? Or on general glog verbosity in Kudu? I'm asking because we don't uphold your INFO-level standard in Kudu in general, and I'm not sure if we should: "expected" stuff in the logs still serve as useful breadcrumbs, and the timestamps are useful for diagnosing unexpected slowdowns or races. For example, opening an FsManager logs the contents of the instance PB file at the INFO level. It's completely expected, but the logged UUID is useful for matching against peer UUIDs. I'm using this as an example because many CLI commands open FsManagers and log this same thing which isn't particularly useful in the CLI context. So going back to my earlier point, I don't see how we can include INFO-level logs in the CLI without either logging too much, or removing too much stuff that's otherwise useful in server glogs. I'll concede that WARNING-level may be reasonable, though there's something to be said for the purity of never logging to stderr in normal operation. Regarding machine-readability, I agree with your points that it's orthogonal to stderr logging and that we shouldn't guarantee anything. I'm willing to erode the second point for some simple actions (i.e. I don't mind signing on a dotted line that "kudu fs dump uuid" prints nothing but a uuid to stdout), but I don't feel strongly. -- To view, visit http://gerrit.cloudera.org:8080/4447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I882a340d4c1d205e4e998c888f487b7185000e3c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
