On Fri, Mar 13, 2009 at 6:38 PM, <[email protected]> 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
