You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@impala.apache.org by "Michael Brown (Code Review)" <ge...@cloudera.org> on 2016/08/10 18:30:06 UTC

[Impala-ASF-CR] IMPALA-3969: stress test: add option to (en|dis)able codegen

Michael Brown has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3887

Change subject: IMPALA-3969: stress test: add option to (en|dis)able codegen
......................................................................

IMPALA-3969: stress test: add option to (en|dis)able codegen

It can be useful for debugging purposes to run the stress test with
codegen disabled. This patch adds a commandline option to the stress
test entry point that enables or disables codegen for all queries and
passes the value through to the appropriate objects for eventually
setting as a query option.

Testing: I ran concurrent_select.py by hand against an Impala cluster
with codegen both enabled and disabled. Both the log written by
concurrent_select.py and the Impala logs on the cluster indicated
DISABLE_CODEGEN was being set as I directed.

Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
---
M tests/stress/concurrent_select.py
1 file changed, 12 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/3887/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-3969: stress test: add option to set common query options

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.

David Knupp has posted comments on this change.

Change subject: IMPALA-3969: stress test: add option to set common query options
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3887/3/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS3, Line 1367: Comma-delimited string of query options and values
> The nargs idea is simple enough. I have a patch where that works (it's a tw
The verbosity complaint is valid, but not so bad if we provided short options, e.g., -o as a viable alternative for --common-query-option. Hopefully setting nargs="*" (or whatever is valid) will work.


-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
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: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3969: stress test: add option to set common query options

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.

Michael Brown has posted comments on this change.

Change subject: IMPALA-3969: stress test: add option to set common query options
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3887/3/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS3, Line 1367: Comma-delimited string of query options and values
> I think argparse has a standard way of dealing with multiple command line a
The nargs idea is simple enough. I have a patch where that works (it's a two-line change). I need to make sure that when a Jenkins parameter value has spaces in it, Jenkins properly quotes when exporting it to the environment.

I think using an append action would increase verbosity in the cases someone calls this from the command line: he'd have to reuse the option over and over again, which can be tedious to do.

I'll test on Jenkins next and see what it does.


-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
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: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3969: stress test: add option to set common query options

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.

Michael Brown has uploaded a new patch set (#3).

Change subject: IMPALA-3969: stress test: add option to set common query options
......................................................................

IMPALA-3969: stress test: add option to set common query options

It can be useful for debugging purposes to run the stress test with
custom query options, for example with codegen disabled. This patch adds
a commandline option to the stress test entry point that allows a caller
to set query options.

To reduce support as new options are chosen, we allow a freeform,
comma-delimited string as the option's value, where the string looks
like:

 option1=value1,...,optionN=valueN

This means we don't do much validation that this string is well-formed.
Callers must take care to type the correct options and values.

Testing: I ran concurrent_select.py by hand against an Impala cluster
with codegen both enabled and disabled, ala

 --common_query_options DISABLE_CODEGEN=true

Both the log written by concurrent_select.py and the Impala logs on the
cluster indicated DISABLE_CODEGEN was being set as I directed. I also
did negative testing for a few bad --common_query_options values. Either
concurrent_select.py catches the error, or when the first query runs,
the error is reported.

Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
---
M tests/stress/concurrent_select.py
1 file changed, 48 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/3887/3
-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
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: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-3969: stress test: add option to set common query options

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.

Hello David Knupp, Tim Armstrong,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3887

to look at the new patch set (#5).

Change subject: IMPALA-3969: stress test: add option to set common query options
......................................................................

IMPALA-3969: stress test: add option to set common query options

It can be useful for debugging purposes to run the stress test with
custom query options, for example with codegen disabled. This patch adds
a command line option to the stress test entry point that allows a
caller to set query options.

To reduce support as new options are chosen, we allow freeform,
space-delimited option=value arguments as the option's value, like:

 --common_query_options option1=value1 ... optionN=valueN

This means we don't do much validation that these options and values are
well-formed.  Callers must take care to type the correct options and
values.

Testing: I ran concurrent_select.py by hand against an Impala cluster
with codegen both enabled and disabled, ala

 --common_query_options DISABLE_CODEGEN=true

Both the log written by concurrent_select.py and the Impala logs on the
cluster indicated DISABLE_CODEGEN was being set as I directed. I also
did negative testing for a few bad --common_query_options values. Either
concurrent_select.py catches the error, or when the first query runs,
the error is reported.

Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
---
M tests/stress/concurrent_select.py
1 file changed, 49 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/3887/5
-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-3969: stress test: add option to set common query options

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.

David Knupp has posted comments on this change.

Change subject: IMPALA-3969: stress test: add option to set common query options
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3887/3/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS3, Line 1367: Comma-delimited string of query options and values
I think argparse has a standard way of dealing with multiple command line arguments, rather than using a comma-delimited string. Perhaps...

https://docs.python.org/3/library/argparse.html#nargs

Or maybe action='append' ?


-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
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: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3969: stress test: add option to set common query options

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.

Michael Brown has posted comments on this change.

Change subject: IMPALA-3969: stress test: add option to set common query options
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3887/3/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS3, Line 1367: Comma-delimited string of query options and values
> The verbosity complaint is valid, but not so bad if we provided short optio
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
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: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3969: stress test: add option to set common query options

Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.

David Knupp has posted comments on this change.

Change subject: IMPALA-3969: stress test: add option to set common query options
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

Looks good, with an optional suggestion if you feel like following it.

http://gerrit.cloudera.org:8080/#/c/3887/4/tests/stress/concurrent_select.py
File tests/stress/concurrent_select.py:

PS4, Line 1408: Ignoring {query_option} in common query options: '{opt}'
This is a minor suggestion. You might consider adding some explanation as to why these are being ignored, i.e., that these are default options set by the stress test.


-- 
To view, visit http://gerrit.cloudera.org:8080/3887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada041aace60c218a12178d8f1b9a68ff29de72e
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes