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