HBase, mail # dev - Re: Review Request: HBASE-2223

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-05-27, 01:09

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

(Updated 2010-05-26 18:09:30.362592)
Review request for hbase.
Changes
-------

This new patch takes care of almost all comments except:
ReplicationZookeeperHelper.java
- use a <pre> block to make this more readable in the HTML version of the javadoc.

ReplicationSink.java
- I think it would be good to document the fact that this method will typically be called from another thread than the thread that executes `run' so that other people reading the code will quickly get a good grasp of what are the concurrency / locking requirements.

- So Delete operations are "unbuffered" unlike Put operations, which you "buffer" in the `puts' list. Does that mean that a Delete can be executed before the Put that was creating the data in the first place, and that the Delete will fail first and the Put will survive second?

// Should we log rejected edits in a file for replay?
- I vote yes
The major change I did was removing ReplicationConnectionManager and using HCM directly since it was the same code (so the comments left by Benoit still apply, but to HCM). Other than that it's mostly refactoring and fixing nits.
Summary
-------

This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
This addresses bug HBASE-2223.
   http://issues.apache.org/jira/browse/HBASE-2223
Diffs (updated)
-----

  src/main/java/org/apache/hadoop/hbase/HConstants.java 13aff26
  src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 4cbe52a
  src/main/java/org/apache/hadoop/hbase/master/ServerManager.java a197b8f
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b5ff43a
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 12a3cd8
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 7c1184c
  src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java ed8709f
  src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java PRE-CREATION

Diff: http://review.hbase.org/r/76/diff
Testing
-------
Thanks,

Jean-Daniel

Re: Review Request: HBASE-2223

Benoit Sigoure 2010-05-27, 06:48

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/76/#review84
-----------------------------------------------------------
I'll pick up where I stopped in TestReplication.java tomorrow.
src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment412>

   Terminate this sentence with a period.

   Generally speak you must terminate the first sentence of a javadoc with a period.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment413>

   I don't find `replication.source.size.capacity' particularly explicit. What is it supposed to control?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment414>

   Ditto.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment415>

   This is unnecessary.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment416>

   Nit pick: mapOfAddr since in English "Address" starts with "Addr" :)

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment417>

   s/adr/addr/ :)

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment418>

   How about using a do-while instead of repeating this line of code?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment419>

   If HServerAddress respects the equal contract, consider using a set instead of a map.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment420>

   Move this outside of the try block.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment422>

   The explicit call to toString() here is useless.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment421>

   Is there any reason why you're checking this here?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment423>

   Log a message too.
   This will call Log#error(java.lang.Object message) and I presume you meant to call Log#error(java.lang.Object message, java.lang.Throwable t)

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment424>

   I find this comment confusing. Can you make it a bit more explicit, especially towards the "normally has a position" part?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment425>

   Don't lock in the `try' block.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment426>

   Why are you checking this again here?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment432>

   All the double-negations with noIOE are harder to read than if instead you were starting with boolean gotIOE = false and set it to true when you get an IOE.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment427>

   This `try' block is massive, would it be possible to refactor it using a private method to make the code a bit more readable?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment428>

   Use a local variable for entry.getKey() instead of calling it so many times in a row.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment429>

   Unnecessary call to toString().

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment430>

   It seems that considerDumping will always be true except when you fail to stat() the file due to an unexpected error. That seems suspicious to me. Is this intended? If yes, can you explain?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment431>

   Need a message in first argument.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment433>

   Please document.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment434>

   Please document everything properly.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment435>

   Document.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment436>

   How about storing the result of edit.getKeyValues() in a local instead of repeating that code and repeating the call twice at every loop?

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment437>

   Please document everything.

src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
<http://review.hbase.org/r/76/#comment438>

   Put this in a try-finally block. Just because you're nulling one reference doesn't protect you from this thread getting interrupted and then you're left with a locke

Re: Review Request: HBASE-2223

Benoit Sigoure 2010-05-27, 19:55

-----------------------------------------------------------
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
On 2010-05-26 18:09:30, Jean-Daniel Cryans wrote:

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-05-28, 00:19

> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 120
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line120>
> >
> > I don't find `replication.source.size.capacity' particularly explicit. What is it supposed to control?

I'll put more details into their attributes. Also if we document those configs I will add details in hbase-default.xml
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 167
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line167>
> >
> > Nit pick: mapOfAddr since in English "Address" starts with "Addr" :)

Avoue que ça te mélanges autant que moi ;)
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 179
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line179>
> >
> > If HServerAddress respects the equal contract, consider using a set instead of a map.

