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 <atti...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: No