Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21383 )
Change subject: IMPALA-13040: Add waiting mechanism in UpdateFilterFromRemote ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/scheduling/scheduler.cc File be/src/scheduling/scheduler.cc: http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/scheduling/scheduler.cc@341 PS4, Line 341: if (UNLIKELY(FLAGS_sort_runtime_filter_aggregator_candidates)) { > Would it be worthwhile to wrap this condition with UNLIKELY since the flag Done http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/service/data-stream-service.cc@159 PS4, Line 159: } else if (qs->is_initialized()) { > Is this condition the more likely to happen? If so, please make it the if It is more likely, but if IsTerminalState() is true, there is no need to process filter update at all. Therefore, it is checked first in the earlier branch. Removed IsCancelled() check since IsTerminalState() already include BackendExecState::CANCELLED checking. http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h File be/src/util/network-util.h: http://gerrit.cloudera.org:8080/#/c/21383/4/be/src/util/network-util.h@124 PS4, Line 124: if (lhs.has_uds_address()) { > What if only one has_uds_address? Seems like they should be unequal then. Done. -- To view, visit http://gerrit.cloudera.org:8080/21383 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I156d1f0c694b91ba34be70bc53ae9bacf924b3b9 Gerrit-Change-Number: 21383 Gerrit-PatchSet: 5 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Wed, 08 May 2024 20:17:36 +0000 Gerrit-HasComments: Yes