HBASE-13546 handle nulls in MasterAddressTracker when there is no master active.

hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java

@@ -84,7 +84,11 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
    */
   public int getMasterInfoPort() {
     try {
-      return parse(this.getData(false)).getInfoPort();
+      final ZooKeeperProtos.Master master = parse(this.getData(false));
+      if (master == null) {
+        return 0;
+      }
+      return master.getInfoPort();
     } catch (DeserializationException e) {
       LOG.warn("Failed parse master zk node data", e);
       return 0;
@@ -92,7 +96,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
   }
   /**
    * Get the info port of the backup master if it is available.
-   * Return 0 if no current master or zookeeper is unavailable
+   * Return 0 if no backup master or zookeeper is unavailable
    * @param sn server name of backup master
    * @return info port or 0 if timed out or exceptions
    */
@@ -100,7 +104,11 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
     String backupZNode = ZKUtil.joinZNode(watcher.backupMasterAddressesZNode, sn.toString());
     try {
       byte[] data = ZKUtil.getData(watcher, backupZNode);
-      return parse(data).getInfoPort();
+      final ZooKeeperProtos.Master backup = parse(data);
+      if (backup == null) {
+        return 0;
+      }
+      return backup.getInfoPort();
     } catch (Exception e) {
       LOG.warn("Failed to get backup master: " + sn + "'s info port.", e);
       return 0;
@@ -142,6 +150,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
     } catch (InterruptedException e) {
       throw new InterruptedIOException();
     }
+    // TODO javadoc claims we return null in this case. :/
     if (data == null){
       throw new IOException("Can't get master address from ZooKeeper; znode data == null");
     }
@@ -161,6 +170,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
    * @param zkw ZooKeeperWatcher to use
    * @return master info port in the the master address znode or null if no
    * znode present.
+   * // TODO can't return null for 'int' return type. non-static verison returns 0
    * @throws KeeperException
    * @throws IOException
    */
@@ -172,6 +182,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
     } catch (InterruptedException e) {
       throw new InterruptedIOException();
     }
+    // TODO javadoc claims we return null in this case. :/
     if (data == null) {
       throw new IOException("Can't get master address from ZooKeeper; znode data == null");
     }
@@ -191,7 +202,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
    * @param zkw The ZooKeeperWatcher to use.
    * @param znode Where to create the znode; could be at the top level or it
    * could be under backup masters
-   * @param master ServerName of the current master
+   * @param master ServerName of the current master must not be null.
    * @return true if node created, false if not; a watch is set in both cases
    * @throws KeeperException
    */
@@ -210,7 +221,7 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
   }
 
   /**
-   * @param sn
+   * @param sn must not be null
    * @return Content of the master znode as a serialized pb with the pb
    * magic as prefix.
    */
