[
https://issues.apache.org/jira/browse/HBASE-2223?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12872344#action_12872344
]
HBase Review Board commented on HBASE-2223:
-------------------------------------------
Message from: "Benoit Sigoure" <[email protected]>
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/76/#review86
-----------------------------------------------------------
src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java
<http://review.hbase.org/r/76/#comment472>
This test is pretty long and tests quite a few different things. Consider
to break it down into multiple smaller unit tests.
src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java
<http://review.hbase.org/r/76/#comment473>
I'm not sure to understand why this test is named like that. Could you
comment this test a bit more to explain what the intent is?
src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java
<http://review.hbase.org/r/76/#comment474>
This seems brittle to me.
src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java
<http://review.hbase.org/r/76/#comment475>
?
src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java
<http://review.hbase.org/r/76/#comment476>
I believe this method can be made `static'.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment477>
Copyright header.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment486>
I'm not a big fan of this.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment478>
private.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment479>
Do you really need to define the 3 following empty methods?
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment484>
Consider writing a comment explaining what this test does.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment480>
Missing space before `conf'.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment481>
+"" is an idiom I don't like.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment482>
Missing spaces.
src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java
<http://review.hbase.org/r/76/#comment483>
Missing spaces.
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java
<http://review.hbase.org/r/76/#comment485>
Don't do this.
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java
<http://review.hbase.org/r/76/#comment487>
Don't do this.
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java
<http://review.hbase.org/r/76/#comment488>
Why protected?
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java
<http://review.hbase.org/r/76/#comment489>
Is this really needed?
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java
<http://review.hbase.org/r/76/#comment490>
I don't understand why you're doing +=2 and /2 instead of just setting the
upper bound of the loop to BATCH_SIZE/2.
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java
<http://review.hbase.org/r/76/#comment491>
Missing space before `scanRes'.
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
<http://review.hbase.org/r/76/#comment492>
Why protected?
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
<http://review.hbase.org/r/76/#comment493>
?
src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java
<http://review.hbase.org/r/76/#comment494>
Uh? Manual loop unrolling?
- Benoit
> Handle 10min+ network partitions between clusters
> -------------------------------------------------
>
> Key: HBASE-2223
> URL: https://issues.apache.org/jira/browse/HBASE-2223
> Project: HBase
> Issue Type: Sub-task
> Reporter: Jean-Daniel Cryans
> Assignee: Jean-Daniel Cryans
> Fix For: 0.21.0
>
> Attachments: HBASE-2223.patch
>
>
> We need a nice way of handling long network partitions without impacting a
> master cluster (which pushes the data). Currently it will just retry over and
> over again.
> I think we could:
> - Stop replication to a slave cluster if it didn't respond for more than 10
> minutes
> - Keep track of the duration of the partition
> - When the slave cluster comes back, initiate a MR job like HBASE-2221
> Maybe we want less than 10 minutes, maybe we want this to be all automatic or
> just the first 2 parts. Discuss.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
