Dan Hecht has posted comments on this change.
Change subject: IMPALA-4216: Test became flaky:
TestTpchMemLimitError.test_low_mem_limit_q20
......................................................................
Patch Set 1:
> > But why do we no longer get the MEM_LIMIT_EXCEEDED error status
> > returned by this query?
>
> I could not reproduce the issue. My guess is that we get the
> MEM_LIMIT_EXCEEDED error status but it doesn't get added to the
> error_log. Probably when we call LogError(status.msg), ErrorCount()
> is greater than max_errors:
>
> bool RuntimeState::LogError(const ErrorMsg& message, int
> vlog_level) {
> lock_guard<SpinLock> l(error_log_lock_);
> // All errors go to the log, unreported_error_count_ is counted
> independently of the
> // size of the error_log to account for errors that were already
> reported to the
> // coordinator
> VLOG(vlog_level) << "Error from query " << query_id() << ": " <<
> message.msg();
> if (ErrorCount(error_log_) < query_options().max_errors) {
> AppendError(&error_log_, message);
> return true;
> }
> return false;
> }
>
> I could be mistaken, I'm not really familiar with how error
> handling is implemented.
The query status doesn't come from the error log, so I don't think the limit
should have an impact here. It seems we are dropping a returned mem limit
exceeded status somewhere, and we should fix that.
Do we still have the log file for a failed run? If so, the log should show the
callstack in which the MEM_LIMIT_EXCEEDED status object was created, which may
give us a clue as to where it's getting dropped. If we don't have the log, we
may need to wait for this to repro.
--
To view, visit http://gerrit.cloudera.org:8080/4572
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I471a90c8e3bdecb4fed08fbae3436c50b8618471
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Attila Jeges <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Attila Jeges <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: No