@@ -227,11 +238,14 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
   }
 
   /**
-   * @param data zookeeper data
+   * @param data zookeeper data. may be null
-   * @return pb object of master
+   * @return pb object of master, null if no active master
    * @throws DeserializationException
    */
   public static ZooKeeperProtos.Master parse(byte[] data) throws DeserializationException {
+    if (data == null) {
+      return null;
+    }
     int prefixLen = ProtobufUtil.lengthOfPBMagic();
     try {
       return ZooKeeperProtos.Master.PARSER.parseFrom(data, prefixLen, data.length - prefixLen);
@@ -241,6 +255,8 @@ public class MasterAddressTracker extends ZooKeeperNodeTracker {
   }
   /**
    * delete the master znode if its content is same as the parameter
+   * @param zkw must not be null
+   * @param content must not be null
    */
   public static boolean deleteIfEquals(ZooKeeperWatcher zkw, final String content) {
     if (content == null){

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java

@@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.regionserver;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.util.concurrent.Semaphore;
@@ -36,6 +37,8 @@ import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.Rule;
+import org.junit.rules.TestName;
 import org.junit.experimental.categories.Category;
 
 @Category({RegionServerTests.class, MediumTests.class})
@@ -44,6 +47,9 @@ public class TestMasterAddressTracker {
 
   private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
 
+  @Rule
+  public TestName name = new TestName();
+
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
     TEST_UTIL.startMiniZKCluster();
@@ -53,16 +59,29 @@ public class TestMasterAddressTracker {
   public static void tearDownAfterClass() throws Exception {
     TEST_UTIL.shutdownMiniZKCluster();
   }
-  /**
-   * Unit tests that uses ZooKeeper but does not use the master-side methods
-   * but rather acts directly on ZK.
-   * @throws Exception
-   */
-  @Test
-  public void testMasterAddressTrackerFromZK() throws Exception {
 
+  @Test
+  public void testDeleteIfEquals() throws Exception {
+    final ServerName sn = ServerName.valueOf("localhost", 1234, System.currentTimeMillis());
+    final MasterAddressTracker addressTracker = setupMasterTracker(sn, 1772);
+    try {
+      assertFalse("shouldn't have deleted wrong master server.",
+          MasterAddressTracker.deleteIfEquals(addressTracker.getWatcher(), "some other string."));
+    } finally {
+      assertTrue("Couldn't clean up master",
+          MasterAddressTracker.deleteIfEquals(addressTracker.getWatcher(), sn.toString()));
+    }
+  }
+
+  /**
+   * create an address tracker instance
+   * @param sn if not-null set the active master
+   * @param infoPort if there is an active master, set its info port.
+   */
+  private MasterAddressTracker setupMasterTracker(final ServerName sn, final int infoPort)
+      throws Exception {
     ZooKeeperWatcher zk = new ZooKeeperWatcher(TEST_UTIL.getConfiguration(),
-        "testMasterAddressTrackerFromZK", null);
+        name.getMethodName(), null);
     ZKUtil.createAndFailSilent(zk, zk.baseZNode);
 
     // Should not have a master yet
@@ -75,11 +94,7 @@ public class TestMasterAddressTracker {
     NodeCreationListener listener = new NodeCreationListener(zk, zk.getMasterAddressZNode());
     zk.registerListener(listener);
 
-    // Create the master node with a dummy address
+    if (sn != null) {
-    String host = "localhost";
-    int port = 1234;
-    int infoPort = 1235;
-    ServerName sn = ServerName.valueOf(host, port, System.currentTimeMillis());
       LOG.info("Creating master node");
       MasterAddressTracker.setMasterAddress(zk, zk.getMasterAddressZNode(), sn, infoPort);
 
@@ -87,10 +102,58 @@ public class TestMasterAddressTracker {
       LOG.info("Waiting for master address manager to be notified");
       listener.waitForCreation();
       LOG.info("Master node created");
+    }
+    return addressTracker;
+  }
+
+  /**
+   * Unit tests that uses ZooKeeper but does not use the master-side methods
+   * but rather acts directly on ZK.
+   * @throws Exception
+   */
+  @Test
+  public void testMasterAddressTrackerFromZK() throws Exception {
+    // Create the master node with a dummy address
+    final int infoPort = 1235;
+    final ServerName sn = ServerName.valueOf("localhost", 1234, System.currentTimeMillis());
+    final MasterAddressTracker addressTracker = setupMasterTracker(sn, infoPort);
+    try {
       assertTrue(addressTracker.hasMaster());
       ServerName pulledAddress = addressTracker.getMasterAddress();
       assertTrue(pulledAddress.equals(sn));
       assertEquals(infoPort, addressTracker.getMasterInfoPort());
+    } finally {
+      assertTrue("Couldn't clean up master",
+          MasterAddressTracker.deleteIfEquals(addressTracker.getWatcher(), sn.toString()));
+    }
+  }
+
+
+  @Test
+  public void testParsingNull() throws Exception {
+    assertNull("parse on null data should return null.", MasterAddressTracker.parse(null));
+  }
+
+  @Test
+  public void testNoBackups() throws Exception {
+    final ServerName sn = ServerName.valueOf("localhost", 1234, System.currentTimeMillis());
+    final MasterAddressTracker addressTracker = setupMasterTracker(sn, 1772);
+    try {
+      assertEquals("Should receive 0 for backup not found.", 0,
+          addressTracker.getBackupMasterInfoPort(
+              ServerName.valueOf("doesnotexist.example.com", 1234, System.currentTimeMillis())));
+    } finally {
+      assertTrue("Couldn't clean up master",
+          MasterAddressTracker.deleteIfEquals(addressTracker.getWatcher(), sn.toString()));
+    }
+  }
+
+  @Test
+  public void testNoMaster() throws Exception {
+    final MasterAddressTracker addressTracker = setupMasterTracker(null, 1772);
+    assertFalse(addressTracker.hasMaster());
+    assertNull("should get null master when none active.", addressTracker.getMasterAddress());
+    assertEquals("Should receive 0 for backup not found.", 0, addressTracker.getMasterInfoPort());
   }
 
   public static class NodeCreationListener extends ZooKeeperListener {