[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownTested-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 Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5281: stress test: introduce stricter pass guidelines
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 BrownGerrit-Reviewer: David Knupp Gerrit-Reviewer: Matthew Mulder Gerrit-HasComments: No
[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