Home | About | Sematext search-lucene.com search-hadoop.com
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB
 Search Hadoop and all its subprojects:

Switch to Threaded View
HBase >> mail # dev >> Review for HBASE-7568 [replication] Create an interface for replication queues


Copy link to this message
-
Re: Review for HBASE-7568 [replication] Create an interface for replication queues
Thanks J-D!

On Thu, Mar 28, 2013 at 2:10 PM, Jean-Daniel Cryans <[EMAIL PROTECTED]>wrote:

> Two general comments about the patch. The code itself looks good.
>
> J-D
>
> On Thu, Mar 28, 2013 at 1:40 PM, Chris Trezzo <[EMAIL PROTECTED]> wrote:
> > Hi All,
> >
> > I recently posted a patch for HBASE-7568 and it would be great if I could
> > get some feedback/eyes on it. If you need a higher-level context, the
> > parent issue HBASE-7564 has a brief document that outlines the intended
> > class structure of the refactor. I have split the refactor into multiple
> > patches to hopefully make this more digestible. This patch only covers
> > state related to replication queues. State for replication peers will be
> > split out in a follow on patch (HBASE-7567) that is almost ready.
> >
> > Review: https://reviews.apache.org/r/9987/
> >
> > Some of the major motivations for refactoring the replication code are:
> > 1. Make it more testable. With clear interfaces we can easily unit test
> how
> > replication maintains its state.
>
> I think your argument for a refactoring would be a lot stronger if the
> patch was adding new tests, at least to demonstrate the added
> testability.
>

Good point. I do have a follow on jira (HBASE-8149) for increasing coverage
around the new interfaces, but I can add some new tests as part of this
patch as well.
>
> > 2. Make the code more understandable. In the code's current form, there
> is
> > a lot of complexity in the interaction between ReplicationZookeeper,
> > ReplicationSource and ReplicationSourceManager. Hopefully, the refactor
> > will make these interactions more explicit.
> > 3. Make the code more flexible for alternate implementations. If in the
> > future we wanted to store replication queues in something other than
> > Zookeeper (not saying we necessarily want to do this), having the
> > interfaces will make this easier.
>
> If we want to be able to add other implementations, should it be
> better giving more accurate names to the current ones? If what we have
> is ZK-based, I would then replace "Impl" in the class names with "ZK"
> or something like that.
>
>
Another good point. I will rename the impl classes to indicate that they
are the Zookeeper implementation.
> >
> > Thanks!
> > Chris Trezzo
>
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB