impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tianyi Wang (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5789: Add always false flag in bloom filter
Date Wed, 04 Oct 2017 00:05:07 GMT
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/8170 )

Change subject: IMPALA-5789: Add always_false flag in bloom filter
......................................................................


Patch Set 1:

(3 comments)

> Patch Set 1:
> 
> (2 comments)
> 
> Will wait for the new PS but had a couple of high-level bits of feedback first. It may
be worth looking at Thomas' patches if you haven't already to make sure your changes are compatible.

Thanks for the tips. I'm sure rabasing on Thomas's patches is not trivial.

http://gerrit.cloudera.org:8080/#/c/8170/1/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/8170/1/be/src/exec/hdfs-scanner.cc@104
PS1, Line 104:     if (FilterContext::HasAlwaysFalseInList(FilterStats::SPLITS_KEY,
> Not sure if this is relevant but there is a known problem with filtering ou
Yeah just found it hanged on Jenkins for 4 days though the tests passed locally. I plan to
disable this part of code with sequence scanner.


http://gerrit.cloudera.org:8080/#/c/8170/1/be/src/runtime/runtime-filter-bank.cc
File be/src/runtime/runtime-filter-bank.cc:

http://gerrit.cloudera.org:8080/#/c/8170/1/be/src/runtime/runtime-filter-bank.cc@203
PS1, Line 203:   memory_allocated_->Add(required_space);
The accounting is changed to required_space (not space used) here.


http://gerrit.cloudera.org:8080/#/c/8170/1/be/src/util/bloom-filter.h
File be/src/util/bloom-filter.h:

http://gerrit.cloudera.org:8080/#/c/8170/1/be/src/util/bloom-filter.h@106
PS1, Line 106:     if (always_false_) return 0;
> I think this change will mess up some accounting in RuntimeFilterBank.
OK. But see comment in runtime-filter-bank.cc.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If680240a3cd4583fc97c3192177d86d9567c4f8d
Gerrit-Change-Number: 8170
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <twang@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Oct 2017 00:05:07 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message