You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org> on 2018/07/05 22:00:50 UTC
[Impala-ASF-CR] IMPALA-6031: Distributed plan describes coordinator-only nodes as scanning
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10873
Change subject: IMPALA-6031: Distributed plan describes coordinator-only nodes as scanning
......................................................................
IMPALA-6031: Distributed plan describes coordinator-only nodes as scanning
The MembershipCallback function of ImpalaServer class was modified
to only send details of executor nodes to the frontend. Thus, while
generating scan plans, the frontend would not assign fragments to
coordinator-only nodes. The MembershipSnapshot class was renamed to
ExecutorMembershipSnapshot for better semantics.
Testing:
Added a new custom_cluster test where one impalad is a
coordinator-only node. The test verifies that the scan fragments
are assigned only to the executors.
Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
---
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, 82 insertions(+), 44 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/10873/2
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 07 Jul 2018 00:23:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] 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/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 8: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 8
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 07 Jul 2018 03:33:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] 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/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2782/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 8
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 07 Jul 2018 00:23:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Bikramjeet Vig,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10873
to look at the new patch set (#7).
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
---
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, 103 insertions(+), 62 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/10873/7
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 5: Code-Review+2
(4 comments)
lgtm, only a few more nits
http://gerrit.cloudera.org:8080/#/c/10873/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10873/5//COMMIT_MSG@9
PS5, Line 9: Prior to this change, the backend would send the frontend a set of
: all nodes on the cluster. This resulted in scan fragments being
: assigned to coordinator only nodes. In this change, the
: MembershipCallback function of ImpalaServer class was modified to
: only send details of executor nodes to the frontend.
this makes it sound like the problem you were fixing was the backend sending all nodes to FE. maybe use something like:
"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."
http://gerrit.cloudera.org:8080/#/c/10873/5//COMMIT_MSG@16
PS5, Line 16: custom_cluster
nit: custom cluster test
http://gerrit.cloudera.org:8080/#/c/10873/5/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:
http://gerrit.cloudera.org:8080/#/c/10873/5/common/thrift/Frontend.thrift@723
PS5, Line 723: needed when multiple impalads run
: // on the same machine.
nit: "needed when" implies that it is only needed for that case. Can you reword it to something like you had earlier: "needed since there can be multiple impalads running on a single host."
http://gerrit.cloudera.org:8080/#/c/10873/5/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
File fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java:
http://gerrit.cloudera.org:8080/#/c/10873/5/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@30
PS5, Line 30:
nit: extra space
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 22:27:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 7:
> Patch Set 6:
>
> (1 comment)
Sorry, changed it now.
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 23:45:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
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/10873 )
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, 103 insertions(+), 62 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 9
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6031: Distributed plan describes coordinator-only nodes as scanning
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/10873 )
Change subject: IMPALA-6031: Distributed plan describes coordinator-only nodes as scanning
......................................................................
Patch Set 2:
(11 comments)
lgtm, just a few nits
http://gerrit.cloudera.org:8080/#/c/10873/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10873/2//COMMIT_MSG@7
PS2, Line 7: Distributed plan describes coordinator-only nodes as scanning
the title should usually describe what this patch does/fixed
http://gerrit.cloudera.org:8080/#/c/10873/2//COMMIT_MSG@9
PS2, Line 9: The MembershipCallback function of ImpalaServer class was modified
: to only send details of executor nodes to the frontend. Thus, while
: generating scan plans, the frontend would not assign fragments to
: coordinator-only nodes. The MembershipSnapshot class was renamed to
: ExecutorMembershipSnapshot for better semantics.
you dont need to describe all the changes in the commit message. Just describing the problem, its high level solution, things like that should be good enough. On the other hand you can make a call on weather some specific change to the code needs to be highlighted in the commit message.
http://gerrit.cloudera.org:8080/#/c/10873/2//COMMIT_MSG@16
PS2, Line 16: Added a new custom_cluster test where one impalad is a
: coordinator-only node. The test verifies that the scan fragments
: are assigned only to the executors.
again, you dont need to be verbose while explaining this. You can just write something like: "Added a new custom cluster test to verify the same"
http://gerrit.cloudera.org:8080/#/c/10873/2/be/src/service/frontend.h
File be/src/service/frontend.h:
http://gerrit.cloudera.org:8080/#/c/10873/2/be/src/service/frontend.h@48
PS2, Line 48: cluster membership snapshot
nit: cluster membership snapshot of executors
http://gerrit.cloudera.org:8080/#/c/10873/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/10873/2/be/src/service/impala-server.cc@1674
PS2, Line 1674: // Send the hostname and ip_address to the frontend only for executor nodes.
nit: can probably incorporate this into the comment on L1669
http://gerrit.cloudera.org:8080/#/c/10873/2/be/src/service/impala-server.cc@1678
PS2, Line 1678: Increment the number of executor nodes.
nit: superfluous comment.
http://gerrit.cloudera.org:8080/#/c/10873/2/be/src/service/impala-server.cc@1678
PS2, Line 1678: We need to track the number of
: // executors because there may be multiple impalads running on a single host.
we can probably move this comment to the thrift definition of TUpdateMembershipRequest in Frontend.thrift
http://gerrit.cloudera.org:8080/#/c/10873/2/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:
http://gerrit.cloudera.org:8080/#/c/10873/2/common/thrift/Frontend.thrift@714
PS2, Line 714: // Sent from the impalad BE to FE with the latest cluster membership snapshot resulting
: // from the Membership heartbeat.
update comment to mention executor membership is only updated.
http://gerrit.cloudera.org:8080/#/c/10873/2/common/thrift/Frontend.thrift@716
PS2, Line 716: TUpdateMembershipRequest
would be a pretty big name but just to be consistent, how about we rename this to TUpdateExecutorMembershipRequest
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1153
PS2, Line 1153: cluster
nit: executorNodes
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
File fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java:
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@28
PS2, Line 28: * Singleton class that represents a snapshot of the Impalad cluster membership. Host
update comment to mention cluster membership of only executor nodes
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 00:33:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] 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/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 8
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 07 Jul 2018 00:23:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/10873 )
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 backend would send the frontend a set of
all nodes on the cluster. This resulted in scan fragments being
assigned to coordinator only nodes. In this change, the
MembershipCallback function of ImpalaServer class was modified to
only send details of executor nodes to the frontend.
Testing:
Added a new custom_cluster to verify the same.
Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
---
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, 102 insertions(+), 61 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/10873/5
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] 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/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 22:07:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/10873 )
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 to verify the same.
Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
---
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, 103 insertions(+), 62 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/10873/6
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6031: Distributed plan describes coordinator-only nodes as scanning
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10873 )
Change subject: IMPALA-6031: Distributed plan describes coordinator-only nodes as scanning
......................................................................
Patch Set 2:
(7 comments)
Change looks good to me - we discussed the approach earlier. Just have some cleanup suggestions to make it easier for future people to read.
http://gerrit.cloudera.org:8080/#/c/10873/2/common/thrift/Frontend.thrift
File common/thrift/Frontend.thrift:
http://gerrit.cloudera.org:8080/#/c/10873/2/common/thrift/Frontend.thrift@715
PS2, Line 715: // from the Membership heartbeat.
Briefly mention that it only includes executors?
http://gerrit.cloudera.org:8080/#/c/10873/2/common/thrift/Frontend.thrift@719
PS2, Line 719: num_executor_nodes
num_executors.
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java
File fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java:
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@28
PS2, Line 28: * Singleton class that represents a snapshot of the Impalad cluster membership. Host
Briefly mention that it only contains executors.
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@34
PS2, Line 34: MembershipSnapshot
ExecutorMembershipSnapshot
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@49
PS2, Line 49: MembershipSnapshot
ExecutorMembershipSnapshot
http://gerrit.cloudera.org:8080/#/c/10873/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@54
PS2, Line 54: numExecutorNodes_
Maybe numExecutors_?
http://gerrit.cloudera.org:8080/#/c/10873/2/tests/custom_cluster/test_coordinators.py
File tests/custom_cluster/test_coordinators.py:
http://gerrit.cloudera.org:8080/#/c/10873/2/tests/custom_cluster/test_coordinators.py@274
PS2, Line 274: "where id NOT IN (0,1,2) and string_col IN ('aaaa', 'bbbb', 'cccc', NULL) "\
nit: continuations might not be needed because this is in parentheses
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 00:19:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6031: Fix executor node count in distributed plans
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/10873 )
Change subject: IMPALA-6031: Fix executor node count in distributed plans
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10873/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10873/5//COMMIT_MSG@16
PS5, Line 16:
> Done
forgot this
--
To view, visit http://gerrit.cloudera.org:8080/10873
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44af6b40099a495e13a0a5dc72c491d486d23aa8
Gerrit-Change-Number: 10873
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 23:43:56 +0000
Gerrit-HasComments: Yes