And then, even better, might as well just use a HashSet :P
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 208
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line208>
> >
> > Is there any reason why you're checking this here?

It's a symptom of a problem with my code, I'll fix that.
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 229
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line229>
> >
> > I find this comment confusing. Can you make it a bit more explicit, especially towards the "normally has a position" part?

Yeah, in fact I could just remove it... It means that we recovered a queue from another region server, and normally you are tailing a file but a RS could fail between 2 logs and that means that the new one wasn't read from yet.
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 268
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line268>
> >
> > All the double-negations with noIOE are harder to read than if instead you were starting with boolean gotIOE = false and set it to true when you get an IOE.

y
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 270
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line270>
> >
> > This `try' block is massive, would it be possible to refactor it using a private method to make the code a bit more readable?

I'll do my best.
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java, line 329
> > <http://review.hbase.org/r/76/diff/2/?file=664#file664line329>
> >
> > It seems that considerDumping will always be true except when you fail to stat() the file due to an unexpected error. That seems suspicious to me. Is this intended? If yes, can you explain?

I agree that part is weird, I built this while HDFS wasn't stable yet for tail'ing files and ended up having to decide if I should ditch a file that was broken. Currently you consider dumping the file when you get an EOF and if:
- The queue was recovered and is empty. It's very suspicious. Usually you get EOFs on file that are opened but totally empty. Might as well get rid of it?
- The number of entries read is greater than 0. That means you were reading the file, and instead of just getting null at the end you get an EOF in the face. By experience, it's usually a race condition à la HDFS-1057 then just redo the reading. If it failed 10 times, it means that that file is totally broken.

Comments?
> On 2010-05-26 23:48:35, Benoit Sigoure wrote:

Nah doesn't need to.

Already documented in the interface
All from the implemented interface

Excellent question...

Start 2 region servers

Seems the class would be even more complicated.

Was keeping it around in case I need it again.
- Jean-Daniel
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/76/#review84
On 2010-05-26 18:09:30, Jean-Daniel Cryans wrote:

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-05-28, 18:55

> On 2010-05-27 12:55:57, Benoit Sigoure wrote:
> > src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java, line 378
> > <http://review.hbase.org/r/76/diff/2/?file=669#file669line378>
> >
> > This seems brittle to me.

Kinda, but it's currently the best I found.
> On 2010-05-27 12:55:57, Benoit Sigoure wrote:
> > src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java, line 385
> > <http://review.hbase.org/r/76/diff/2/?file=669#file669line385>
> >
> > ?

There are some race conditions currently preventing that to work. Same reason I need to make sure in this test that the master cluster has all the edits when I kill a region server GC-style. See this code:

   // Test we actually have all the rows, we may miss some because we
   // don't have IO fencing.
   if (res.length != initialCount) {
   LOG.warn("We lost some rows on the master cluster!");
   // We don't really expect the other cluster to have more rows
   initialCount = res.length;
   }

So if I can lose rows on the master cluster, I can also lose rows on the slave cluster... but rows that are lost because of a lack of IO-fencing or because of a bug in the replication will be the same from the client perspective.
> On 2010-05-27 12:55:57, Benoit Sigoure wrote:
> > src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java, line 135
> > <http://review.hbase.org/r/76/diff/2/?file=671#file671line135>
> >
> > 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.

/me feels dumb
> On 2010-05-27 12:55:57, Benoit Sigoure wrote:
> > src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java, line 198
> > <http://review.hbase.org/r/76/diff/2/?file=672#file672line198>
> >
> > Uh? Manual loop unrolling?

Mostly cruft
- Jean-Daniel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/76/#review86
-----------------------------------------------------------
On 2010-05-26 18:09:30, Jean-Daniel Cryans wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/76/
> -----------------------------------------------------------
>
> (Updated 2010-05-26 18:09:30)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
>
>
> This addresses bug HBASE-2223.
> http://issues.apache.org/jira/browse/HBASE-2223
>
>
> Diffs
> -----
>
> src/main/java/org/apache/hadoop/hbase/HConstants.java 13aff26
> src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 4cbe52a
> src/main/java/org/apache/hadoop/hbase/master/ServerManager.java a197b8f
> src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b5ff43a
> src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 12a3cd8
> src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 7c1184c
> src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-05-28, 19:03

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

(Updated 2010-05-28 12:03:35.298094)
Review request for hbase.
Changes
-------

New patch that takes care of almost all Benoit's comments. My current TODO is:
ReplicationZookeeperHelper.java
- use a <pre> block to make this more readable in the HTML version of the javadoc.

ReplicationSink.java
- I think it would be good to document the fact that this method will typically be called from another thread than the thread that executes `run' so that other people reading the code will quickly get a good grasp of what are the concurrency / locking requirements.

