[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Attila Jeges has abandoned this change. Change subject: IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/4572 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I471a90c8e3bdecb4fed08fbae3436c50b8618471 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Dan Hecht has posted comments on this change. Change subject: IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 .. Patch Set 1: > That seems possible. I never understood how the status didn't show > up. In that case, I think we should see if this repros after Tim's fixes go in. If not, I think we shouldn't change anything. If it still reproduces, then I think we should look at the callstack where the mem-limit-exceeded status is generated and figure out why it doesn't get propagated. -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 .. Patch Set 1: That seems possible. I never understood how the status didn't show up. -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Dan Hecht has posted comments on this change. Change subject: IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 .. Patch Set 1: Tim, maybe this was one of the dropped scanner status that you've fixed? -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
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 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Attila Jeges 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 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. -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
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? -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 .. Patch Set 1: Code-Review+1 -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Alex Behm has posted comments on this change. Change subject: IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 .. Patch Set 1: Code-Review+1 -- 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 Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Attila Jeges has uploaded a new change for review. http://gerrit.cloudera.org:8080/4572 Change subject: IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 .. IMPALA-4216: Test became flaky: TestTpchMemLimitError.test_low_mem_limit_q20 Changed the capitalisation of "Memory limit exceeded" error report message because it was inconsistent with the capitalisation elsewhere. This will make tests that expect a generic "Memory limit exceeded" error more robust. Change-Id: I471a90c8e3bdecb4fed08fbae3436c50b8618471 --- M be/src/runtime/runtime-state.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/4572/1 -- To view, visit http://gerrit.cloudera.org:8080/4572 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I471a90c8e3bdecb4fed08fbae3436c50b8618471 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges