Sailesh Mukil has posted comments on this change.

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


Patch Set 2: Code-Review+1

(2 comments)

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

PS2, Line 2035: rather than copy it and take double the memory
              :       // cost
Isn't this an issue for non broadcast join filters too though?

At least there we've reserved memory for one of the filters, and I'm okay with 
how it is now if there's no easy way to track the memory.


Line 2039:       TBloomFilter* filter = 
&(const_cast<TBloomFilter&>(params.bloom_filter));
Good call. Maybe it's worth adding the following in L2038 too:
DCHECK_EQ(state->bloom_filter.get(), NULL)


-- 
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: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to