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

Reply via email to