impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3610: Account for memory used by filters in the coordinator
Date Wed, 31 Aug 2016 21:00:41 GMT
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-3610: Account for memory used by filters in the coordinator
......................................................................


Patch Set 11: Code-Review+2

(6 comments)

Carry +2

http://gerrit.cloudera.org:8080/#/c/4066/10/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

Line 2110:     DCHECK_EQ(state->bloom_filter(), reinterpret_cast<TBloomFilter*>(NULL));
> move this todo
Done


http://gerrit.cloudera.org:8080/#/c/4066/11/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

Line 281: 
> you can remove all blank lines between the getters
Done


Line 380:     // TODO-MT: remove
> Can you remove this comment?
Done


Line 380:     // TODO-MT: remove
> there shouldn't be references to any query-related state when the QueryStat
The scanner threads would not know if an error has been hit on the main thread, so they would
still be accessing query state.


Line 2098:       // Complete partitioned or broadcast join filter case.
> "complete filter case"
Done


Line 2133:   //if (desc_.is_broadcast_join) return;
> remove
Done


-- 
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: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message