You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Andrew Purtell <ap...@apache.org> on 2010/09/13 09:16:06 UTC

Review Request: Support alternate compression for major compactions

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/
-----------------------------------------------------------

Review request for hbase.


Summary
-------

Support alternate compression for major compactions.

This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.


This addresses bug HBASE-2988.
    http://issues.apache.org/jira/browse/HBASE-2988


Diffs
-----

  src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
  src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
  src/main/ruby/hbase/admin.rb 82d7e54 

Diff: http://review.cloudera.org/r/821/diff


Testing
-------

Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.


Thanks,

Andrew

Re: Review Request: Support alternate compression for major compactions

Posted by Andrew Purtell <ap...@apache.org>.

> On 2010-09-13 10:16:21, Todd Lipcon wrote:
> > src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java, line 368
> > <http://review.cloudera.org/r/821/diff/1/?file=11487#file11487line368>
> >
> >     This will throw NPE if it's not set, right?
> >     Why not just make this return getCompression() if n == null, then you can get rid of the hasCompactionCompression() check down in StoreFile below and simplify things a bit?
> >     
> >     Also why redundant getCompactionCompression and getCompactionCompressionType?

There is also getCompaction and getCompactionType.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/#review1163
-----------------------------------------------------------


On 2010-09-13 20:20:49, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/821/
> -----------------------------------------------------------
> 
> (Updated 2010-09-13 20:20:49)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Support alternate compression for major compactions.
> 
> This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.
> 
> 
> This addresses bug HBASE-2988.
>     http://issues.apache.org/jira/browse/HBASE-2988
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
>   src/main/ruby/hbase/admin.rb 82d7e54 
> 
> Diff: http://review.cloudera.org/r/821/diff
> 
> 
> Testing
> -------
> 
> Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Support alternate compression for major compactions

Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/#review1163
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
<http://review.cloudera.org/r/821/#comment4000>

    This will throw NPE if it's not set, right?
    Why not just make this return getCompression() if n == null, then you can get rid of the hasCompactionCompression() check down in StoreFile below and simplify things a bit?
    
    Also why redundant getCompactionCompression and getCompactionCompressionType?


- Todd


On 2010-09-13 00:16:06, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/821/
> -----------------------------------------------------------
> 
> (Updated 2010-09-13 00:16:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Support alternate compression for major compactions.
> 
> This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.
> 
> 
> This addresses bug HBASE-2988.
>     http://issues.apache.org/jira/browse/HBASE-2988
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
>   src/main/ruby/hbase/admin.rb 82d7e54 
> 
> Diff: http://review.cloudera.org/r/821/diff
> 
> 
> Testing
> -------
> 
> Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Support alternate compression for major compactions

Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/#review1215
-----------------------------------------------------------

Ship it!


+1

- Todd


On 2010-09-14 17:22:18, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/821/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 17:22:18)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Support alternate compression for major compactions.
> 
> This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.
> 
> 
> This addresses bug HBASE-2988.
>     http://issues.apache.org/jira/browse/HBASE-2988
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
>   src/main/ruby/hbase/admin.rb 82d7e54 
> 
> Diff: http://review.cloudera.org/r/821/diff
> 
> 
> Testing
> -------
> 
> Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Support alternate compression for major compactions

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/#review1212
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
<http://review.cloudera.org/r/821/#comment4123>

    Makes sense.


- Andrew


On 2010-09-14 17:22:18, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/821/
> -----------------------------------------------------------
> 
> (Updated 2010-09-14 17:22:18)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Support alternate compression for major compactions.
> 
> This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.
> 
> 
> This addresses bug HBASE-2988.
>     http://issues.apache.org/jira/browse/HBASE-2988
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
>   src/main/ruby/hbase/admin.rb 82d7e54 
> 
> Diff: http://review.cloudera.org/r/821/diff
> 
> 
> Testing
> -------
> 
> Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Support alternate compression for major compactions

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/
-----------------------------------------------------------

