[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-16 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 10: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 16 Feb 2018 09:26:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-16 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Reviewed-on: http://gerrit.cloudera.org:8080/9291
Reviewed-by: David Knupp 
Tested-by: Impala Public Jenkins
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/custom_cluster/test_rpc_exception.py
8 files changed, 86 insertions(+), 7 deletions(-)

Approvals:
  David Knupp: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1946/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 16 Feb 2018 05:51:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 16 Feb 2018 05:31:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 10:

(1 comment)

Thanks for the reviews. Please see PS10.

http://gerrit.cloudera.org:8080/#/c/9291/9/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/9291/9/bin/run-all-tests.sh@56
PS9, Line 56: # Extra arguments passed to start-impala-cluster for tests. These 
do not apply to custom
: # cluster tests.
> Can you add a comment that this does not apply to custom cluster tests?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 16 Feb 2018 03:43:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Lars Volker (Code Review)
Hello Michael Ho, Michael Brown, Sailesh Mukil, David Knupp,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/custom_cluster/test_rpc_exception.py
8 files changed, 86 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9291/10
--
To view, visit http://gerrit.cloudera.org:8080/9291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 9: Code-Review+1

(1 comment)

Feel free to carry forward my +1 after fixing. I'll let David +2 when he's 
happy.

http://gerrit.cloudera.org:8080/#/c/9291/9/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/9291/9/bin/run-all-tests.sh@56
PS9, Line 56: # Extra arguments passed to start-impala-cluster for tests
: : ${TEST_START_CLUSTER_ARGS:=}
Can you add a comment that this does not apply to custom cluster tests?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Fri, 16 Feb 2018 03:01:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 9:

(3 comments)

Thanks for the reviews, please see PS9.

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if use_exclusive_coordinators:
 :   cmd.append("--use_exclusive_coordinato
> Not sure I'm totally following (haven't really looked much at the custom cl
Thanks for the feedback here and on the dev@ list. I added a comment to clarify 
that we ignore TEST_START_CLUSTER_ARGS on purpose here.


http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_exchange_delays.py
File tests/custom_cluster/test_exchange_delays.py:

http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_exchange_delays.py@31
PS8, Line 31: @SkipIfBuildType.n
> May want to remove it now that the fix for IMPALA-6512 is being merged.
Done


http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_krpc_mem_usage.py@64
PS8, Line 64:
> If this applies to all the tests here (it appears to), then I think SkipIf
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 15 Feb 2018 22:05:09 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Lars Volker (Code Review)
Hello Michael Ho, Michael Brown, Sailesh Mukil, David Knupp,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/custom_cluster/test_rpc_exception.py
8 files changed, 84 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9291/9
--
To view, visit http://gerrit.cloudera.org:8080/9291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--use_krpc")
> I wonder if that's on purpose, would you like me to ask on dev@?
Not sure I'm totally following (haven't really looked much at the custom 
cluster tests), but for what it's worth, I think that it's best that tests be 
100% repeatable, and not vary according to differences between one dev 
environment and another. If we're specifically testing that certain custom args 
work, then we should (I think) exclude TEST_START_CLUSTER_ARGS.


http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_krpc_mem_usage.py@64
PS8, Line 64:   @SkipIf.not_krpc
If this applies to all the tests here (it appears to), then I think SkipIf 
decorator can be applied to the entire test class.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 15 Feb 2018 17:32:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 8:

Michael and/or David may want to do another pass and +2 the change.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 15 Feb 2018 08:57:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-15 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 8: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_exchange_delays.py
File tests/custom_cluster/test_exchange_delays.py:

http://gerrit.cloudera.org:8080/#/c/9291/8/tests/custom_cluster/test_exchange_delays.py@31
PS8, Line 31: @SkipIf.not_thrift
May want to remove it now that the fix for IMPALA-6512 is being merged.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 15 Feb 2018 08:56:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-14 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9291/7/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/9291/7/bin/start-impala-cluster.py@264
PS7, Line 264: "-use_krpc=tru
> seems more consistent to do -use_krpc=true here as other options above seem
Done


http://gerrit.cloudera.org:8080/#/c/9291/7/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/7/tests/common/custom_cluster_test_suite.py@138
PS7, Line 138: --use_krpc")
> why not just --use_krpc here ?
Good point, done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 15 Feb 2018 03:27:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-14 Thread Lars Volker (Code Review)
Hello Michael Ho, Michael Brown, Sailesh Mukil, David Knupp,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_exchange_delays.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/custom_cluster/test_rpc_exception.py
9 files changed, 85 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9291/8
--
To view, visit http://gerrit.cloudera.org:8080/9291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-14 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9291/7/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/9291/7/bin/start-impala-cluster.py@264
PS7, Line 264: "-use_krpc %s"
seems more consistent to do -use_krpc=true here as other options above seem to 
explicitly set the boolean flag.


http://gerrit.cloudera.org:8080/#/c/9291/7/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/7/tests/common/custom_cluster_test_suite.py@138
PS7, Line 138: --impalad_args=-use_krpc
why not just --use_krpc here ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Thu, 15 Feb 2018 00:22:50 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread David Knupp (Code Review)
David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 7:

> Patch Set 7:
>
> David, it'd be great if you also had time for a look here. Thanks :)

Will do, although probably not until tomorrow morning.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 20:00:58 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 7:

David, it'd be great if you also had time for a look here. Thanks :)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: David Knupp 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 19:56:32 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 7:

I ran private core and exhaustive tests and added skips for tests that didn't 
work with KRPC. I filed IMPALA-6512 and IMPALA-6513 for them. All other tests 
passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 18:48:59 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Lars Volker (Code Review)
Hello Michael Ho, Michael Brown, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_exchange_delays.py
M tests/custom_cluster/test_krpc_mem_usage.py
M tests/custom_cluster/test_rpc_exception.py
9 files changed, 85 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9291/7
--
To view, visit http://gerrit.cloudera.org:8080/9291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
> I just meant that if someone performs a private build with their own TEST_S
I wonder if that's on purpose, would you like me to ask on dev@?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 18:07:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Lars Volker (Code Review)
Hello Michael Ho, Michael Brown, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_exchange_delays.py
M tests/custom_cluster/test_krpc_mem_usage.py
8 files changed, 82 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9291/6
--
To view, visit http://gerrit.cloudera.org:8080/9291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(2 comments)

> This means we will have automated test runs with both krpc enabled and 
> disabled for the foreseeable future

I'm convinced, then. Thanks.

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
> I'm not sure I understand what you mean. Should they use $TEST_START_CLUSTE
I just meant that if someone performs a private build with their own 
TEST_START_CLUSTER_ARGS, then the TEST_START_CLUSTER_ARGS will not be applied 
in custom cluster tests. This is something I noticed while poking around in the 
code. You don't have to fix that in this patch.


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py@77
PS5, Line 77:   
@CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
> I'll reply on the change itself.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:34:41 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(3 comments)

> Patch Set 5:
>
> (3 comments)
>
> I was trying to think about how this could be done without yet another 
> "global" environment variable, and an unenforceable flag between pytest and 
> the cluster, and more skip logic.
>
> Do you think it's possible to achieve the same effect by:
>
> 1. Creating CI that sets TEST_START_CLUSTER_ARGS appropriately for enabling 
> KRPC
>
> 2. Ensuring $TEST_START_CLUSTER_ARGS is properly reconciled any time the 
> cluster starts, even in custom cluster tests.
>
> 3. Running the 3 tests that are KRPC specific with --use-krpc always set, 
> unconditionally.
>
> I have a feeling this would reduce some of the churn that's introduced in 
> this patch and then would be gutted again later. What do you think?

I think we plan on keeping the option to turn KRPC off, even after we turn it 
on by default. This means we will have automated test runs with both krpc 
enabled and disabled for the foreseeable future, until we completely remove the 
old Thrift RPC code. It would be very convenient to add tests into the 
appropriate test files, e.g. add a test for KRPC metrics showing up on the 
/rpcz page (IMPALA-6269) to test_web_pages.py. In these files we would need a 
way to decorate tests that require KRPC and should be skipped for runs with 
KRPC disabled. Similarly I anticipate that we will have tests that require 
Thrift RPC at some time. Our e2e tests should also be faster than the custom 
cluster tests.

Not having a SkipIf means we would have to look at the environment to skip 
those tests from within the test methods, but wrapping that in a function then 
seems to be equivalent of having a SkipIf.

Hopefully we will also have a lot more than 3 tests that require KRPC to be 
turned on. :)

