You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Ho (Code Review)" <ge...@cloudera.org> on 2018/02/28 19:52:37 UTC

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default 

Michael Ho has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9461


Change subject: IMPALA-2567: Enable KRPC by default
......................................................................

IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
---
M be/src/runtime/exec-env.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
4 files changed, 6 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/9461/2
-- 
To view, visit http://gerrit.cloudera.org:8080/9461
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9461/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/9461/2/be/src/runtime/exec-env.cc@83
PS2, Line 83: Used to indicate whether to use KRPC for the "
            :     "DataStream subsystem, or the Thrift RPC layer instead.
This could be clearer:

If true, use KRPC for the DataStream subsystem. Otherwise use Thrift RPC.


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

http://gerrit.cloudera.org:8080/#/c/9461/2/bin/run-all-tests.sh@70
PS2, Line 70: if [[ "${TEST_KRPC}" != "false" ]]; then
            :   TEST_START_CLUSTER_ARGS="${TEST_START_CLUSTER_ARGS} --use_krpc"
            : fi
and this


http://gerrit.cloudera.org:8080/#/c/9461/2/bin/run-all-tests.sh@124
PS2, Line 124: if [[ "${TEST_KRPC}" != "false" ]]; then
             :   COMMON_PYTEST_ARGS+=" --test_krpc"
             : fi
I think this needs to be adjusted (so we can test with KRPC disabled)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Mar 2018 17:49:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Dan Hecht, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................

IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Also bumped the memory limit for a query in test_row_filters().
It seems to fail consistently in exhastive build after recent
scanner memory changes.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
---
M be/src/runtime/exec-env.cc
M be/src/runtime/krpc-data-stream-mgr.cc
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
M tests/common/test_skip.py
M tests/conftest.py
8 files changed, 25 insertions(+), 26 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 5
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Dan Hecht, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................

IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
---
M be/src/runtime/exec-env.cc
M be/src/runtime/krpc-data-stream-mgr.cc
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
M tests/common/test_skip.py
M tests/conftest.py
8 files changed, 25 insertions(+), 26 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 4:

Hold off from merging until the builds are in better shape.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 03 Mar 2018 00:04:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 2:

Once this is checked in, please verify that impala is configured with krpc on/off correctly for the various classes of jenkins jobs.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Mar 2018 17:51:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 2: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Wed, 28 Feb 2018 20:20:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

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/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................

IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Reviewed-on: http://gerrit.cloudera.org:8080/9461
Reviewed-by: Michael Ho <kw...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/exec-env.cc
M be/src/runtime/krpc-data-stream-mgr.cc
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
M tests/common/test_skip.py
M tests/conftest.py
8 files changed, 25 insertions(+), 26 deletions(-)

Approvals:
  Michael Ho: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 6: Code-Review+2

Rebased and reverted bumping the memory limit in test_row_filters() as that's not necessary anymore.

Carry Dan's +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Mar 2018 05:10:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Mar 2018 08:57:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 2:

(3 comments)

The existing jenkins jobs to test with KRPC enabled will be replaced with tests with Thrift enabled.

http://gerrit.cloudera.org:8080/#/c/9461/2/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:

http://gerrit.cloudera.org:8080/#/c/9461/2/be/src/runtime/exec-env.cc@83
PS2, Line 83: Used to indicate whether to use KRPC for the "
            :     "DataStream subsystem, or the Thrift RPC layer instead.
> This could be clearer:
Done


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

http://gerrit.cloudera.org:8080/#/c/9461/2/bin/run-all-tests.sh@70
PS2, Line 70: if [[ "${TEST_KRPC}" != "false" ]]; then
            :   TEST_START_CLUSTER_ARGS="${TEST_START_CLUSTER_ARGS} --use_krpc"
            : fi
> and this
Done


