IMPALA-4613: Make sure timers are finished before sending report profile FragmentComplete() sends the last report profile. If a timer is still running while the profile is sent, it may not be included in the final report. This patch ensures that the timers are all out of scope before FragmentComplete() gets called.
Change-Id: If841e8b0a1effdab4349f081535b785c8bf2af7a Reviewed-on: http://gerrit.cloudera.org:8080/5394 Reviewed-by: Henry Robinson <[email protected]> Tested-by: Henry Robinson <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/9baf4f03 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/9baf4f03 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/9baf4f03 Branch: refs/heads/hadoop-next Commit: 9baf4f03ad0f37d54b0fe7f315cbb473e0c8a723 Parents: bb63339 Author: Henry Robinson <[email protected]> Authored: Tue Dec 6 04:10:19 2016 -0800 Committer: Henry Robinson <[email protected]> Committed: Wed Dec 7 15:20:27 2016 +0000 ---------------------------------------------------------------------- be/src/runtime/plan-fragment-executor.cc | 28 +++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/9baf4f03/be/src/runtime/plan-fragment-executor.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/plan-fragment-executor.cc b/be/src/runtime/plan-fragment-executor.cc index 07dcfb0..3657882 100644 --- a/be/src/runtime/plan-fragment-executor.cc +++ b/be/src/runtime/plan-fragment-executor.cc @@ -293,10 +293,13 @@ void PlanFragmentExecutor::PrintVolumeIds( Status PlanFragmentExecutor::Open() { DCHECK(prepared_promise_.IsSet() && prepared_promise_.Get().ok()); - SCOPED_TIMER(profile()->total_time_counter()); - SCOPED_TIMER(ADD_TIMER(timings_profile_, OPEN_TIMER_NAME)); - VLOG_QUERY << "Open(): instance_id=" << runtime_state_->fragment_instance_id(); - Status status = OpenInternal(); + Status status; + { + SCOPED_TIMER(profile()->total_time_counter()); + SCOPED_TIMER(ADD_TIMER(timings_profile_, OPEN_TIMER_NAME)); + VLOG_QUERY << "Open(): instance_id=" << runtime_state_->fragment_instance_id(); + status = OpenInternal(); + } if (!status.ok()) FragmentComplete(status); opened_promise_.Set(status); return status; @@ -330,19 +333,20 @@ Status PlanFragmentExecutor::OpenInternal() { Status PlanFragmentExecutor::Exec() { DCHECK(opened_promise_.IsSet() && opened_promise_.Get().ok()); - SCOPED_TIMER(profile()->total_time_counter()); Status status; { // Must go out of scope before FragmentComplete(), otherwise counter will not be - // updated by time final profile is sent, and will always be 0. + // updated by time final profile is sent. + SCOPED_TIMER(profile()->total_time_counter()); SCOPED_TIMER(ADD_TIMER(timings_profile_, EXEC_TIMER_NAME)); status = ExecInternal(); - } - if (!status.ok() && !status.IsCancelled() && !status.IsMemLimitExceeded()) { - // Log error message in addition to returning in Status. Queries that do not fetch - // results (e.g. insert) may not receive the message directly and can only retrieve - // the log. - runtime_state_->LogError(status.msg()); + + if (!status.ok() && !status.IsCancelled() && !status.IsMemLimitExceeded()) { + // Log error message in addition to returning in Status. Queries that do not fetch + // results (e.g. insert) may not receive the message directly and can only retrieve + // the log. + runtime_state_->LogError(status.msg()); + } } FragmentComplete(status); return status;
