You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Attila Jeges (Code Review)" <ge...@cloudera.org> on 2016/09/30 09:01:05 UTC
[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 <at...@cloudera.com>
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4216: Test became flaky: TestTpchMemLimitError.test low mem limit q20
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
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<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.
--
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 <at...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No