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 12: (7 comments) http://gerrit.cloudera.org:8080/#/c/16369/11/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/16369/11/be/src/scheduling/admission-controller.cc@1573 PS11, Line 1573: const Schedule > Probably should also report error when !created. Some of the code from line If returned status is not OK, the function will return in line 1583. This part of code was changed in patch 12. http://gerrit.cloudera.org:8080/#/c/16369/12/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/16369/12/be/src/scheduling/admission-controller.cc@1544 PS12, Line 1544: balance queries across executor groups more evenly > This probably is a high priority enhancement. Currently there is only one executor group. We cannot address it in this patch. http://gerrit.cloudera.org:8080/#/c/16369/12/be/src/scheduling/admission-controller.cc@1568 PS12, Line 1568: return Status::OK(); > Seems this is not right, since we are in the loop at line 1545 and are disc Moved the code after the loop. http://gerrit.cloudera.org:8080/#/c/16369/12/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/16369/12/be/src/service/client-request-state.cc@191 PS12, Line 191: void ClientRequestState::AddBlacklistedExecutorAddress(const NetworkAddressPB& addr) { : blacklisted_executor_addresses_.emplace(addr); : } > Can this function be inlined? Fixed. http://gerrit.cloudera.org:8080/#/c/16369/12/tests/custom_cluster/test_query_retries.py File tests/custom_cluster/test_query_retries.py: http://gerrit.cloudera.org:8080/#/c/16369/12/tests/custom_cluster/test_query_retries.py@298 PS12, Line 298: make > nit: makes Fixed http://gerrit.cloudera.org:8080/#/c/16369/12/tests/custom_cluster/test_query_retries.py@381 PS12, Line 381: ..." > May be useful to list the reason and detail here. Added in assert statement below. http://gerrit.cloudera.org:8080/#/c/16369/12/tests/custom_cluster/test_query_retries.py@386 PS12, Line 386: Admission for query exceeded timeout 60000ms in pool default-pool. > May be useful to check the reason and detail here. added "Queued reason" and "Additional Details" -- 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: 12 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: Fri, 11 Sep 2020 16:30:22 +0000 Gerrit-HasComments: Yes
