You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Zach York (JIRA)" <ji...@apache.org> on 2017/06/14 22:31:00 UTC

[jira] [Commented] (HBASE-18119) Improve HFile readability and modify ChecksumUtil log level

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

Zach York commented on HBASE-18119:
-----------------------------------

[~Qilin Cao] The code simplification makes sense to me. 
A few things:

Why change that message to trace? What is the rationale?

bq. +    HFile.checkHFileVersion(conf);
bq. +    conf.setInt(HFile.FORMAT_VERSION_KEY, HFile.MIN_FORMAT_VERSION);
bq. +    HFile.checkHFileVersion(conf);

This test could pass erroneously. If the first checkHFileVersion threw an IllegalArgumentException, the test would still pass even though this would be incorrect. Perhaps split it into two tests (1 that shows it works normally and another one that has the exception thrown).



> Improve HFile readability and modify ChecksumUtil log level
> -----------------------------------------------------------
>
>                 Key: HBASE-18119
>                 URL: https://issues.apache.org/jira/browse/HBASE-18119
>             Project: HBase
>          Issue Type: Improvement
>    Affects Versions: 2.0.0
>            Reporter: Qilin Cao
>            Assignee: Qilin Cao
>            Priority: Minor
>         Attachments: HBASE-18119-v1.patch
>
>
> It is confused when I read the HFile.checkHFileVersion method, so I change the if expression. Simultaneously, I change ChecksumUtil the info log level to trace.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)