HBASE-3827 hbase-1502, removing heartbeats, broke master joining a running cluster and was returning master hostname for rs to use

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1097587 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -89,6 +89,8 @@ Release 0.91.0 - Unreleased
    HBASE-3819  TestSplitLogWorker has too many SLWs running -- makes for
                contention and occasional failures
    HBASE-3210  HBASE-1921 for the new master
+   HBASE-3827  hbase-1502, removing heartbeats, broke master joining a running
+               cluster and was returning master hostname for rs to use
 
   IMPROVEMENTS
    HBASE-3290  Max Compaction Size (Nicolas Spiegelberg via Stack)  

src/main/java/org/apache/hadoop/hbase/ServerName.java

@@ -211,11 +211,21 @@ public class ServerName implements Comparable<ServerName> {
   public static ServerName findServerWithSameHostnamePort(final Collection<ServerName> names,
       final ServerName serverName) {
     for (ServerName sn: names) {
-      if (sn.getHostname().equals(serverName.getHostname()) &&
+      if (isSameHostnameAndPort(serverName, sn)) return sn;
-          sn.getPort() == serverName.getPort()) {
-        return sn;
-      }
     }
     return null;
   }
+
+  /**
+   * @param left
+   * @param rigth
+   * @return True if <code>other</code> has same hostname and port.
+   */
+  public static boolean isSameHostnameAndPort(final ServerName left,
+      final ServerName right) {
+    if (left == null) return false;
+    if (right == null) return false;
+    return left.getHostname().equals(right.getHostname()) &&
+      left.getPort() == right.getPort();
+  }
 }

src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

@@ -142,7 +142,7 @@ class ActiveMasterManager extends ZooKeeperListener {
       byte [] bytes =
         ZKUtil.getDataAndWatch(this.watcher, this.watcher.masterAddressZNode);
       ServerName currentMaster = new ServerName(Bytes.toString(bytes));
-      if (currentMaster != null && currentMaster.equals(this.sn)) {
+      if (ServerName.isSameHostnameAndPort(currentMaster, this.sn)) {
         LOG.info("Current master has this master's address, " + currentMaster +
           "; master was restarted?  Waiting on znode to expire...");
         // Hurry along the expiration of the znode.

src/main/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -22,6 +22,7 @@ package org.apache.hadoop.hbase.master;
 import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -636,12 +637,13 @@ implements HMasterInterface, HMasterRegionInterface, MasterServices, Server {
     final long serverStartCode, final long serverCurrentTime)
   throws IOException {
     // Register with server manager
-    this.serverManager.regionServerStartup(HBaseServer.getRemoteIp(), port,
+    InetAddress ia = HBaseServer.getRemoteIp();
+    ServerName rs = this.serverManager.regionServerStartup(ia, port,
       serverStartCode, serverCurrentTime);
     // Send back some config info
     MapWritable mw = createConfigurationSubset();
     mw.put(new Text(HConstants.KEY_FOR_HOSTNAME_SEEN_BY_MASTER),
-      new Text(this.serverName.getHostname()));
+      new Text(rs.getHostname()));
     return mw;
   }
 
@@ -660,7 +662,7 @@ implements HMasterInterface, HMasterRegionInterface, MasterServices, Server {
   }
 
   @Override
-  public void regionServerReport(byte[] sn, HServerLoad hsl)
+  public void regionServerReport(final byte [] sn, final HServerLoad hsl)
   throws IOException {
     this.serverManager.regionServerReport(new ServerName(sn), hsl);
     if (hsl != null && this.metrics != null) {

src/main/java/org/apache/hadoop/hbase/master/ServerManager.java

@@ -104,9 +104,10 @@ public class ServerManager {
    * @param port The remote port
    * @param serverStartcode
    * @param serverCurrentTime The current time of the region server in ms
+   * @return The ServerName we know this server as.
    * @throws IOException
    */
-  void regionServerStartup(final InetAddress ia, final int port,
+  ServerName regionServerStartup(final InetAddress ia, final int port,
     final long serverStartcode, long serverCurrentTime)
   throws IOException {
     // Test for case where we get a region startup message from a regionserver
@@ -121,6 +122,7 @@ public class ServerManager {
     checkIsDead(sn, "STARTUP");
     checkAlreadySameHostPort(sn);
     recordNewServer(sn, HServerLoad.EMPTY_HSERVERLOAD);
+    return sn;
   }
 
   void regionServerReport(ServerName sn, HServerLoad hsl)

src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

@@ -44,8 +44,6 @@ import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentSkipListSet;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -69,7 +67,6 @@ import org.apache.hadoop.hbase.KeyValue;
 import org.apache.hadoop.hbase.MasterAddressTracker;
 import org.apache.hadoop.hbase.NotServingRegionException;
 import org.apache.hadoop.hbase.RemoteExceptionHandler;
-import org.apache.hadoop.hbase.Server;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.Stoppable;
 import org.apache.hadoop.hbase.UnknownRowLockException;

src/main/java/org/apache/hadoop/hbase/zookeeper/RootRegionTracker.java

@@ -22,6 +22,7 @@ package org.apache.hadoop.hbase.zookeeper;
 import org.apache.hadoop.hbase.Abortable;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.catalog.RootLocationEditor;
+import org.apache.hadoop.hbase.util.Addressing;
 import org.apache.hadoop.hbase.util.Bytes;
 
 /**
@@ -57,8 +58,7 @@ public class RootRegionTracker extends ZooKeeperNodeTracker {
    * @throws InterruptedException 
    */
   public ServerName getRootRegionLocation() throws InterruptedException {
-    byte [] data = super.getData();
+    return dataToServerName(super.getData());
-    return data == null? null: new ServerName(dataToString(data));
   }
 
   /**
@@ -71,16 +71,28 @@ public class RootRegionTracker extends ZooKeeperNodeTracker {
    */
   public ServerName waitRootRegionLocation(long timeout)
   throws InterruptedException {
-    String str = dataToString(super.blockUntilAvailable(timeout));
+    return dataToServerName(super.blockUntilAvailable(timeout));
-    return str == null? null: new ServerName(str);
   }
 
   /*
    * @param data
    * @return Returns null if <code>data</code> is null else converts passed data
-   * to a String instance.
+   * to a ServerName instance.
    */
-  private static String dataToString(final byte [] data) {
+  private static ServerName dataToServerName(final byte [] data) {
-    return data == null ? null: Bytes.toString(data);
+    // The str returned could be old style -- pre hbase-1502 -- which was
+    // hostname and port seperated by a colon rather than hostname, port and
+    // startcode delimited by a ','.
+    if (data == null || data.length <= 0) return null;
+    String str = Bytes.toString(data);
+    int index = str.indexOf(ServerName.SERVERNAME_SEPARATOR);
+    if (index != -1) {
+      // Presume its ServerName.toString() format.
+      return new ServerName(str);
+    }
+    // Presume it a hostname:port format.
+    String hostname = Addressing.parseHostname(str);
+    int port = Addressing.parsePort(str);
+    return new ServerName(hostname, port, -1L);
   }
 }