We could try to interfere the SkipIf from the command line argument to reduce 
the amount of config variables. Alternatively we could unhide -use_krpc and get 
the value from the /varz page.

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
> It seems like a bug that custom cluster tests don't prepend $TEST_START_CLU
I'm not sure I understand what you mean. Should they use 
$TEST_START_CLUSTER_ARGS instead of appending to the command line themselves?


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py@18
PS5, Line 18: import os
> Unused import.
Done


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py@77
PS5, Line 77:   
@CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
> An alternative to skipping would be to just force --use-krpc always to on i
I'll reply on the change itself.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:25:59 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-13 Thread Michael Brown (Code Review)
Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 5:

(3 comments)

I was trying to think about how this could be done without yet another "global" 
environment variable, and an unenforceable flag between pytest and the cluster, 
and more skip logic.

Do you think it's possible to achieve the same effect by:

1. Creating CI that sets TEST_START_CLUSTER_ARGS appropriately for enabling KRPC

2. Ensuring $TEST_START_CLUSTER_ARGS is properly reconciled any time the 
cluster starts, even in custom cluster tests.

3. Running the 3 tests that are KRPC specific with --use-krpc always set, 
unconditionally.

I have a feeling this would reduce some of the churn that's introduced in this 
patch and then would be gutted again later. What do you think?

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/custom_cluster_test_suite.py@137
PS5, Line 137: if pytest.config.option.test_krpc:
 :   cmd.append("--impalad_args=-use_krpc")
