hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jean-Daniel Cryans" <jdcry...@apache.org>
Subject Re: Review Request: Make HBASE-2694 replication-friendly
Date Wed, 16 Jun 2010 18:03:10 GMT


> On 2010-06-15 21:02:06, stack wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java, line
232
> > <http://review.hbase.org/r/185/diff/1/?file=1385#file1385line232>
> >
> >     This looks like its needed but I took a look at getZKClusterKey.  Should we
make sure there are no spaces in the ZOOKEEPER_QUORUM value?  Also, what happens if ensemble
is made up of 3 members in one config and 5 in another, is that a different zk ensemble? 
Is ZK_ZNODE_PARENT where this cluster is homed up in the ensemble?
> >     
> >     Why have the HCM class name in the wrapper name? (I notice why later but why
have class name one time and then hostname and port another and then encoded name -- I think
-- elsewhere?)
> 
> Jean-Daniel Cryans wrote:
>     - Should we make sure there are no spaces in the ZOOKEEPER_QUORUM value?
>     Why?
>     - Also, what happens if ensemble is made up of 3 members in one config and 5 in another,
is that a different zk ensemble?
>     Yes, so a new ZKW will be instantiated. Unless that happens 30 times inside the same
JVM, it's not an issue.
>      - Is ZK_ZNODE_PARENT where this cluster is homed up in the ensemble?
>     Yes.
>     - Why have the HCM class name in the wrapper name?
>     This comes from HBASE-2694, nothing new from my end.
> 
> stack wrote:
>     .bq Should we make sure there are no spaces in the ZOOKEEPER_QUORUM value?
>     
>     So, that one w/ spaces is not different from one without -- at a minimum.
>     
>     .bq Yes, so a new ZKW will be instantiated. Unless that happens 30 times inside the
same JVM, it's not an issue.
>     
>     If so, why not have all in same jvm use same ZKW?  (Drop the class name suffix --
jgray?  karthik?)

bq. So, that one w/ spaces is not different from one without -- at a minimum.

I guess it's cheap to do. Ok.

bq. If so, why not have all in same jvm use same ZKW?  (Drop the class name suffix -- jgray?
 karthik?)

Then you cannot have a single client talking to multiple ensembles? Also you need some kind
of identification that this ZKW comes from the client else expiring a region server would
expire a client in the unit tests.


> On 2010-06-15 21:02:06, stack wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/master/ZKUnassignedWatcher.java, line
46
> > <http://review.hbase.org/r/185/diff/1/?file=1389#file1389line46>
> >
> >     Why this defined in here when its over in HConstants or in HRegion.. don't remember
which.
> 
> Jean-Daniel Cryans wrote:
>     I didn't touch that code, this comes from HBASE-2694.
> 
> stack wrote:
>     Its broke defining anew.  Would you mind addressing in your patch?

Sure.


> On 2010-06-15 21:02:06, stack wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java, line
153
> > <http://review.hbase.org/r/185/diff/1/?file=1392#file1392line153>
> >
> >     Ain't this duplicated code from the method just above - in getInstance?
> 
> Jean-Daniel Cryans wrote:
>     Yes, but also I was wondering if we should cache those in some way. Then it'd be
a very good reason to refactor.
> 
> stack wrote:
>     Should refactor anyways.

K


> On 2010-06-15 21:02:06, stack wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java, line
831
> > <http://review.hbase.org/r/185/diff/1/?file=1392#file1392line831>
> >
> >     Can this throw session expired?
> 
> Jean-Daniel Cryans wrote:
>     It will be printed as an error and the method will return an empty list.
> 
> stack wrote:
>     Is that what we want?  Don't we want to konw about session expired soon as possible
to mitigate damage a RS could do proceeding as though session expired had not happened?

Then we should do it for all of ZKW, and IIRC that's in the scope of another jira that Jon
is working on?


- Jean-Daniel


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


On 2010-06-15 18:25:01, Jean-Daniel Cryans wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/185/
> -----------------------------------------------------------
> 
> (Updated 2010-06-15 18:25:01)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> This patch adds the listZnodes version that takes a watcher and changes the way the ZKW
works to include the notion of multiple clusters in the same JVM.
> 
> 
> This addresses bug HBASE-2735.
>     http://issues.apache.org/jira/browse/HBASE-2735
> 
> 
> Diffs
> -----
> 
>   /trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java 955103

>   /trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 955103 
>   /trunk/src/main/java/org/apache/hadoop/hbase/master/ProcessRegionOpen.java 955103 
>   /trunk/src/main/java/org/apache/hadoop/hbase/master/RegionManager.java 955103 
>   /trunk/src/main/java/org/apache/hadoop/hbase/master/ZKUnassignedWatcher.java 955103

>   /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java 955103

>   /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RSZookeeperUpdater.java 955103

>   /trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWrapper.java 955103

>   /trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 955103 
>   /trunk/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java 955103 
> 
> Diff: http://review.hbase.org/r/185/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jean-Daniel
> 
>


Mime
View raw message