Mailing-List: contact dev-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Received-SPF: neutral (nike.apache.org: 184.73.217.71 is neither permitted nor
 denied by domain of stack@duboce.net)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Subject: Re: Review Request: HBASE-2819 HBCK repair problems (new master
 remix)
From: stack@duboce.net
To: stack@duboce.net
Date: Fri, 22 Oct 2010 20:05:11 -0000
Message-ID: <20101022200511.11863.65575@ip-10-202-7-187.ec2.internal>
Cc: "Jonathan Gray" <jgray@apache.org>, jiraposter@review.hbase.org,
	dev@hbase.apache.org
In-Reply-To: <20101021004711.11863.17696@ip-10-202-7-187.ec2.internal>
References: <20101021004711.11863.17696@ip-10-202-7-187.ec2.internal>


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/1036/#review1622
-----------------------------------------------------------


trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/1036/#comment5479>

    Should we do a notifyall on rit after this remove?  I'm looking at regi=
onOnline.  It does a notify all if the remove actually removed something.  =
In fact, can we do a little refactoring so we share the first part of regio=
nOnline between regionOnline and here?


trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
<http://review.cloudera.org/r/1036/#comment5480>

    Is this safe?  We need a sync on regions above, but not here?  And if y=
ou sync, is there danger of deadlock?


trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
<http://review.cloudera.org/r/1036/#comment5474>

    this is an odd check?  why not have the check done inside in clearRegio=
nFromTransition... Its a little racy?


- stack


On 2010-10-20 17:47:11, Jonathan Gray wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.cloudera.org/r/1036/
> -----------------------------------------------------------
> =

> (Updated 2010-10-20 17:47:11)
> =

> =

> Review request for hbase and stack.
> =

> =

> Summary
> -------
> =

> Migration of latest HBCK patch made against old master by Alex.
> =

> I'm not sure this really makes sense anymore on the new master.  Dupe ass=
ignment and non-assignment in the "traditional" cases should not happen any=
more, at least not in the way it used to happen.
> =

> This is mostly a straight migration and adds stuff to clear things out of=
 transition on master, close a region on an RS w/o using ZK (already suppor=
ted just exposed via rpc now), etc...
> =

> I don't think it hurts to have this code in there but mostly the detectio=
n code should be used for now.  Once we trip over assignment issues / hbck =
fixable issues, we can add fixup to this new stuff.
> =

> Can just rip out the repair for now but figured would be simpler just to =
leave it and continue improving it rather than going back a step.
> =

> =

> This addresses bug HBSAE-2819.
>     http://issues.apache.org/jira/browse/HBSAE-2819
> =

> =

> Diffs
> -----
> =

>   trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseFsck.java 10257=
90 =

>   trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseFsckRepair.java=
 PRE-CREATION =

>   trunk/src/main/java/org/apache/hadoop/hbase/ipc/HRegionInterface.java 1=
025790 =

>   trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.ja=
va 1025790 =

>   trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java 1025790 =

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

> =

> Diff: http://review.cloudera.org/r/1036/diff
> =

> =

> Testing
> -------
> =

> =

> Thanks,
> =

> Jonathan
> =

>