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 <pooja.nilange...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 06 Jul 2018 22:27:33 +0000 Gerrit-HasComments: Yes
