Return-Path: Delivered-To: apmail-hbase-dev-archive@www.apache.org Received: (qmail 36775 invoked from network); 1 Oct 2010 21:45:20 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 1 Oct 2010 21:45:20 -0000 Received: (qmail 20844 invoked by uid 500); 1 Oct 2010 21:45:20 -0000 Delivered-To: apmail-hbase-dev-archive@hbase.apache.org Received: (qmail 20743 invoked by uid 500); 1 Oct 2010 21:45:19 -0000 Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list dev@hbase.apache.org Received: (qmail 20735 invoked by uid 99); 1 Oct 2010 21:45:19 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 01 Oct 2010 21:45:19 +0000 X-ASF-Spam-Status: No, hits=1.8 required=10.0 tests=FH_HELO_EQ_D_D_D_D,MIME_QP_LONG_LINE,SPF_NEUTRAL X-Spam-Check-By: apache.org Received-SPF: neutral (athena.apache.org: 184.73.217.71 is neither permitted nor denied by domain of stack@duboce.net) Received: from [184.73.217.71] (HELO ip-10-202-7-187.ec2.internal) (184.73.217.71) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 01 Oct 2010 21:45:14 +0000 Received: from ip-10-202-7-187.ec2.internal (localhost [127.0.0.1]) by ip-10-202-7-187.ec2.internal (Postfix) with ESMTP id 7F71B8A1FE; Fri, 1 Oct 2010 21:44:53 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: Review Request: Add ability to have multiple Masters in LocalHBaseCluster for test writing From: stack@duboce.net To: "Karthik Ranganathan" , "Kannan Muthukkaruppan" , stack@duboce.net Date: Fri, 01 Oct 2010 21:44:53 -0000 Message-ID: <20101001214453.12012.3172@ip-10-202-7-187.ec2.internal> Cc: "Jonathan Gray" , jiraposter@review.hbase.org, dev@hbase.apache.org In-Reply-To: <20101001170003.12040.26551@ip-10-202-7-187.ec2.internal> References: <20101001170003.12040.26551@ip-10-202-7-187.ec2.internal> ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.cloudera.org/r/925/#review1378 ----------------------------------------------------------- This is great. Follows the ugly pattern that was in place for RSs. Just so= me comments below... see what you think. trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java This needs to be data member? Its not just used once locally? trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java Why do this in constructor? Why not declare and allocate at same time? trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java No biggie but I like the previous syntax trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java Can you read this from zk instead? trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Go ahead and remove it. It used to be used figuring if fresh startup b= ut now its just logged to help debugging what state was like on master star= tup. trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java No need for this comment. CHeck the javadoc on this method. It used t= o be detailed steps IIRC. The detail no longer is correct. trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Yeah, this 3. is a bit odd in here w/o its 1. and 2. trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Does this if span too much? Could you test stopped before calling init= ialization? loop already does stopped? trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java What services are started in teh constructor now? trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Do we lose messages here? Important we not drop split, etc. FYI, we n= eed to do some more thought around split message and master failover. See o= ver in master where we register servers on region server report for note. trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java Yeah, whats this about? trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java How about a test where there are RIT and master joins and assertion it = does the fixup? = There is white space above the 'yay'. - stack On 2010-10-01 10:00:03, Jonathan Gray wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.cloudera.org/r/925/ > ----------------------------------------------------------- > = > (Updated 2010-10-01 10:00:03) > = > = > Review request for hbase, stack, Karthik Ranganathan, and Kannan Muthukka= ruppan. > = > = > Summary > ------- > = > To really be able to unit test the new master properly, we need to be abl= e to have multiple masters running at once within a single logical cluster. > = > Also exposes methods to get the currently active master and can access ea= ch individually in the same way that it's done for multiple RS. > = > This is mostly duplicating what we do for RS but for masters. > = > = > This addresses bug HBASE-3053. > http://issues.apache.org/jira/browse/HBASE-3053 > = > = > Diffs > ----- > = > trunk/src/main/java/org/apache/hadoop/hbase/HConstants.java 1003327 = > trunk/src/main/java/org/apache/hadoop/hbase/LocalHBaseCluster.java 1003= 327 = > trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1003327 = > trunk/src/main/java/org/apache/hadoop/hbase/master/HMasterCommandLine.j= ava 1003327 = > trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.= java 1003327 = > trunk/src/main/java/org/apache/hadoop/hbase/util/JVMClusterUtil.java 10= 03327 = > trunk/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.= java 1003327 = > trunk/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java 10= 03327 = > trunk/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java 10033= 27 = > trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.j= ava PRE-CREATION = > = > Diff: http://review.cloudera.org/r/925/diff > = > = > Testing > ------- > = > Untested but compiles. This is first go at a patch. I'm going to work o= n writing a new unit test that uses this next. > = > = > Thanks, > = > Jonathan > = >