hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stack <st...@duboce.net>
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/
Date Sat, 14 Mar 2009 05:00:56 GMT
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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message