You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Brown (Code Review)" <ge...@cloudera.org> on 2017/06/23 21:44:37 UTC
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Michael Brown has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7282
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
IMPALA-5281: stress test: introduce stricter pass guidelines
1. Report incorrect results count in the console log table. Previously,
the stress test knew about incorrect results but only reported them to
the console log inline. In was on the onus of a caller to find this. Now
we have a summed count.
2. Fail the process if there are errors, incorrect results, or timeouts.
Previously, the stress test just counted these, but would not fail its
process. This leads to a much stricter pass criteria for the stress
test. This will allow CI to fail and alert a maintainer that something
went wrong.
Testing:
I modified the result hashes for queries in a local runtime_info.json
and observed the reporting of incorrect results, incremented incorrect
results counts, and ultimately process failure.
Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
---
M tests/stress/concurrent_select.py
1 file changed, 24 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/7282/1
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
IMPALA-5281: stress test: introduce stricter pass guidelines
1. Report incorrect results count in the console log table. Previously,
the stress test knew about incorrect results but only reported them to
the console log inline. In was on the onus of a caller to find this. Now
we have a summed count.
2. Fail the process if there are errors, incorrect results, or timeouts.
Previously, the stress test just counted these, but would not fail its
process. This leads to a much stricter pass criteria for the stress
test. This will allow CI to fail and alert a maintainer that something
went wrong.
Testing:
I modified the result hashes for queries in a local runtime_info.json
and observed the reporting of incorrect results, incremented incorrect
results counts, and ultimately process failure.
Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Reviewed-on: http://gerrit.cloudera.org:8080/7282
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/stress/concurrent_select.py
1 file changed, 24 insertions(+), 2 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Michael Brown: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7282/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:
PS2, Line 443: self._num_queries_timedout.value - self._num_queries_cancelled.value
> just curious, what's the relationship between "timed out" and "cancelled" q
> I always thought timed out means hung and cancelled were the deliberately cancelled queries (to test cancellation)
In terms of the final report distinction, you're essentially correct. The short answer is that cancelled queries are also considered "timed out" as far as the stress test execution is concerned.
Long answer:
At the query execution level, all queries are given some sort of expected time duration in which to complete. If they don't, they are cancelled, and a timed_out Boolean is set. This control flow is the same for all queries. You can see it here:
https://github.com/apache/incubator-impala/blob/master/tests/stress/concurrent_select.py#L821
At a level above, that duration is set based on whether or not the query is going to be intentionally cancelled.
https://github.com/apache/incubator-impala/blob/master/tests/stress/concurrent_select.py#L639
This gives an intentionally-cancelling query time to "ramp up" before being cancelled.
Once a query finished, if it has timed out, a flag is examined to see whether the query was meant to be intentionally cancelled. If yes, great. If not, there's logic below to handle some of the cases.
There is other logic that increments the report:
https://github.com/apache/incubator-impala/blob/master/tests/stress/concurrent_select.py#L729
I suppose we could change the flow here not to need the arithmetic.
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Matthew Mulder (Code Review)" <ge...@cloudera.org>.
Matthew Mulder has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 1: Code-Review+1
I reviewed the code and performed a test run, but I'm relying on Michael's testing of changing the result hashes to produce incorrect results.
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 1:
From a code perspective, it's straightforward. But Matt may have more stress test knowledge than I do re: the implications. When he gives it a thumbs up, I can +2 it.
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 2:
Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/809/
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7282/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:
PS2, Line 443: self._num_queries_timedout.value - self._num_queries_cancelled.value
just curious, what's the relationship between "timed out" and "cancelled" queries? I always thought timed out means hung and cancelled were the deliberately cancelled queries (to test cancellation), but apparently, and so don't see why the bookkeeping is related, but I must be missing something? (But I also see this is consistent with line 713 and so does seem "correct").
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-5281: stress test: introduce stricter pass guidelines
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7282/2/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:
PS2, Line 443: self._num_queries_timedout.value - self._num_queries_cancelled.value
> > I always thought timed out means hung and cancelled were the deliberately
Thanks.
--
To view, visit http://gerrit.cloudera.org:8080/7282
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f2174a527193ae01be45b8ed56315c465883346
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Mulder <mm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes