hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-4608) HLog Compression
Date Wed, 22 Feb 2012 05:12:51 GMT

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

jiraposter@reviews.apache.org commented on HBASE-4608:
------------------------------------------------------


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2740/#review5265
-----------------------------------------------------------


This looks great.  Some small comments below.


src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java
<https://reviews.apache.org/r/2740/#comment11488>

    Should this javadoc here in the class include the notes you made for Kannan where you
describe how it all works?  If not here, where else will doc. on how the Compressor works
go?
    
    Maybe you should purge all mention of WAL from this class -- e.g. WALDictionary -- because
it seems like it could be easily generalized (I suppose we can do that later).



src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java
<https://reviews.apache.org/r/2740/#comment11489>

    The way the usage is written, -u and -c are optional.  You should fix that.  Looks like
they are required going by fact that args.length needs to be 3.  Also, it looks like you take
--help, the long form, or -u/-c the short forms.  Either take all short forms or take both
long and short form to be consistent.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java
<https://reviews.apache.org/r/2740/#comment11490>

    Why is the tool called WALCompressor in the usage but the class I invoke is Compressor?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java
<https://reviews.apache.org/r/2740/#comment11491>

    This does not need to be an HBaseConfiguration?  There are no configs in hbase-site.xml
that might effect whats going on here?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java
<https://reviews.apache.org/r/2740/#comment11492>

    Doc the '@return'



src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java
<https://reviews.apache.org/r/2740/#comment11493>

    Doc the return



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/2740/#comment11494>

    White space



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<https://reviews.apache.org/r/2740/#comment11495>

    When is this called?  Post construction?  Should it be part of constructor?  What happens
if its called part way through the writing of a WAL?  Will we start compressing a WAL in the
middle?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java
<https://reviews.apache.org/r/2740/#comment11496>

    I don't follow whats going on here.  What happens when len >= 0?  Why is it < 0?
 Whats that mean?  Whats v2 of hlogkey?  What if keyContext is not null?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/KeyValueCompression.java
<https://reviews.apache.org/r/2740/#comment11497>

    Class comment on what this is about?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/KeyValueCompression.java
<https://reviews.apache.org/r/2740/#comment11498>

    Why do I clear this?  Why not just throw it away?  Does clearing make it so I can recycle
this instance?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/LRUDictionary.java
<https://reviews.apache.org/r/2740/#comment11499>

    Why would I ever let go of terms in the dictionary?  Should you explain why in class comment?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/LRUDictionary.java
<https://reviews.apache.org/r/2740/#comment11501>

    Should this be static?  Does it need reference to outer class?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/LRUDictionary.java
<https://reviews.apache.org/r/2740/#comment11502>

    Class comment?  Should this be static?



src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java
<https://reviews.apache.org/r/2740/#comment11503>

    Why am I reading whether compression is on or off by looking at config?  Why am I not
looking into head of the WAL file and figure its compressed and then decompressing?  Otherwise,
if config is disabled but I'm fed a compressed file, do I just burp?  See the white space
added here.



src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALDictionary.java
<https://reviews.apache.org/r/2740/#comment11504>

    Should be just called Dictionary. Its in the wal package.  No need of the redundant prefix?



src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplayCompressed.java
<https://reviews.apache.org/r/2740/#comment11505>

    This will run all the tests in TestWALReplay?  Nice.


- Michael


On 2012-02-22 03:46:12, Li Pi wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2740/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-02-22 03:46:12)
bq.  
bq.  
bq.  Review request for hbase, Eli Collins and Todd Lipcon.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  HLog compression. Has unit tests and a command line tool for compressing/decompressing.
bq.  
bq.  
bq.  This addresses bug HBase-4608.
bq.      https://issues.apache.org/jira/browse/HBase-4608
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/main/java/org/apache/hadoop/hbase/HConstants.java 35339b6 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java c945a99 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java f067221 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/KeyValueCompression.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/LRUDictionary.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogReader.java d9cd6de

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/SequenceFileLogWriter.java cbef70f

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALDictionary.java PRE-CREATION

bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/WALEdit.java e1117ef 
bq.    src/main/java/org/apache/hadoop/hbase/util/Bytes.java ead9a3b 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestLRUDictionary.java PRE-CREATION

bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplay.java 23d27fd 
bq.    src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestWALReplayCompressed.java
PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/2740/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Li
bq.  
bq.


                
> HLog Compression
> ----------------
>
>                 Key: HBASE-4608
>                 URL: https://issues.apache.org/jira/browse/HBASE-4608
>             Project: HBase
>          Issue Type: New Feature
>            Reporter: Li Pi
>            Assignee: Li Pi
>         Attachments: 4608v1.txt, 4608v13.txt, 4608v13.txt, 4608v14.txt, 4608v5.txt, 4608v6.txt,
4608v7.txt, 4608v8fixed.txt
>
>
> The current bottleneck to HBase write speed is replicating the WAL appends across different
datanodes. We can speed up this process by compressing the HLog. Current plan involves using
a dictionary to compress table name, region id, cf name, and possibly other bits of repeated
data. Also, HLog format may be changed in other ways to produce a smaller HLog.

--
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

        

Mime
View raw message