- So Delete operations are "unbuffered" unlike Put operations, which you "buffer" in the `puts' list. Does that mean that a Delete can be executed before the Put that was creating the data in the first place, and that the Delete will fail first and the Put will survive second?

// Should we log rejected edits in a file for replay?
- I vote yes

ReplicationSource.java
- This `try' block is massive, would it be possible to refactor it using a private method to make the code a bit more readable?
Summary
-------

This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
This addresses bug HBASE-2223.
   http://issues.apache.org/jira/browse/HBASE-2223
Diffs (updated)
-----

  src/main/java/org/apache/hadoop/hbase/HConstants.java 13aff26
  src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 4cbe52a
  src/main/java/org/apache/hadoop/hbase/master/ServerManager.java a197b8f
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b5ff43a
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 12a3cd8
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 7c1184c
  src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java ed8709f
  src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java PRE-CREATION

Diff: http://review.hbase.org/r/76/diff
Testing
-------
Thanks,

Jean-Daniel

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-05-28, 23:02

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

(Updated 2010-05-28 16:02:44.219338)
Review request for hbase.
Changes
-------

The remaining issues and some resolutions:

ReplicationSink.java
- I think it would be good to document the fact that this method will typically be called from another thread than the thread that executes `run' so that other people reading the code will quickly get a good grasp of what are the concurrency / locking requirements.

I chatted with Benoit on this issue, the problem with ReplicationSink at the moment is it's a mix of two solutions: sync and async. It's sync'ed because the thread that writes the log file has to wait for the ReplicationSink to apply it… but instead it could work like the ReplicationSource and leave it up to ZK-based management of log files. It currently work as is, but it's not efficient. I can fix this in the scope of this jira, which is still named "Handle 10min+ network partitions between clusters", or in a later one.

- So Delete operations are "unbuffered" unlike Put operations, which you "buffer" in the `puts' list. Does that mean that a Delete can be executed before the Put that was creating the data in the first place, and that the Delete will fail first and the Put will survive second?

Created a test that inserts 2 rows, deletes the second one, and adds two more. I expect to see the row as deleted, and it works (yeah!). Included in this patch

// Should we log rejected edits in a file for replay?
- I vote yes

This will be in HBASE-2626.

