IMPALA-7157: Avoid unnecessarily pretty printing profiles per fragment instance
In SendReport(), if VLOG_FILE_IS_ON is 'true' (which is not the most verbose logging level, but is higher than default), we pretty print the profile for every fragment instance, which is a very expensive operation, as serializing the profile is non-trivial (look at RuntimeProfile::PrettyPrint()), and printing large amounts of information to the logs isn't cheap as well. Lastly, it is very noisy. This seems unnecessary since this will not benefit us, as all the profiles are merged at the coordinator side. We could argue that this might be necessary when an executor fails to send the profile to the coordinator, but that signifies a network issue which will not be reflected in the profile of any fragment instance. This will help reduce noise in the logs when the log level is bumped up to find other real issues that VLOG_FILE can help with. Change-Id: Ic0445950385fa6160764feaed9a993fa0e59b242 Reviewed-on: http://gerrit.cloudera.org:8080/10669 Reviewed-by: Sailesh Mukil <sail...@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/4cad5507 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/4cad5507 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/4cad5507 Branch: refs/heads/2.x Commit: 4cad55077c5fc2a630b9177bc3799d7bad7087ad Parents: 7c5eb93 Author: Sailesh Mukil <sail...@apache.org> Authored: Fri Jun 8 16:27:34 2018 -0700 Committer: Impala Public Jenkins <impala-public-jenk...@gerrit.cloudera.org> Committed: Wed Jun 13 03:10:18 2018 +0000 ---------------------------------------------------------------------- be/src/runtime/fragment-instance-state.cc | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/4cad5507/be/src/runtime/fragment-instance-state.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/fragment-instance-state.cc b/be/src/runtime/fragment-instance-state.cc index c61cb81..d998932 100644 --- a/be/src/runtime/fragment-instance-state.cc +++ b/be/src/runtime/fragment-instance-state.cc @@ -365,13 +365,8 @@ void FragmentInstanceState::SendReport(bool done, const Status& status) { DCHECK(status.ok() || done); DCHECK(runtime_state_ != nullptr); - if (VLOG_FILE_IS_ON) { - VLOG_FILE << "Reporting " << (done ? "final " : "") << "profile for instance " - << PrintId(runtime_state_->fragment_instance_id()); - stringstream ss; - profile()->PrettyPrint(&ss); - VLOG_FILE << ss.str(); - } + VLOG_FILE << "Reporting " << (done ? "final " : "") << "profile for instance " + << PrintId(runtime_state_->fragment_instance_id()); // Update the counter for the peak per host mem usage. if (per_host_mem_usage_ != nullptr) {
