From cd2a1fe4f42f12f614a58f5113507946fe39277d Mon Sep 17 00:00:00 2001 From: epopevad Date: Sun, 16 Oct 2016 00:21:47 -0700 Subject: [PATCH] Update the ServerManager.drainingServers as appropriate when new Region Servers are added. --- .../apache/hadoop/hbase/master/ServerManager.java | 2 +- .../hbase/zookeeper/DrainingServerTracker.java | 18 +++ .../hadoop/hbase/master/TestServerManager.java | 147 +++++++++++++++++++++ 3 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestServerManager.java diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java index 305687f..df1cea0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java @@ -527,7 +527,6 @@ public class ServerManager { } } - public DeadServer getDeadServers() { return this.deadservers; } @@ -738,6 +737,7 @@ public class ServerManager { "Ignoring request to add it again."); return false; } + LOG.info("Server " + sn + " added to draining server list."); return this.drainingServers.add(sn); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/DrainingServerTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/DrainingServerTracker.java index 5969143..413f226 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/DrainingServerTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/DrainingServerTracker.java @@ -27,6 +27,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.master.ServerListener; import org.apache.hadoop.hbase.master.ServerManager; import org.apache.zookeeper.KeeperException; @@ -68,6 +69,23 @@ public class DrainingServerTracker extends ZooKeeperListener { */ public void start() throws KeeperException, IOException { watcher.registerListener(this); + // Add a ServerListener to check if a server is draining when it's added. + serverManager.registerListener( + new ServerListener() { + + @Override + public void serverAdded(ServerName sn) { + if (drainingServers.contains(sn)){ + serverManager.addServerToDrainList(sn); + } + } + + @Override + public void serverRemoved(ServerName serverName) { + // no-op + } + } + ); List servers = ZKUtil.listChildrenAndWatchThem(watcher, watcher.drainingZNode); add(servers); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestServerManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestServerManager.java new file mode 100644 index 0000000..ac5a8a2 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestServerManager.java @@ -0,0 +1,147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.master; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.*; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.zookeeper.DrainingServerTracker; +import org.apache.hadoop.hbase.zookeeper.RegionServerTracker; +import org.apache.hadoop.hbase.zookeeper.ZKUtil; +import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; +import org.apache.hadoop.metrics2.util.Servers; +import org.junit.Assert; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.mockito.Mockito; + +import java.util.ArrayList; +import java.util.HashMap; + +/** + * Test changes in state of the ServerManager. + */ +@Category(MediumTests.class) +@SuppressWarnings("deprecation") +public class TestServerManager { + private static final Log LOG = LogFactory.getLog(TestServerManager.class); + private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + private static final Configuration conf = TEST_UTIL.getConfiguration(); + private static final String baseZNode = conf.get(HConstants.ZOOKEEPER_ZNODE_PARENT, + HConstants.DEFAULT_ZOOKEEPER_ZNODE_PARENT); + private static final String drainingZNode = ZKUtil.joinZNode(baseZNode, + conf.get("zookeeper.znode.draining.rs", "draining")); + + private final ServerName SERVERNAME_A = ServerName.valueOf("mockserverbulk_a.org", 1000, 8000); + private final ServerName SERVERNAME_B = ServerName.valueOf("mockserverbulk_b.org", 1001, 8000); + private final ServerName SERVERNAME_C = ServerName.valueOf("mockserverbulk_c.org", 1002, 8000); + + private static final Abortable abortable = new Abortable() { + @Override + public boolean isAborted() { + return false; + } + + @Override + public void abort(String why, Throwable e) { + } + }; + + @AfterClass + public static void afterClass() throws Exception { + TEST_UTIL.shutdownMiniZKCluster(); + } + + @BeforeClass + public static void beforeClass() throws Exception { + TEST_UTIL.getConfiguration().setBoolean("hbase.assignment.usezk", true); + TEST_UTIL.startMiniZKCluster(); + } + + @Test + public void testAddNewServerThatExistsInDraining() throws Exception { + // Under certain circumstances, such as when we failover to the Backup + // HMaster, the DrainingServerTracker is started with existing servers in + // draining before all of the Region Servers register with the + // ServerManager as "online". This test is to ensure that Region Servers + // are properly added to the ServerManager.drainingServers when they + // register with the ServerManager under these circumstances. + ZooKeeperWatcher zooKeeper = new ZooKeeperWatcher(conf, + "zkWatcher-NewServerDrainTest", abortable, true); + MasterServices services = Mockito.mock(MasterServices.class); + HMaster master = Mockito.mock(HMaster.class); + Mockito.when(master.getConfiguration()).thenReturn(conf); + + // We'll start with 2 servers in draining that existed before the + // HMaster started. + ArrayList drainingServers = new ArrayList(); + drainingServers.add(SERVERNAME_A); + drainingServers.add(SERVERNAME_B); + + // We'll have 2 servers that come online AFTER the DrainingServerTracker + // is started (just as we see when we failover to the Backup HMaster). + // One of these will already be a draining server. + HashMap onlineServers = new HashMap(); + onlineServers.put(SERVERNAME_A, ServerLoad.EMPTY_SERVERLOAD); + onlineServers.put(SERVERNAME_C, ServerLoad.EMPTY_SERVERLOAD); + + // Create draining znodes for the draining servers, which would have been + // performed when the previous HMaster was running. + for (ServerName sn : drainingServers) { + String znode = ZKUtil.joinZNode(drainingZNode, sn.getServerName()); + ZKUtil.createAndFailSilent(zooKeeper, znode); + } + + // Now, we follow the same order of steps that the HMaster does to setup + // the ServerManager, RegionServerTracker, and DrainingServerTracker. + ServerManager serverManager = new ServerManager(master, services); + + RegionServerTracker regionServerTracker = new RegionServerTracker( + zooKeeper, master, serverManager); + regionServerTracker.start(); + + DrainingServerTracker drainingServerTracker = new DrainingServerTracker( + zooKeeper, master, serverManager); + drainingServerTracker.start(); + + // Confirm our ServerManager lists are empty. + Assert.assertEquals(serverManager.getOnlineServers(), + new HashMap()); + Assert.assertEquals(serverManager.getDrainingServersList(), + new ArrayList()); + + // checkAndRecordNewServer() is how servers are added to the ServerManager. + ArrayList onlineDrainingServers = new ArrayList(); + for (ServerName sn : onlineServers.keySet()){ + // Here's the actual test. + serverManager.checkAndRecordNewServer(sn, onlineServers.get(sn)); + if (drainingServers.contains(sn)){ + onlineDrainingServers.add(sn); // keeping track for later verification + } + } + + // Verify the ServerManager lists are correctly updated. + Assert.assertEquals(serverManager.getOnlineServers(), onlineServers); + Assert.assertEquals(serverManager.getDrainingServersList(), + onlineDrainingServers); + } +} \ No newline at end of file -- 2.8.0-rc2