ReplicationSource.java
- This `try' block is massive, would it be possible to refactor it using a private method to make the code a bit more readable?

Did some refactoring, but still not satisfied.
Summary
-------

This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
This addresses bug HBASE-2223.
   http://issues.apache.org/jira/browse/HBASE-2223
Diffs (updated)
-----

  bin/replication/add_peer.rb PRE-CREATION
  bin/replication/copy_tables_desc.rb PRE-CREATION
  pom.xml 0a009cf
  src/main/java/org/apache/hadoop/hbase/HConstants.java 13aff26
  src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 4cbe52a
  src/main/java/org/apache/hadoop/hbase/master/ServerManager.java a197b8f
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b5ff43a
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 12a3cd8
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 7c1184c
  src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java ed8709f
  src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java PRE-CREATION

Diff: http://review.hbase.org/r/76/diff
Testing
Thanks,

Jean-Daniel

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-06-17, 19:57

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

(Updated 2010-06-17 12:57:07.815144)
Review request for hbase.
Changes
-------

Patch rebased on the new Zookeeper/Master and with a cleaned up ReplicationSink. Also some cleanup from Stack's comments.

Currently fails TestReplication, hitting https://issues.apache.org/jira/browse/HBASE-2741 that kills a master thread. The root cause could be in this code but the NPE doesn't help debugging.
Summary
-------

This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
This addresses bug HBASE-2223.
   http://issues.apache.org/jira/browse/HBASE-2223
Diffs (updated)
-----

  bin/replication/add_peer.rb PRE-CREATION
  bin/replication/copy_tables_desc.rb PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/HConstants.java f5d3e94
  src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 62617ac
  src/main/java/org/apache/hadoop/hbase/master/HMaster.java 66dc697
  src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java 4c5153e
  src/main/java/org/apache/hadoop/hbase/master/RegionServerOperationQueue.java 10f9dbd
  src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 1d95258
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 7ace16a
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 05cf17f
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 5d4cffe
  src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperHelper.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 04957ca
  src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java PRE-CREATION

Diff: http://review.hbase.org/r/76/diff
Testing
-------
Thanks,

Jean-Daniel

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-06-24, 00:24

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

(Updated 2010-06-23 17:24:52.371363)
Review request for hbase.
Changes
-------

This patch now passes its own tests (many core modifications made them unstable) and incorporates comments from Stack's review. It also has the patch from HBASE-2707 applied because without it it's too easy to fail TestReplication.

It also relies on using this hadoop jar http://people.apache.org/~rawson/repo/org/apache/hadoop/hadoop-core/0.20.3-append-r956776+1240+tail/
Summary
-------

This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
This addresses bug HBASE-2223.
   http://issues.apache.org/jira/browse/HBASE-2223
Diffs (updated)
-----

  bin/replication/add_peer.rb PRE-CREATION
  bin/replication/copy_tables_desc.rb PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/HConstants.java 4357ee5
  src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 62617ac
  src/main/java/org/apache/hadoop/hbase/master/HMaster.java 5367638
  src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java 4c5153e
  src/main/java/org/apache/hadoop/hbase/master/RegionServerOperationQueue.java 10f9dbd
  src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 9fb1cce
  src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 33b9a99
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 94f18c6
  src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 5d4cffe
  src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION
  src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java b26c071
  src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 0e69709
  src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java PRE-CREATION
  src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java PRE-CREATION

Diff: http://review.hbase.org/r/76/diff
Testing
-------
Thanks,

Jean-Daniel

Re: Review Request: HBASE-2223

stack@... 2010-06-25, 22:07

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/76/#review281
-----------------------------------------------------------
Missing is more desciption of how this feature works. There is package doc. on how to get it going but needs fuller description of how replication works (how zk is used, how it manages hlogs, how it sends edits to remote cluster, etc.). There is insufficient description in javadoc. Without this, there is a danger that only the author will be able to figure whats going on in here.

How does one get insight on to how well or badly replication is working?

Alot of errors are logged and then we just move on. Thats fine for an alpha package but need a plan for making it all more robust? What you thinking?
bin/replication/add_peer.rb
<http://review.hbase.org/r/76/#comment1187>

   These log messages add nothing? Remove? Just restating what was passed on cmdline.

bin/replication/add_peer.rb
<http://review.hbase.org/r/76/#comment1188>

   Why do this? Its annoying?

   It'd be better spending time interrogating what the user passed to see if it makes sense throwing errors if incorrectly formatted or if it doesn't look like it makes sense?

bin/replication/add_peer.rb
<http://review.hbase.org/r/76/#comment1189>

   A message on end saying what it did can be comforting to users.

bin/replication/copy_tables_desc.rb
<http://review.hbase.org/r/76/#comment1190>

   See above comments.

src/main/java/org/apache/hadoop/hbase/HConstants.java
<http://review.hbase.org/r/76/#comment1191>

   Change name of this config to hbase.replication.

src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java
<http://review.hbase.org/r/76/#comment1192>

   Does this method have prerequisites? For example, does replication have to be enabled?

src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.hbase.org/r/76/#comment1193>

   Is this a leak from another patch?

src/main/java/org/apache/hadoop/hbase/master/RegionServerOperationQueue.java
<http://review.hbase.org/r/76/#comment1194>

   This is a leak from another patch?

src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
<http://review.hbase.org/r/76/#comment1195>

   Whats going on here? We're setting into config the logcleaner to use but then in the lines after this we go ahead and create OldLogsCleaner anyways?

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/76/#comment1196>

   Just call this 'replication' or 'replicationEnabled'

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/76/#comment1198>

   We are a ReplicationSink or a ReplicationMaster? One or the other? Can we not move all of this Replication stuff into a Replication class including the setup of whether we are a Sink or Master rather than have it hang out here in HRS?

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/76/#comment1197>

   I don't grok the comment

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
<http://review.hbase.org/r/76/#comment1199>

   This could be null if we are a replication master?

src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.hbase.org/r/76/#comment1200>

   If the above mentioned Replication class were a singleton, it could be shared here with HRS

src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.hbase.org/r/76/#comment1201>

   It'd be good if we didn't have to create this byte [] per edit. I should reinstitute the comparator I had that took KVs but only compared family portion of the KV... no need to create family byte [] then.

src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java
<http://review.hbase.org/r/76/#comment1202>

   YOu might say why its being moved. Should be INFO level? WALs are kinda important to track?

src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java
<http://review.hbase.org/r/76/#comment1203>

   isReplicating is what you would name the method that accesses the boolean replicating... with this name the accessor should be named isIsReplicating.

src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java
<http://review.hbase.org/r/76/#comment1204>

   Why do it this way? Why an if/else? Why not do if (...length != 3) throw .... no need of an else.

src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java
<http://review.hbase.org/r/76/#comment1205>

   This is an error? Is it critical fail (We dropped recording a WAL) Are we just logging and moving on?

src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java
<http://review.hbase.org/r/76/#comment1206>

   Same here.... We just log and keep going? Are any of these fatal?

src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java
<http://review.hbase.org/r/76/#comment1207>

   If this fails, what are the repercusssions?

src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java
<http://review.hbase.org/r/76/#comment1208>

   What happens if this server dies? It gets cleaned up by master?

   What happens to replication if master dies? Does master failover need to do anything special to pick up running replication?

src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java
<http://review.hbase.org/r/76/#comment1209>

   spelling


src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java
<http://review.hbase.org/r/76/#comment1213>

   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 n

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-06-29, 01:29

> On 2010-06-25 15:07:35, stack wrote:
> > src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java, line 149
> > <http://review.hbase.org/r/76/diff/7/?file=1590#file1590line149>
> >
> > Duplicated code -- see up about ten lines.
>
> Jean-Daniel Cryans wrote:
> Not it's different, one inserts and the other deletes

Ok oh yeah there is a better way to do it.
- Jean-Daniel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/76/#review281
-----------------------------------------------------------
On 2010-06-23 17:24:52, Jean-Daniel Cryans wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/76/
> -----------------------------------------------------------
>
> (Updated 2010-06-23 17:24:52)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
>
>
> This addresses bug HBASE-2223.
> http://issues.apache.org/jira/browse/HBASE-2223
>
>
> Diffs
> -----
>
> bin/replication/add_peer.rb PRE-CREATION
> bin/replication/copy_tables_desc.rb PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/HConstants.java 4357ee5
> src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 62617ac
> src/main/java/org/apache/hadoop/hbase/master/HMaster.java 5367638
> src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java 4c5153e
> src/main/java/org/apache/hadoop/hbase/master/RegionServerOperationQueue.java 10f9dbd
> src/main/java/org/apache/hadoop/hbase/master/ServerManager.java 9fb1cce
> src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 33b9a99
> src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 94f18c6
> src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 5d4cffe
> src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java b26c071
> src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 0e69709
> src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java PRE-CREATION
>
> Diff: http://review.hbase.org/r/76/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jean-Daniel
>
>

Re: Review Request: HBASE-2223

Jean-Daniel Cryans 2010-06-30, 22:36

Re: Review Request: HBASE-2223

stack@... 2010-06-30, 22:39

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

Ship it!
+1 on commit if all tests pass. I sat wé J-D and went over the changes. All is good for first commit.

- stack
On 2010-06-30 15:36:59, Jean-Daniel Cryans wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/76/
> -----------------------------------------------------------
>
> (Updated 2010-06-30 15:36:59)
>
>
> Review request for hbase.
>
>
> Summary
> -------
>
> This is HBASE-2223 AKA Replication 2.0, it is currently only a "preview patch" as it's pretty much feature complete, works on a cluster, has unit tests and whatnot, but it could use a lot more testing and cleaning and ideas from others.
>
>
> This addresses bug HBASE-2223.
> http://issues.apache.org/jira/browse/HBASE-2223
>
>
> Diffs
> -----
>
> bin/replication/add_peer.rb PRE-CREATION
> bin/replication/copy_tables_desc.rb PRE-CREATION
> pom.xml 400cc0c
> src/main/java/org/apache/hadoop/hbase/HConstants.java 4357ee5
> src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 62617ac
> src/main/java/org/apache/hadoop/hbase/master/HMaster.java 04bc95e
> src/main/java/org/apache/hadoop/hbase/master/LogCleanerDelegate.java 4c5153e
> src/main/java/org/apache/hadoop/hbase/master/OldLogsCleaner.java 07e69b4
> src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 33b9a99
> src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLog.java 94f18c6
> src/main/java/org/apache/hadoop/hbase/regionserver/wal/HLogKey.java 5d4cffe
> src/main/java/org/apache/hadoop/hbase/replication/ReplicationZookeeperWrapper.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/master/ReplicationLogCleaner.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/package.html PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSink.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceInterface.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java PRE-CREATION
> src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java b26c071
> src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java d865e38
> src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLog.java e96637f
> src/test/java/org/apache/hadoop/hbase/replication/ReplicationSourceDummy.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/TestReplication.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/TestReplicationSource.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSink.java PRE-CREATION
> src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSourceManager.java PRE-CREATION
>
> Diff: http://review.hbase.org/r/76/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jean-Daniel
>
>