From 9eecd43013b71d461350540564a357961fb25d60 Mon Sep 17 00:00:00 2001 From: Sean Busbey Date: Thu, 23 Apr 2015 22:51:35 -0500 Subject: [PATCH] HBASE-13546 handle nulls in MasterAddressTracker when there is no master active. --- .../hbase/zookeeper/MasterAddressTracker.java | 30 +++-- .../TestMasterAddressTracker.java | 109 ++++++++++++++---- 2 files changed, 109 insertions(+), 30 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java index 1a538bead70..6f4859ad621 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java +++ b/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 - * @return pb object of master + * @param data zookeeper data. may be null + * @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){ diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java index 4c4c94096bf..487bb25c146 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java +++ b/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,22 +94,66 @@ public class TestMasterAddressTracker { NodeCreationListener listener = new NodeCreationListener(zk, zk.getMasterAddressZNode()); zk.registerListener(listener); - // Create the master node with a dummy address - 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); + if (sn != null) { + LOG.info("Creating master node"); + MasterAddressTracker.setMasterAddress(zk, zk.getMasterAddressZNode(), sn, infoPort); - // Wait for the node to be created - LOG.info("Waiting for master address manager to be notified"); - listener.waitForCreation(); - LOG.info("Master node created"); - assertTrue(addressTracker.hasMaster()); - ServerName pulledAddress = addressTracker.getMasterAddress(); - assertTrue(pulledAddress.equals(sn)); - assertEquals(infoPort, addressTracker.getMasterInfoPort()); + // Wait for the node to be created + 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 {