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

Reply via email to