You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Gabor Kaszab (Code Review)" <ge...@cloudera.org> on 2017/12/01 17:20:34 UTC
[Impala-ASF-CR] IMPALA-6265 Increase timeout in test cancellation tests
Gabor Kaszab has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8713
Change subject: IMPALA-6265 Increase timeout in test cancellation tests
......................................................................
IMPALA-6265 Increase timeout in test cancellation tests
Apparently, ASAN query execution happens slower than on a Release build.
The query cancellation shell tests use timeout to wait until the query
gets to fetch phase and then cancel the query. With ASAN it seems that
the timeout is not enough as the query doesn't get to fetch phase when
when the cancellation happens.
Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
---
M tests/shell/test_shell_commandline.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8713/1
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
IMPALA-6265 Query cancellation test enhancements
In the query cancellation tests it is essential to wait until the
query gets to a desired state (waiting_to_finish, fetching) and then
cancel it. Apparently, ASAN query execution happens slower than on a
Release build. As a result a hard coded timeout threshold is not
sufficient to cover all the builds, or should be set to a wastingly
high value.
As a solution the query state is checked on the Impala debug page in
intervals until it reaches the desired state or the maximum retry
attempt value is reached.
Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Reviewed-on: http://gerrit.cloudera.org:8080/8713
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/shell/test_shell_commandline.py
1 file changed, 36 insertions(+), 9 deletions(-)
Approvals:
Tim Armstrong: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2017 18:08:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@349
PS2, Line 349: def wait_query_until_desired_state(self, stmt, state, max_retry=10):
Thanks, this looks good. The name is a little non-idiomatic. Maybe something like wait_for_query_state()?
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@362
PS2, Line 362: return "The found in flight query is not the one under test: " + \
Maybe just raise an exception? Makes it hard for tests to accidentally forget to check the return value
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@366
PS2, Line 366: ++retry_count
I don't think ++ works in python. It is syntactically valid but doesn't increment the variable. I think "retry_count += 1" is the most concise way of doing this.
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@368
PS2, Line 368: return "Query didn't reach desired state: " + state
Same here - maybe raise an exception or just assert?
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@385
PS2, Line 385: get_statement_from_args
It looks like we have the original statement in the tests so it seems better to pass the statement through wait_query_until_desired_state() instead of trying to extract it from the command line.
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@386
PS2, Line 386: ImapalShell
ImpalaShell
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Dec 2017 17:39:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1569/
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2017 18:08:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8713
to look at the new patch set (#2).
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
IMPALA-6265 Query cancellation test enhancements
In the query cancellation tests it is essential to wait until the
query gets to a desired state (waiting_to_finish, fetching) and then
cancel it. Apparently, ASAN query execution happens slower than on a
Release build. As a result a hard coded timeout threshold is not
sufficient to cover all the builds, or should be set to a wastingly
high value.
As a solution the query state is checked on the Impala debug page in
intervals until it reaches the desired state or the maximum retry
attempt value is reached.
Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
---
M tests/shell/test_shell_commandline.py
1 file changed, 41 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8713/2
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/8713/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/8713/1/tests/shell/test_shell_commandline.py@352
PS1, Line 352: a state given via parameter or a maximum retry count is reached.
> Can we hit the Web UI in a loop to figure out where it is?
Thanks for the suggestions!
I made the tests to poll the status of the query on the debug page. Checks in every 1 sec, until a threshold of 10 retries.
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@349
PS2, Line 349: def wait_for_query_state(self, stmt, state, max_retry=15):
> Thanks, this looks good. The name is a little non-idiomatic. Maybe somethin
Indeed, that is a better name.
Done
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@362
PS2, Line 362: exc_text = "The found in flight query is not the one under test: " + \
> Maybe just raise an exception? Makes it hard for tests to accidentally forg
Good observation.
Done
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@366
PS2, Line 366: return
> I don't think ++ works in python. It is syntactically valid but doesn't inc
Good to know! Thanks!
Done
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@368
PS2, Line 368: sleep(1.0)
> Same here - maybe raise an exception or just assert?
Done
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@385
PS2, Line 385: test_get_log_once(self,
> It looks like we have the original statement in the tests so it seems bette
I'm not sure I get this comment :)
In the tests I create a string variable that contains the args for the command line like: '-q "select 1;"'
What I can do is to introduce instead a variable to hold the command itself and when I execute ImpalaShell then I bring it to the format to act like the args parameter.
Is this what you meant?
(If yes, then it's done)
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@386
PS2, Line 386: hat get_log
> ImpalaShell
Done
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2017 11:20:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6265 Increase timeout in test cancellation tests
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Increase timeout in test cancellation tests
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8713/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/8713/1/tests/shell/test_shell_commandline.py@352
PS1, Line 352: sleep(2.5)
It would be nice to set this to a higher value for ASAN just to be confident that it is fast enough.
Maybe we should use specific_build_type_timeout() to set a different timeout for slow build types.
E.g.
specific_build_type_timeout(2, slow_build_timeout=5)
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Dec 2017 17:40:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6265 Increase timeout in test cancellation tests
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Increase timeout in test cancellation tests
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8713/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/8713/1/tests/shell/test_shell_commandline.py@352
PS1, Line 352: sleep(2.5)
> It would be nice to set this to a higher value for ASAN just to be confiden
Can we hit the Web UI in a loop to figure out where it is?
timeout = time.time() + 60
while time.time() < timeout:
if get_state_of_query_in_webui() == "WHATEVER":
break
You probably get the idea. These are so annoying.
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Dec 2017 18:46:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2017 21:40:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8713 )
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/8713/2/tests/shell/test_shell_commandline.py@385
PS2, Line 385: test_get_log_once(self,
> I'm not sure I get this comment :)
Yep, that looks better.
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Dec 2017 18:08:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6265 Query cancellation test enhancements
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8713
to look at the new patch set (#3).
Change subject: IMPALA-6265 Query cancellation test enhancements
......................................................................
IMPALA-6265 Query cancellation test enhancements
In the query cancellation tests it is essential to wait until the
query gets to a desired state (waiting_to_finish, fetching) and then
cancel it. Apparently, ASAN query execution happens slower than on a
Release build. As a result a hard coded timeout threshold is not
sufficient to cover all the builds, or should be set to a wastingly
high value.
As a solution the query state is checked on the Impala debug page in
intervals until it reaches the desired state or the maximum retry
attempt value is reached.
Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
---
M tests/shell/test_shell_commandline.py
1 file changed, 36 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8713/3
--
To view, visit http://gerrit.cloudera.org:8080/8713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie0bff485a872df7be8efd784314a6ca91aaadd11
Gerrit-Change-Number: 8713
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>