You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2019/03/20 02:04:17 UTC

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

Hello Impala Public Jenkins,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................

IMPALA-6031: Fix executor node count in distributed plans

Prior to this change, the planner also considered coordinator-only
nodes as executors while estimating the number of scan nodes to be
used in the distributed plan. This change ensures that only
executor nodes are considered for that estimation.

Testing:
Added a new custom cluster test to verify the same.

Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Reviewed-on: http://gerrit.cloudera.org:8080/10873
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
R fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M tests/custom_cluster/test_coordinators.py
11 files changed, 104 insertions(+), 63 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

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

Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................


Patch Set 1:

This is a clean pick.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 02:26:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

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

Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3934/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 04:40:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

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

Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 15:32:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

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

Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2474/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 02:44:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12801 )

Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................

IMPALA-6031: Fix executor node count in distributed plans

Prior to this change, the planner also considered coordinator-only
nodes as executors while estimating the number of scan nodes to be
used in the distributed plan. This change ensures that only
executor nodes are considered for that estimation.

Testing:
Added a new custom cluster test to verify the same.

Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Reviewed-on: http://gerrit.cloudera.org:8080/10873
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/12801
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M be/src/service/frontend.cc
M be/src/service/frontend.h
M be/src/service/impala-server.cc
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
R fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M tests/custom_cluster/test_coordinators.py
11 files changed, 104 insertions(+), 63 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Tim Armstrong: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

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

Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2019 08:37:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6031: Fix executor node count in distributed plans

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

Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/12801/1/tests/custom_cluster/test_coordinators.py
File tests/custom_cluster/test_coordinators.py:

http://gerrit.cloudera.org:8080/#/c/12801/1/tests/custom_cluster/test_coordinators.py@255
PS1, Line 255: u
flake8: E128 continuation line under-indented for visual indent


http://gerrit.cloudera.org:8080/#/c/12801/1/tests/custom_cluster/test_coordinators.py@260
PS1, Line 260: w
flake8: F841 local variable 'worker1' is assigned to but never used


http://gerrit.cloudera.org:8080/#/c/12801/1/tests/custom_cluster/test_coordinators.py@261
PS1, Line 261: w
flake8: F841 local variable 'worker2' is assigned to but never used


http://gerrit.cloudera.org:8080/#/c/12801/1/tests/custom_cluster/test_coordinators.py@274
PS1, Line 274: "
flake8: E128 continuation line under-indented for visual indent


http://gerrit.cloudera.org:8080/#/c/12801/1/tests/custom_cluster/test_coordinators.py@275
PS1, Line 275: "
flake8: E128 continuation line under-indented for visual indent


http://gerrit.cloudera.org:8080/#/c/12801/1/tests/custom_cluster/test_coordinators.py@279
PS1, Line 279: "
flake8: E128 continuation line under-indented for visual indent



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 12801
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Mar 2019 02:07:28 +0000
Gerrit-HasComments: Yes