It seems like a bug that custom cluster tests don't prepend 
$TEST_START_CLUSTER_ARGS with other custom args.


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/common/test_skip.py@18
PS5, Line 18: import os
Unused import.


http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py
File tests/custom_cluster/test_krpc_mem_usage.py:

http://gerrit.cloudera.org:8080/#/c/9291/5/tests/custom_cluster/test_krpc_mem_usage.py@77
PS5, Line 77:   
@CustomClusterTestSuite.with_args("--stress_datastream_recvr_delay_ms=1000")
An alternative to skipping would be to just force --use-krpc always to on in 
these tests.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:05:45 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-12 Thread Lars Volker (Code Review)
Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add KRPC test flag
..


Patch Set 4:

(4 comments)

Thanks for the review, please see my inline comments and PS5.

http://gerrit.cloudera.org:8080/#/c/9291/4/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/bin/start-impala-cluster.py@57
PS4, Line 57: krpc
> KRPC
Done


http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/custom_cluster_test_suite.py@138
PS4, Line 138: "--impalad_args=-use_krpc"
> Not quite following what this translates to ? For now, I have been using --
Yes, -use_krpc is a boolean and as such its presence enables it. This page has 
all four variations that are supported for a boolean flag: 
http://gflags.github.io/gflags/

 app_containing_foo --big_menu
 app_containing_foo --nobig_menu
 app_containing_foo --big_menu=true
 app_containing_foo --big_menu=false

I picked the single-dash variant because I've seen this convention elsewhere I 
think. Let me know if you prefer two dashes.


http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/skip.py
File tests/common/skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/skip.py@89
PS4, Line 89: krpc
> nit: KRPC
Done


http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/test_skip.py@24
PS4, Line 24: class TestSkipIf(ImpalaTestSuite):
> Is this test gonna be part of the patch or is it for testing only ?
My idea was to keep it in this patch so that we actually have a test for the 
things I added. Once we add a proper (non-custom-cluster) test using the 
SkipIfs I plan to drop this test again.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 04:07:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add KRPC test flag

2018-02-12 Thread Lars Volker (Code Review)
Hello Michael Ho, Michael Brown, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6508: add KRPC test flag
..

IMPALA-6508: add KRPC test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable KRPC for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with KRPC support or not.

- SkipIf.not_krpc can be used to mark a test that depends on KRPC.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_krpc_mem_usage.py
7 files changed, 81 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 


[Impala-ASF-CR] IMPALA-6508: add krpc test flag

2018-02-12 Thread Michael Ho (Code Review)
Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9291 )

Change subject: IMPALA-6508: add krpc test flag
..


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/9291/4/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/bin/start-impala-cluster.py@57
PS4, Line 57: krpc
KRPC


http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/custom_cluster_test_suite.py
File tests/common/custom_cluster_test_suite.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/custom_cluster_test_suite.py@138
PS4, Line 138: "--impalad_args=-use_krpc"
Not quite following what this translates to ? For now, I have been using 
--impalad_args="--use_krpc=true". Does this line have similar effect ?


http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/skip.py
File tests/common/skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/skip.py@89
PS4, Line 89: krpc
nit: KRPC


http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9291/4/tests/common/test_skip.py@24
PS4, Line 24: class TestSkipIf(ImpalaTestSuite):
Is this test gonna be part of the patch or is it for testing only ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 13 Feb 2018 02:31:55 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6508: add krpc test flag

2018-02-12 Thread Lars Volker (Code Review)
Hello Michael Ho, Michael Brown, Sailesh Mukil,

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

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

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

Change subject: IMPALA-6508: add krpc test flag
..

IMPALA-6508: add krpc test flag

This change adds a flag "--use_krpc" to start-impala-cluster.py. The
flag is currently passed as an argument to the impalad daemon. In the
future it will also enable krpc for the catalogd and statestored
daemons.

This change also adds a flag "--test_krpc" to pytest. When running tests
using "impala-py.test --test_krpc", the test cluster will be started
by passing "--use_krpc" to start-impala-cluster.py (see above).

This change also adds a SkipIf to skip tests based on whether the
cluster was started with krpc support or not.

- SkipIf.not_krpc can be used to mark a test that depends on krpc.
- SkipIf.not_thrift can be used to mark a test that depends on Thrift
  RPC.

This change adds a meta test to make sure that the new SkipIf decorators
work correctly. The test should be removed as soon as real tests have
been added with the new decorators.

Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
---
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
A tests/common/test_skip.py
M tests/conftest.py
M tests/custom_cluster/test_krpc_mem_usage.py
7 files changed, 81 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/9291/4
--
To view, visit http://gerrit.cloudera.org:8080/9291
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie01a5de2afac4a0f43d5fceff283f6108ad6a3ab
Gerrit-Change-Number: 9291
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil