Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14183 )

Change subject: IMPALA-8830: Fix executor group assignment of coordinator only 
queries
......................................................................


Patch Set 7:

(2 comments)

This looks good overall, had minor comments only.

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/cluster-membership-mgr.h@212
PS7, Line 212:   static const ExecutorGroup empty_exec_group_;
I don't feel too strongly about this, but it might be better in some ways to 
have this be a field of ClusterMembershipMgr, so that we don't run the static 
destructors, etc when the process shuts down.


http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:

http://gerrit.cloudera.org:8080/#/c/14183/7/be/src/scheduling/scheduler.cc@558
PS7, Line 558:   VLOG_QUERY << "Exec at coord is " << (exec_at_coord ? "true" : 
"false");
While we're here, can we make this VLOG(2) or equivalent? It is kinda spammy 
when I look at logs.



--
To view, visit http://gerrit.cloudera.org:8080/14183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 7
Gerrit-Owner: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Tue, 16 Jun 2020 00:47:55 +0000
Gerrit-HasComments: Yes

Reply via email to