IMPALA-6997: Avoid redundant dumping in SetMemLimitExceeded() When a UDF hits a MemLimitExceeded, the query does not immediately abort. Instead, UDFs rely on the caller checking the query_status_ periodically. This means that on some codepaths, UDFs can call SetMemLimitExceeded() many times (e.g. once per row) before the query fragment exits.
RuntimeState::SetMemLimitExceeded() currently constructs a MemLimitExceeded Status and dumps it for each call, even if the query has already hit an error. This is expensive and can delay an fragment from exiting when UDFs are repeatedly hitting MemLimitExceeded. This changes SetMemLimitExceeded() to avoid dumping if the query_status_ is already not ok. Change-Id: I92b87f370a68a2f695ebbc2520a98dd143730701 Reviewed-on: http://gerrit.cloudera.org:8080/10364 Reviewed-by: Tim Armstrong <tarmstr...@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/8e5c18c3 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/8e5c18c3 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/8e5c18c3 Branch: refs/heads/2.x Commit: 8e5c18c3b789da8208611e77cd25899be78d4c8e Parents: 60fdae7 Author: Joe McDonnell <joemcdonn...@cloudera.com> Authored: Wed May 9 15:05:37 2018 -0700 Committer: Impala Public Jenkins <impala-public-jenk...@gerrit.cloudera.org> Committed: Thu May 17 22:03:02 2018 +0000 ---------------------------------------------------------------------- be/src/runtime/runtime-state.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/8e5c18c3/be/src/runtime/runtime-state.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/runtime-state.cc b/be/src/runtime/runtime-state.cc index 15b3354..041c19b 100644 --- a/be/src/runtime/runtime-state.cc +++ b/be/src/runtime/runtime-state.cc @@ -206,6 +206,16 @@ Status RuntimeState::LogOrReturnError(const ErrorMsg& message) { void RuntimeState::SetMemLimitExceeded(MemTracker* tracker, int64_t failed_allocation_size, const ErrorMsg* msg) { + // Constructing the MemLimitExceeded and logging it is not cheap, so + // avoid the cost if the query has already hit an error. + // This is particularly important on the UDF codepath, because the UDF codepath + // cannot abort the fragment immediately. It relies on callers checking status + // periodically. This means that this function could be called a large number of times + // (e.g. once per row) before the fragment aborts. See IMPALA-6997. + { + lock_guard<SpinLock> l(query_status_lock_); + if (!query_status_.ok()) return; + } Status status = tracker->MemLimitExceeded(this, msg == nullptr ? "" : msg->msg(), failed_allocation_size); {
