Mailing-List: contact hbase-dev-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: hbase-dev@hadoop.apache.org
Received-SPF: pass (athena.apache.org: domain of saint.ack@gmail.com
 designates 209.85.132.247 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:sender:in-reply-to:references:date
         :x-google-sender-auth:message-id:subject:from:to:content-type;
        b=Z9gJJucMtesS2rlsUItLLGtKTCkJENdbGc7ZLh1tZyNkmDXBjUay5aZ0IuraGSUhMy
         nh8aOdthGarZCXfM3gSLODf8BNrgwpXo8aj+imslK26Z11bim1gOcMn9ShPy79iRA+PK
         EqcxttU0hmemtp7aEUnVqguyZMtIJEtha5W/w=
MIME-Version: 1.0
Sender: saint.ack@gmail.com
In-Reply-To: <20090314013846.DF9BC238898B@eris.apache.org>
References: <20090314013846.DF9BC238898B@eris.apache.org>
Date: Fri, 13 Mar 2009 22:00:56 -0700
Message-ID: <7c962aed0903132200n1e984087la04e45a733770e8e@mail.gmail.com>
Subject: Re: svn commit: r753483 - in /hadoop/hbase/trunk: ./
	src/java/org/apache/hadoop/hbase/ src/java/org/apache/hadoop/hbase/master/
	src/java/org/apache/hadoop/hbase/regionserver/
From: stack <stack@duboce.net>
To: hbase-dev@hadoop.apache.org
Content-Type: multipart/alternative; boundary=0016368e205645379a04650d1a73

--0016368e205645379a04650d1a73
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On Fri, Mar 13, 2009 at 6:38 PM, <jimk@apache.org> wrote:Modified:
hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionInfo.java

> URL:
> http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionInfo.java?rev=753483&r1=753482&r2=753483&view=diff
>
> ==============================================================================
> --- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionInfo.java
> (original)
> +++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HRegionInfo.java
> Sat Mar 14 01:38:45 2009
> @@ -23,7 +23,6 @@
>  import java.io.DataOutput;
>  import java.io.IOException;
>
> -import org.apache.hadoop.hbase.HStoreKey;
>  import org.apache.hadoop.hbase.util.Bytes;
>  import org.apache.hadoop.hbase.util.JenkinsHash;
>  import org.apache.hadoop.io.VersionedWritable;
> @@ -59,7 +58,7 @@
>   private byte [] endKey = HConstants.EMPTY_BYTE_ARRAY;
>   private boolean offLine = false;
>   private long regionId = -1;
> -  private byte [] regionName = HConstants.EMPTY_BYTE_ARRAY;
> +  private transient byte [] regionName = HConstants.EMPTY_BYTE_ARRAY;


Did you mean volatile rather than transient?  (I thought transient only
applied when using java serialization?)


> Modified:
> hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HServerInfo.java
> URL:
> http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HServerInfo.java?rev=753483&r1=753482&r2=753483&view=diff
>
> ==============================================================================
> --- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HServerInfo.java
> (original)
> +++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/HServerInfo.java
> Sat Mar 14 01:38:45 2009
> @@ -38,6 +38,7 @@
>   private long startCode;
>   private HServerLoad load;
>   private int infoPort;
> +  private transient volatile String serverName = null;


Do you need the transient here?


> +  private static String getServerName(String hostName, int port, long
> startCode) {
> +    StringBuilder name = new StringBuilder(hostName);
> +    name.append("_");
> +    name.append(startCode);
> +    name.append("_");
> +    name.append(port);
> +    return name.toString();
>   }
>  }
>

Above seems a little odd?  Usually port follows host?  Usually a ':'
delimiter?


> -          if (!master.regionManager.regionIsOpening(i.getRegionName())) {
> +          if
> (!master.regionManager.regionIsOpening(i.getRegionNameAsString())) {


Any particular reason for move to String here and elsewhere in the patch?

In general, I'd be interested in notes on how well this patch was tested.
IIRC, a bunch of the code removed -- ghost reference checker, etc. -- was
put in place to handle the case where a large amount of logs after server
crash and then the crashed server is brought back on line near immediately.
That the above code passes unit tests, the justification for commit given in
the JIRA, says nothing about how this code will actually work in the
scenario its meant to handle.

Thanks,
St.Ack

--0016368e205645379a04650d1a73--