[ https://issues.apache.org/jira/browse/HBASE-5521?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231788#comment-13231788 ]
Phabricator commented on HBASE-5521: ------------------------------------ tedyu has commented on the revision "HBASE-5521 [jira] Move compression/decompression to an encoder specific encoding context". INLINE COMMENTS src/main/java/org/apache/hadoop/hbase/io/encoding/EncodedDataBlock.java:202 How about naming this method encodeData() ? src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDefaultEncodingContext.java:106 Typo: 'to start' src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDefaultEncodingContext.java:109 I think this method should be in the interface. src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockEncodingContext.java:34 When getOnDiskBytesWithHeader() is called, encoding is done already, right ? src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDefaultEncodingContext.java:131 I think this method should be in the interface. src/main/java/org/apache/hadoop/hbase/io/encoding/HFileBlockDefaultEncodingContext.java:177 Is this method called anywhere ? src/main/java/org/apache/hadoop/hbase/io/encoding/DataBlockEncoder.java:56 Does 'as well as' mean 'or' here ? src/main/java/org/apache/hadoop/hbase/io/encoding/DataBlockEncoder.java:61 I still think this method should be renamed because compression is optional. REVISION DETAIL https://reviews.facebook.net/D2097 > Move compression/decompression to an encoder specific encoding context > ---------------------------------------------------------------------- > > Key: HBASE-5521 > URL: https://issues.apache.org/jira/browse/HBASE-5521 > Project: HBase > Issue Type: Improvement > Reporter: He Yongqiang > Assignee: He Yongqiang > Attachments: HBASE-5521.1.patch, HBASE-5521.D2097.1.patch, > HBASE-5521.D2097.2.patch, HBASE-5521.D2097.3.patch, HBASE-5521.D2097.4.patch, > HBASE-5521.D2097.5.patch, HBASE-5521.D2097.6.patch, HBASE-5521.D2097.7.patch, > HBASE-5521.D2097.8.patch > > > As part of working on HBASE-5313, we want to add a new columnar > encoder/decoder. It makes sense to move compression to be part of > encoder/decoder: > 1) a scanner for a columnar encoded block can do lazy decompression to a > specific part of a key value object > 2) avoid an extra bytes copy from encoder to hblock-writer. > If there is no encoder specified for a writer, the HBlock.Writer will use a > default compression-context to do something very similar to today's code. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira