hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: HBASE-2792: Create a better way to chain log cleaners
Date Sat, 24 Jul 2010 06:26:50 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/372/#review479
-----------------------------------------------------------


Patch looks great.  Can you add a little more to the unit test to prove your new chain mechanism
works?

I'd suggest that you remove all mention of snapshot from your patch (e.g. in the conf file
below) so we can commit this infrastructural change without requiring snapshot code to be
in place.  In one of your snapshot patches to come, include edit to conf file to add back
mention of snapshot.  Good stuff Li.

Make this standalone, and if you can, add some more to unit tests (its ok if you can't figure
something but it'd be nice)... and then I'll commit.


src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java
<http://review.hbase.org/r/372/#comment1971>

    This is nice.



src/main/java/org/apache/hadoop/hbase/master/OldLogsCleaner.java
<http://review.hbase.org/r/372/#comment1972>

    Please rename this class Li (even though you did not originally name it).  OldLogsCleaner
seems inappropriate; 'old' is not sufficient reason cleaning logs going forward.



src/main/java/org/apache/hadoop/hbase/master/OldLogsCleaner.java
<http://review.hbase.org/r/372/#comment1973>

    LogsCleaner is better than OldLogsCleaner I think.



src/main/java/org/apache/hadoop/hbase/master/OldLogsCleaner.java
<http://review.hbase.org/r/372/#comment1974>

    I think that is fine -- skipping if can't instantiate.


- stack


On 2010-07-22 23:00:05, Chongxin Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/372/
> -----------------------------------------------------------
> 
> (Updated 2010-07-22 23:00:05)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> HBASE-2792: Create a better way to chain log cleaners
> 
> 
> This addresses bug HBASE-2792.
>     http://issues.apache.org/jira/browse/HBASE-2792
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java 3ca3611 
>   src/main/java/org/apache/hadoop/hbase/master/OldLogsCleaner.java 37b2c3c 
>   src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java
eb859aa 
>   src/main/resources/hbase-default.xml e3a9669 
>   src/test/java/org/apache/hadoop/hbase/master/TestOldLogsCleaner.java a92e0da 
> 
> Diff: http://review.hbase.org/r/372/diff
> 
> 
> Testing
> -------
> 
> Unit test TestOldLogsCleaner passed.
> 
> 
> Thanks,
> 
> Chongxin
> 
>


Mime
View raw message