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 <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/caf275c1 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/caf275c1 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/caf275c1 Branch: refs/heads/master Commit: caf275c11a62c33d0211e71f3285c4977dd6799d Parents: a64cfc5 Author: Joe McDonnell <[email protected]> Authored: Wed May 9 15:05:37 2018 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Wed May 16 02:22:24 2018 +0000 ---------------------------------------------------------------------- be/src/runtime/runtime-state.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/caf275c1/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); {
