Repository: hbase Updated Branches: refs/heads/branch-1 42ad84503 -> 1a0c61c97
HBASE-13546 handle nulls in MasterAddressTracker when there is no master active. Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/1a0c61c9 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/1a0c61c9 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/1a0c61c9 Branch: refs/heads/branch-1 Commit: 1a0c61c9745da3c601acf39a5b13e5af94a8d58b Parents: 42ad845 Author: Sean Busbey <[email protected]> Authored: Thu Apr 23 22:51:35 2015 -0500 Committer: Sean Busbey <[email protected]> Committed: Fri Apr 24 13:54:36 2015 -0500 ---------------------------------------------------------------------- .../hbase/zookeeper/MasterAddressTracker.java | 30 ++++-- .../regionserver/TestMasterAddressTracker.java | 107 +++++++++++++++---- 2 files changed, 108 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/1a0c61c9/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java ---------------------------------------------------------------------- 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 6ffb813..311202c 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 @@ -83,7 +83,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; @@ -91,7 +95,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 */ @@ -99,7 +103,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; @@ -141,6 +149,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"); } @@ -160,6 +169,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 */ @@ -171,6 +181,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"); } @@ -190,7 +201,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 */ @@ -209,7 +220,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. */ @@ -226,11 +237,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); @@ -240,6 +254,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){ http://git-wip-us.apache.org/repos/asf/hbase/blob/1a0c61c9/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMasterAddressTracker.java ---------------------------------------------------------------------- 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 f205eec..fd8c4dc 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; @@ -35,6 +36,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(MediumTests.class) @@ -43,6 +46,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(); @@ -52,16 +58,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 { + 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 @@ -74,22 +93,66 @@ public class TestMasterAddressTracker { NodeCreationListener listener = new NodeCreationListener(zk, zk.getMasterAddressZNode()); zk.registerListener(listener); + 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"); + } + 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 - 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); - - // 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()); + 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 {
