You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Chongxin Li <li...@zju.edu.cn> on 2010/07/23 06:23:52 UTC
Review Request: HBASE-2792: Create a better way to chain log cleaners
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/372/
-----------------------------------------------------------
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
Re: Review Request: HBASE-2792: Create a better way to chain log cleaners
Posted by st...@duboce.net.
-----------------------------------------------------------
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
>
>
Re: Review Request: HBASE-2792: Create a better way to chain log cleaners
Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/372/
-----------------------------------------------------------
(Updated 2010-07-29 06:51:56.166539)
Review request for hbase.
Changes
-------
I've tested this path. This can be applied.
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 (updated)
-----
src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java 3ca3611
src/main/java/org/apache/hadoop/hbase/master/LogsCleaner.java PRE-CREATION
src/main/java/org/apache/hadoop/hbase/master/OldLogsCleaner.java 37b2c3c
src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 9fb1cce
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/TestLogsCleaner.java PRE-CREATION
src/test/java/org/apache/hadoop/hbase/master/TestOldLogsCleaner.java a92e0da
Diff: http://review.cloudera.org/r/372/diff
Testing
-------
Unit test TestOldLogsCleaner passed.
Thanks,
Chongxin
Re: Review Request: HBASE-2792: Create a better way to chain log cleaners
Posted by st...@duboce.net.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/372/#review496
-----------------------------------------------------------
Ship it!
+1
Will commit in a little while.
- stack
On 2010-07-24 05:10:08, Chongxin Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/372/
> -----------------------------------------------------------
>
> (Updated 2010-07-24 05:10:08)
>
>
> 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/LogsCleaner.java 37b2c3c
> src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 9fb1cce
> 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/TestLogsCleaner.java a92e0da
>
> Diff: http://review.hbase.org/r/372/diff
>
>
> Testing
> -------
>
> Unit test TestOldLogsCleaner passed.
>
>
> Thanks,
>
> Chongxin
>
>
Re: Review Request: HBASE-2792: Create a better way to chain log cleaners
Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/372/
-----------------------------------------------------------
(Updated 2010-07-24 05:10:08.798554)
Review request for hbase.
Changes
-------
Since currently there are only two log cleaners (TimeToLiveLogCleaner and ReplicationLogCleaner) in the chain, there would be 3 scenarios, that is log file doesn't pass the first log cleaner; log file passes the first log cleaner but is rejected by the second; log file passes both log cleaners and is then deleted. I think these 3 cases are all covered by the unit test. I've added some comments to the unit test to explain these 3 cases.
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 (updated)
-----
src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java 3ca3611
src/main/java/org/apache/hadoop/hbase/master/LogsCleaner.java 37b2c3c
src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 9fb1cce
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/TestLogsCleaner.java a92e0da
Diff: http://review.hbase.org/r/372/diff
Testing
-------
Unit test TestOldLogsCleaner passed.
Thanks,
Chongxin
Re: Review Request: HBASE-2792: Create a better way to chain log cleaners
Posted by Chongxin Li <li...@zju.edu.cn>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/372/
-----------------------------------------------------------
(Updated 2010-07-22 23:00:05.908203)
Review request for hbase.
Changes
-------
HBASE-2792: Create a better way to chain log cleaners
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 (updated)
-----
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