Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15197 )

Change subject: KUDU-2996: trace when rpc_duration_too_long_ms exceeded
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15197/3/src/kudu/rpc/rpcz_store.cc
File src/kudu/rpc/rpcz_store.cc:

http://gerrit.cloudera.org:8080/#/c/15197/3/src/kudu/rpc/rpcz_store.cc@266
PS3, Line 266:   if (PREDICT_FALSE(FLAGS_rpc_dump_all_traces)) {
             :     LOG(INFO) << call->ToString() << " took " << duration_ms << 
"ms. Trace:";
             :     call->trace()->Dump(&LOG(INFO), true);
             :   } else if (duration_ms > FLAGS_rpc_duration_too_long_ms) {
             :     LOG(INFO) << call->ToString() << " took " << duration_ms << 
"ms.";
             :     string s = call->trace()->DumpToString();
             :     if (!s.empty()) {
             :       LOG(INFO) << "Trace:\n" << s;
             :     }
             :   }
> Only in PS3 I realized the code under those 2 conditional scopes might be u
I'm not sure about dumping directly to LOG(INFO), won't that result in the bit 
of the trace being split across many log messages? Not necessarily in a logical 
way. My understanding of glog is that each of these statements would result in 
a separate log line:

  https://github.com/cloudera/kudu/blob/master/src/kudu/util/trace.cc#L163

I was copying the code from l258. Would it make sense for the dump_all_traces 
branch to use the DumpToString() approach instead, if we want to unify it?



--
To view, visit http://gerrit.cloudera.org:8080/15197
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20d9b5e318f4a987051f1f7c2cd5b1d92c61bd05
Gerrit-Change-Number: 15197
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Feb 2020 21:25:48 +0000
Gerrit-HasComments: Yes

Reply via email to