Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3610: Account for memory used by filters in the coordinator ......................................................................
Patch Set 9: (4 comments) http://gerrit.cloudera.org:8080/#/c/4066/9/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: Line 718: // Cast the const away to read non-const member functions. No modification is done. > I can't call non-const functions otherwise, specifically state.targets() an then why not remove the 'const' in the for? Line 2048: for (auto& filter: filter_routing_table_) { > I don't understad, versus? this is an iterator, no? Line 2107: state->set_completion_time(query_events_->ElapsedTime()); > This has to be set only once we are sure that we aren't going to wait for a that's right, and FilterState has all the information it needs to do this correctly. Line 2119: if (state->desc().is_broadcast_join) { > That would cause broadcast filters having to swap() twice, which is two tim swap() means there's no copy (for the string field). -- To view, visit http://gerrit.cloudera.org:8080/4066 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c52c8a1c2e79ef370c77bf264885fc859678d1b Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
