Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14745 )
Change subject: Import Impala's blocked based BloomFilter ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14745/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14745/1//COMMIT_MSG@14 PS1, Line 14: : Created a separate sub-directory kudu/impala/util : and retained impala namespace to distinguish : between existing kudu BloomFilter and this one. > Since Impala consumes kudu_util, would it be possible to incorporate the Im I'd also suggest just making it a first-class citizen in Kudu's util directory. In some ways it would be nice to generalise it enough that it could be consumed by impala, but I suspect it's more work overall to maintain one general-purpose implementation vs two special-purpose implementations, since the core logic of the bloom filter should be low-churn, but the integrations with Impala query execution are more subject to change, e.g. this patch https://gerrit.cloudera.org/#/c/14538/. The buffer pool and thrift/protobuf integration on the Impala side, in particular, would require work to abstract and just not sure it's worth it. If we did want to share things, I kinda wonder if it would be better to just have static functions that implement the core algorithms (insert, find, or, maxndv, minlogspace, falsepositiveprob) on raw arrays. -- To view, visit http://gerrit.cloudera.org:8080/14745 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89c54a051c5093cf5fb81481a47a0a6677d7d906 Gerrit-Change-Number: 14745 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 19 Nov 2019 21:32:01 +0000 Gerrit-HasComments: Yes
