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 <tw...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 04 Oct 2017 00:05:07 +0000 Gerrit-HasComments: Yes
