[ 
https://issues.apache.org/jira/browse/HBASE-18201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16569931#comment-16569931
 ] 

Reid Chan commented on HBASE-18201:
-----------------------------------

lgtm, overall.
You may need rebase again, i tried {{git apply}} with some conflicts (docs 
parts).

Nit.
Add a blank new line between
{code}
70 +  private List<Boolean> isTagsLenZero = new ArrayList<>();
71    /**
{code}
{code} //check if HFile use Tag. {code}
blank between `//` and `check`, and pay attention to properly use upper case, 
like `C` in this case. There're few other places, please check.

Naming: {code}itIsTagsLenZero > it // or iterator is better{code}
Question:
1. Is {{meta.getDataBlockEncoding()}} equals {{this.encoding}} (it looks so to 
me). If so, {{this.encoding = encoding}} can be removed IMO.
2. Can check other available Compression.Algorithm as well? (not only GZ).




> add UT and docs for DataBlockEncodingTool
> -----------------------------------------
>
>                 Key: HBASE-18201
>                 URL: https://issues.apache.org/jira/browse/HBASE-18201
>             Project: HBase
>          Issue Type: Sub-task
>          Components: tooling
>            Reporter: Chia-Ping Tsai
>            Assignee: Kuan-Po Tseng
>            Priority: Minor
>              Labels: beginner
>         Attachments: HBASE-18201.master.001.patch, 
> HBASE-18201.master.002.patch, HBASE-18201.master.002.patch, 
> HBASE-18201.master.003.patch, HBASE-18201.master.004.patch, 
> HBASE-18201.master.005.patch, HBASE-18201.master.005.patch, 
> HBASE-18201.master.005.patch
>
>
> There is no example, documents, or tests for DataBlockEncodingTool. We should 
> have it friendly if any use case exists. Otherwise, we should just get rid of 
> it because DataBlockEncodingTool presumes that the implementation of cell 
> returned from DataBlockEncoder is KeyValue. The presume may obstruct the 
> cleanup of KeyValue references in the code base of read/write path.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to