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

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



bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > 
src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressedKeyValue.java, 
line 37
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70700#file70700line37>
bq.  >
bq.  >     I'd rename this class to KeyValueCompression or even KVCompression. 
Then rename readFields to just "read" -- since this is just utility functions, 
not actually an instance of a compressed keyvalue.

fixed. legacy name. <3 eclipse.


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java, 
line 207
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70702#file70702line207>
bq.  >
bq.  >     *un*compressed value, right?

fixed.


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > 
src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressionContext.java, 
line 28
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70701#file70701line28>
bq.  >
bq.  >     Since this is so simple, I'd move it to be a static inner class of 
KVCompression above

fixed.


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java, 
line 152
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70702#file70702line152>
bq.  >
bq.  >     why is this split into two if/elses? looks like the top clauses can 
be combined, as can the bottom clauses

fixed.


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java, 
line 174
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70702#file70702line174>
bq.  >
bq.  >     switch order of "in" and "offset" here.
bq.  >     
bq.  >     Perhaps clearer to name this as "uncompressIntoArray"?

fixed.


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java, 
line 44
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70702#file70702line44>
bq.  >
bq.  >     I think we can merge this with the other class that just has static 
methods as well.

Compressor contains static methods for general purpose compression. 
KeyValueCompression.java contains static methods for compressing the KeyValue 
type. Should I merge them?


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java, 
line 185
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70702#file70702line185>
bq.  >
bq.  >     worth a comment here to explain that the "status" byte actually has 
the high-order byte of the dictionary entry in the case that it's in the 
dictionary

done


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > 
src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressedKeyValue.java, 
line 96
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70700#file70700line96>
bq.  >
bq.  >     rather than using keyVal.getRow(), keyVal.getFamily(), 
keyVal.getQualifer(), you should use the versions of those functions that just 
return offsets and lengths (eg getKeyOffset, getKeyLength). Then expand the 
writeCompressed API to take (byte[] buf, int off, int len). Otherwise you're 
making needless copies/garbage here.

This is gonna take a while. Since I'm currently relying on default 
Array.HashCode. Will need to use Bytes.HashCode and do a wrapper for insertion 
into the dictionary.


bq.  On 2012-02-01 02:29:54, Todd Lipcon wrote:
bq.  > src/main/java/org/apache/hadoop/hbase/regionserver/wal/Compressor.java, 
line 100
bq.  > <https://reviews.apache.org/r/2740/diff/16/?file=70702#file70702line100>
bq.  >
bq.  >     this function requires that the whole log data fit in RAM - not a 
great assumption

old one. will do eventually...


- Li


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


On 2012-02-15 04:57:45, 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-15 04:57:45)
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 763fe89 
bq.    
src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressedKeyValue.java 
PRE-CREATION 
bq.    
src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressionContext.java 
PRE-CREATION 
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 e46a7a0 
bq.    src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 
f067221 
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/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, 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

        

Reply via email to