Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/16369 )
Change subject: IMPALA-9636: Don't run retried query on the blacklisted nodes ...................................................................... Patch Set 16: (2 comments) http://gerrit.cloudera.org:8080/#/c/16369/16/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/16369/16/be/src/scheduling/admission-controller.cc@1557 PS16, Line 1557: bool all_blacklisted; > why do you need this? can't you just check if 'NumExecutors() == 0'? The function NumExecutors() need to go through the map to find the total number of executors. By using all_blacklisted variable, we can let GetFilteredExecutorGroup() to call NumExecutors() if needed. If no executor to be filtered, then we don't need to call NumExecutors(). http://gerrit.cloudera.org:8080/#/c/16369/16/be/src/scheduling/executor-group.cc File be/src/scheduling/executor-group.cc: http://gerrit.cloudera.org:8080/#/c/16369/16/be/src/scheduling/executor-group.cc@52 PS16, Line 52: if (!blacklisted_executor_found) return nullptr; > I think it a bit confusing to return nullptr if either (1) nothing was filt I guess that in most cases the executor, which cause first attempt failed, is still in blacklist and not in executor group so it's most likely nothing to be filtered out. By return nullptr, it avoid to create a copy of executor group and reduce the cost. -- To view, visit http://gerrit.cloudera.org:8080/16369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00bc1b5026efbd0670ffbe57bcebc457d34cb105 Gerrit-Change-Number: 16369 Gerrit-PatchSet: 16 Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Mon, 14 Sep 2020 23:32:57 +0000 Gerrit-HasComments: Yes