(Updated 2010-09-14 17:22:18.235446)


Review request for hbase.


Changes
-------

getCompactionCompression falls back to getCompression if COMPRESSION_COMPACT is unset


Summary
-------

Support alternate compression for major compactions.

This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.


This addresses bug HBASE-2988.
    http://issues.apache.org/jira/browse/HBASE-2988


Diffs (updated)
-----

  src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
  src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
  src/main/ruby/hbase/admin.rb 82d7e54 

Diff: http://review.cloudera.org/r/821/diff


Testing
-------

Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.


Thanks,

Andrew

Re: Review Request: Support alternate compression for major compactions

Posted by Todd Lipcon <to...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/#review1177
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
<http://review.cloudera.org/r/821/#comment4037>

    don't you want to return getCompression() in this type? ie compaction compression defaults to "same as the rest of the table" unless specifically overridden?



src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/821/#comment4038>

    this way you can't specify "I don't want to compress majors"... probably unlikely, but we should probably support it


- Todd


On 2010-09-13 20:20:49, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/821/
> -----------------------------------------------------------
> 
> (Updated 2010-09-13 20:20:49)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Support alternate compression for major compactions.
> 
> This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.
> 
> 
> This addresses bug HBASE-2988.
>     http://issues.apache.org/jira/browse/HBASE-2988
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
>   src/main/ruby/hbase/admin.rb 82d7e54 
> 
> Diff: http://review.cloudera.org/r/821/diff
> 
> 
> Testing
> -------
> 
> Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.
> 
> 
> Thanks,
> 
> Andrew
> 
>

Re: Review Request: Support alternate compression for major compactions

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/
-----------------------------------------------------------

(Updated 2010-09-13 20:20:49.489804)


Review request for hbase.


Changes
-------

Updated diff to handle dumb NPE pointed out by tlipcon, also addresses the same problem in existing code.


Summary
-------

Support alternate compression for major compactions.

This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.


This addresses bug HBASE-2988.
    http://issues.apache.org/jira/browse/HBASE-2988


Diffs (updated)
-----

  src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
  src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
  src/main/ruby/hbase/admin.rb 82d7e54 

Diff: http://review.cloudera.org/r/821/diff


Testing
-------

Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.


Thanks,

Andrew

Re: Review Request: Support alternate compression for major compactions

Posted by Andrew Purtell <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/821/#review1160
-----------------------------------------------------------



src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java
<http://review.cloudera.org/r/821/#comment3997>

    I thought MAJOR_COMPRESSION was worse.



src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
<http://review.cloudera.org/r/821/#comment3998>

    This is needed because the get*Compression() methods by convention return Compression.Algorithm.NONE if unset.


- Andrew


On 2010-09-13 00:16:06, Andrew Purtell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/821/
> -----------------------------------------------------------
> 
> (Updated 2010-09-13 00:16:06)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Support alternate compression for major compactions.
> 
> This is expected to be an uncommmon configuration so I did not pollute the HColumnDescriptor constructor with the new option; instead only added convenience {get,set}ters.
> 
> 
> This addresses bug HBASE-2988.
>     http://issues.apache.org/jira/browse/HBASE-2988
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/HColumnDescriptor.java 8e3bd53 
>   src/main/java/org/apache/hadoop/hbase/regionserver/Store.java 4f777f0 
>   src/main/ruby/hbase/admin.rb 82d7e54 
> 
> Diff: http://review.cloudera.org/r/821/diff
> 
> 
> Testing
> -------
> 
> Created table with LZO compression, inserted 10GB of data, altered schema with COMPRESSION_COMPACT of 'LZMA' (custom LZMA Hadoop compression plugin), initiated major compaction, confirmed the results by examining HFiles on local fs with the UNIX file utility.
> 
> 
> Thanks,
> 
> Andrew
> 
>