[
https://issues.apache.org/jira/browse/HBASE-2792?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12891943#action_12891943
]
HBase Review Board commented on HBASE-2792:
-------------------------------------------
Message from: [email protected]
-----------------------------------------------------------
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
> Create a better way to chain log cleaners
> -----------------------------------------
>
> Key: HBASE-2792
> URL: https://issues.apache.org/jira/browse/HBASE-2792
> Project: HBase
> Issue Type: Improvement
> Reporter: Jean-Daniel Cryans
> Assignee: Li Chongxin
> Fix For: 0.90.0
>
>
> From Stack's review of HBASE-2223:
> {quote}
> Why this implementation have to know about other implementations? Can't we
> do a chain of decision classes? Any class can say no? As soon as any
> decision class says no, we exit the chain.... So in this case, first on the
> chain would be the ttl decision... then would be this one... and third would
> be the snapshotting decision. You don't have to do the chain as part of this
> patch but please open an issue to implement.
> {quote}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
