Bankim Bhavsar 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. > If I were you I'd fully kudu-ify the code - i.e. prune out unnecessary stuf Thanks Adar, Alexey and Tim for the feedback. This is what I plan to do: - Don't create a separate impala/util sub-directory and instead import the BloomFilter into kudu/util. Rename it to BlockBloomFilter under kudu namespace to distinguish from the existing BloomFilter in kudu/util. - For the buffer pool stuff, incorporate the suggestion from Todd of using an interface that allocates/de-allocates memory and add a default implementation that uses malloc/posix_memalign. This way it'll be easier to consume the BF in Impala from kudu-util. - Use existing utilities from gutil for cpu info and pretty printer as pointed out by Alexey and Adar. So no need to those from Impala. - Like in this change, BF will not depend on any generated code and not implement any Thrift/Protobuf related methods. -- 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 <ban...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 19 Nov 2019 22:44:28 +0000 Gerrit-HasComments: Yes