Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8971 )

Change subject: IMPALA-5519: Allocate fragment's runtime filter memory from 
Buffer pool
......................................................................


Patch Set 8:

(5 comments)

Did you look at the tests that Alex mentioned for creating a test table with 
different cardinalities and nvds?

http://gerrit.cloudera.org:8080/#/c/8971/8/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java
File fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java:

http://gerrit.cloudera.org:8080/#/c/8971/8/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@95
PS8, Line 95:   private static final long MIN_BLOOM_FILTER_SIZE = 4 * 1024;
            :   private static final long MAX_BLOOM_FILTER_SIZE = 512 * 1024 * 
1024;
> the query option restrictions already take care of this limit, by making su
You may remove them and add a check instead.


http://gerrit.cloudera.org:8080/#/c/8971/8/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@109
PS8, Line 109: _
nit: no need for '_' when the fields are public


http://gerrit.cloudera.org:8080/#/c/8971/8/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@125
PS8, Line 125: bloomFilterSizeLimits_.max_ = 
tQueryOptions.getRuntime_filter_max_size();
             :     bloomFilterSizeLimits_.max_ = 
Math.max(bloomFilterSizeLimits_.max_,
             :         MIN_BLOOM_FILTER_SIZE);
             :     bloomFilterSizeLimits_.max_ = 
Math.max(bloomFilterSizeLimits_.max_,
             :         BackendConfig.INSTANCE.getMinBufferSize());
             :     bloomFilterSizeLimits_.max_ = Math.min(
             :         BitUtil.roundUpToPowerOf2(bloomFilterSizeLimits_.max_), 
MAX_BLOOM_FILTER_SIZE);
You may want to see if it makes sense to add this logic to FilterSizeLimits 
class. Something like setMax(), setMin(), setDefault() functions.


http://gerrit.cloudera.org:8080/#/c/8971/8/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@482
PS8, Line 482: sizeLimits
'filterSizeLimits'


http://gerrit.cloudera.org:8080/#/c/8971/8/fe/src/main/java/org/apache/impala/planner/RuntimeFilterGenerator.java@494
PS8, Line 494:       filterSizeBytes_ = Math.min(filterSizeBytes_, 
filterSizeLimits.max_);
There is always the case that the size needed to achieve the desired fp rate is 
higher than the max limit size. In that case we should add a log message.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iea2759665fb2e8bef9433014a8d42a7ebf99ce1f
Gerrit-Change-Number: 8971
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Feb 2018 18:12:07 +0000
Gerrit-HasComments: Yes

Reply via email to