helifu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11722 )

Change subject: KUDU-2038: Support bitmap indexing
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11722/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11722/1//COMMIT_MSG@10
PS1, Line 10: has one bitmap info. One bitmap info is composed of two CFiles, 
one
            : for keys and the other for bitmap data. The bitmap info is 
generated
            : d
> Are the bitmap info files generated every time the main column cfile is wri
Yes, you are right.
The bitmap info files are separate blocks, just like adhoc index does. And i 
think it's easier for me to write to different cfile.


http://gerrit.cloudera.org:8080/#/c/11722/1//COMMIT_MSG@14
PS1, Line 14: The index could be created when creating or altering table, and
            : it could be dropped later.
> if it's created by altering the table, does that eagerly index all of the c
No, the generation of the bitmap info depends on the compaction operations and 
it is asynchronous process.
The index creation does not mean the index is created at the same time.
The rows in a DRS is limited, so the memory for the bitmaps is limited too.


http://gerrit.cloudera.org:8080/#/c/11722/1//COMMIT_MSG@18
PS1, Line 18: invalidate
> does the invalidation need to invalidate the index for the whole DRS? Could
I have no idea on the more fine-grained basis :(


http://gerrit.cloudera.org:8080/#/c/11722/1//COMMIT_MSG@18
PS1, Line 18: would
> nit: the commit message uses the word "would" which gives the sense that th
Yes, they have been implemented in this patch.


http://gerrit.cloudera.org:8080/#/c/11722/1//COMMIT_MSG@23
PS1, Line 23: The bitmap index supports Equality, InList, Range predicates.
> do you have any benchmark info to share on this implementation, both on the
I merged it from branch-1.4.x, and the benchmark is fascinating at the time. 
But now in master branch, it's not noteworthy, maybe there is a big improvement 
recently.
So, i will re-check the code and optimize them.


http://gerrit.cloudera.org:8080/#/c/11722/2/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:

http://gerrit.cloudera.org:8080/#/c/11722/2/src/kudu/tablet/compaction-test.cc@55
PS2, Line 55: #include "kudu/tablet/column_index_set.h"
If i removed the header, then building will fail:
/usr/include/c++/4.9/bits/unique_ptr.h:74:22: error: invalid application of 
‘sizeof’ to incomplete type ‘kudu::tablet::ColumnIndexSetReaderIterator’
  static_assert(sizeof(_Tp)>0,

And if i add the header, then the IWYU will complain:
-#include "kudu/tablet/cfile_set.h"

Is there a good way to fix it except modifying the iwyu.py?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0edaa0ef1dba2dbce85ebf15f0a731e4939a7860
Gerrit-Change-Number: 11722
Gerrit-PatchSet: 2
Gerrit-Owner: helifu <hzhel...@corp.netease.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: helifu <hzhel...@corp.netease.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 07:14:34 +0000
Gerrit-HasComments: Yes

Reply via email to