Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18093 )
Change subject: IMPALA-11033: Add support for specifying multiple executor group sets ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/18093/2/be/src/scheduling/cluster-membership-mgr.cc File be/src/scheduling/cluster-membership-mgr.cc: http://gerrit.cloudera.org:8080/#/c/18093/2/be/src/scheduling/cluster-membership-mgr.cc@685 PS2, Line 685: parsed_group_prefixes.insert(group_prefix); > For the first part of your comment: FE is sent the max size of an executor Since FE will use ExecutorMembershipSnapshot.exec_group_sets to generate several distributed plans, I would think for each group, the logic to obtain a worktable executor size is as follows. int workableExecutorSize() { return (curr_num_executors > 0) ? curr_num_executors : expected_num_executors; } Is it about right? Currently, FE iterates over exec_group_sets (which is a list) and stops generating more plans when the current group sets fit. So is it possible to arrange group sets in exec_group_sets in the order of threshold? In other words, exec_group_sets provides all the information needed by the planner. // Types of executor groups struct TExecutorGroupSet { // The current max number of executors among all healthy groups of this group set. 1: i32 curr_num_executors = 0 // The expected size of the executor groups. Can be used to plan queries when // no healthy executor groups are present(curr_num_executors is 0). 2: i32 expected_num_executors = 0 // The name of the request pool associated with this executor group type. All // executor groups that match this prefix will be included as a part of this set. // Note: this will be empty when 'default' executor group is used or // 'expected_executor_group_sets' startup flag is not specified. 3: string exec_group_name_prefix } http://gerrit.cloudera.org:8080/#/c/18093/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/18093/2/fe/src/main/java/org/apache/impala/util/ExecutorMembershipSnapshot.java@71 PS2, Line 71: exec_group_sets > Since this functionality is not used directly in this patch I deferred it t I think the only missing piece in TExecutorGroupSet is the threshold. If FE is to read it from the HDFS configuration file, then every FE on every node will do so which creates extra network pressure. A better solution is for BE to read it once and present it consistently to every FE as part of TExecutorGroupSet. -- To view, visit http://gerrit.cloudera.org:8080/18093 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e0a3a5fe2b1f0b7507b7c096b7a3c373bc2e684 Gerrit-Change-Number: 18093 Gerrit-PatchSet: 3 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Thu, 16 Dec 2021 15:43:01 +0000 Gerrit-HasComments: Yes