http://gerrit.cloudera.org:8080/#/c/9461/2/bin/run-all-tests.sh@124
PS2, Line 124: if [[ "${TEST_KRPC}" != "false" ]]; then
             :   COMMON_PYTEST_ARGS+=" --test_krpc"
             : fi
> I think this needs to be adjusted (so we can test with KRPC disabled)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Mar 2018 18:59:30 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Mar 2018 23:18:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Dan Hecht, 

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

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

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

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................

IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Also bumped the memory limit for a query in test_row_filters().
It seems to fail consistently in exhastive build after recent
scanner memory changes.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
---
M be/src/runtime/exec-env.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
M tests/common/test_skip.py
M tests/conftest.py
9 files changed, 26 insertions(+), 27 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 4:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9461/3/bin/start-impala-cluster.py@57
PS3, Line 57: , help
> thrift is always enabled. How about "Enable Thrift DataStream service ..."
Done


http://gerrit.cloudera.org:8080/#/c/9461/3/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9461/3/tests/common/test_skip.py@35
PS3, Line 35:     assert not pytest.config.option.test_no_krpc
            : 
            :   @SkipIf.not_thrift
            :   def test_skip_if_not_thrift(self):
            :     assert pytest.config.option.test_no_krpc
> these look backwards. don't you have to move the 'not'?
Oops. Fixed.


http://gerrit.cloudera.org:8080/#/c/9461/3/tests/conftest.py
File tests/conftest.py:

http://gerrit.cloudera.org:8080/#/c/9461/3/tests/conftest.py@120
PS3, Line 120: all tests with
> Thrift DataStream service enabled.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Mar 2018 22:11:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 3:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/9461/3/bin/start-impala-cluster.py@57
PS3, Line 57: Thrift
thrift is always enabled. How about "Enable Thrift DataStream service ..."
Also, flag effect would probably be clearer if named --disable_krpc


http://gerrit.cloudera.org:8080/#/c/9461/3/tests/common/test_skip.py
File tests/common/test_skip.py:

http://gerrit.cloudera.org:8080/#/c/9461/3/tests/common/test_skip.py@35
PS3, Line 35:     assert pytest.config.option.test_thrift
            : 
            :   @SkipIf.not_thrift
            :   def test_skip_if_not_thrift(self):
            :     assert not pytest.config.option.test_thrift
these look backwards. don't you have to move the 'not'?


http://gerrit.cloudera.org:8080/#/c/9461/3/tests/conftest.py
File tests/conftest.py:

http://gerrit.cloudera.org:8080/#/c/9461/3/tests/conftest.py@120
PS3, Line 120: Thrift enabled
Thrift DataStream service enabled.
Or "KRPC disabled".
Whichever is consistent with what you decide in the other place.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Mar 2018 19:08:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Mar 2018 22:13:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Dan Hecht, 

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

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

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

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................

IMPALA-2567: Enable KRPC by default

This change enables the switch to use KRPC by default.
This change also fixes a bug in KrpcDataStreamMgr to
check if maintenance thread was started before calling
Join() on it. This shows up in BE tests as the maintenance
thread isn't started in them.

Also bumped the memory limit for a query in test_row_filters().
It seems to fail consistently in exhastive build after recent
scanner memory changes.

Testing done: exhaustive build.

Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
---
M be/src/runtime/exec-env.cc
M be/src/runtime/krpc-data-stream-mgr.cc
M bin/run-all-tests.sh
M bin/start-impala-cluster.py
M testdata/workloads/functional-query/queries/QueryTest/runtime_row_filters.test
M tests/common/custom_cluster_test_suite.py
M tests/common/skip.py
M tests/common/test_skip.py
M tests/conftest.py
9 files changed, 26 insertions(+), 27 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-2567: Enable KRPC by default

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9461 )

Change subject: IMPALA-2567: Enable KRPC by default
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae736c1c1351758969b4d84e34fc5b2d048660a0
Gerrit-Change-Number: 9461
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Mar 2018 05:11:30 +0000
Gerrit-HasComments: No