hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@duboce.net
Subject Re: Review Request: Make HBASE-2694 replication-friendly
Date Wed, 16 Jun 2010 17:57:57 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.

.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?)


> 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.

Its broke defining anew.  Would you mind addressing in your patch?


> On 2010-06-15 21:02:06, stack wrote:
> > /trunk/src/main/java/org/apache/hadoop/hbase/master/ZKUnassignedWatcher.java, line
58
> > <http://review.hbase.org/r/185/diff/1/?file=1389#file1389line58>
> >
> >     Should this be the master address as you have over inside the HRS where you
use servername where master address is hostname + port for case of multiple masters, even
in same JVM?
> 
> Jean-Daniel Cryans wrote:
>     From HBASE-2694.

Again, this is broke/inconsistent.


> 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.

Should refactor anyways.


> 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.

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?


- stack


-----------------------------------------------------------
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