Repository: hbase
Updated Branches:
  refs/heads/branch-1 b7f283c6f -> a4cbdede3


HBASE-16853 Regions are assigned to Region Servers in /hbase/draining after 
HBase Master failover (David Pope)


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/a4cbdede
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/a4cbdede
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/a4cbdede

Branch: refs/heads/branch-1
Commit: a4cbdede31f97bb988daf944f603f9f25b368044
Parents: b7f283c
Author: tedyu <yuzhih...@gmail.com>
Authored: Sun Oct 16 18:54:59 2016 -0700
Committer: tedyu <yuzhih...@gmail.com>
Committed: Sun Oct 16 18:54:59 2016 -0700

----------------------------------------------------------------------
 .../hadoop/hbase/master/ServerManager.java      |   2 +-
 .../hbase/zookeeper/DrainingServerTracker.java  |  18 ++++
 .../hbase/master/TestAssignmentListener.java    | 100 +++++++++++++++++++
 3 files changed, 119 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/a4cbdede/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.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 bcaa4d0..1817d6e 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
@@ -533,7 +533,6 @@ public class ServerManager {
     }
   }
 
-
   public DeadServer getDeadServers() {
     return this.deadservers;
   }
@@ -744,6 +743,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);
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/a4cbdede/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/DrainingServerTracker.java
----------------------------------------------------------------------
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<String> servers =
       ZKUtil.listChildrenAndWatchThem(watcher, watcher.drainingZNode);
     add(servers);

http://git-wip-us.apache.org/repos/asf/hbase/blob/a4cbdede/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentListener.java
----------------------------------------------------------------------
diff --git 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentListener.java
 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentListener.java
index 6257fe5..f171821 100644
--- 
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentListener.java
+++ 
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentListener.java
@@ -21,16 +21,22 @@ package org.apache.hadoop.hbase.master;
 import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.Abortable;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.MiniHBaseCluster;
 import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.ServerLoad;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.HTable;
@@ -40,16 +46,32 @@ import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.regionserver.Region;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.JVMClusterUtil;
+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.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.mockito.Mockito;
 
 @Category(MediumTests.class)
 public class TestAssignmentListener {
   private static final Log LOG = 
LogFactory.getLog(TestAssignmentListener.class);
 
   private static final HBaseTestingUtility TEST_UTIL = new 
HBaseTestingUtility();
+  private static final Abortable abortable = new Abortable() {
+    @Override
+    public boolean isAborted() {
+      return false;
+    }
+
+    @Override
+    public void abort(String why, Throwable e) {
+    }
+  };
 
   static class DummyListener {
     protected AtomicInteger modified = new AtomicInteger(0);
@@ -260,4 +282,82 @@ public class TestAssignmentListener {
       am.unregisterListener(listener);
     }
   }
+
+  @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.
+    Configuration conf = TEST_UTIL.getConfiguration();
+    ZooKeeperWatcher zooKeeper = new ZooKeeperWatcher(conf,
+        "zkWatcher-NewServerDrainTest", abortable, true);
+    String baseZNode = conf.get(HConstants.ZOOKEEPER_ZNODE_PARENT,
+        HConstants.DEFAULT_ZOOKEEPER_ZNODE_PARENT);
+    String drainingZNode = ZKUtil.joinZNode(baseZNode,
+        conf.get("zookeeper.znode.draining.rs", "draining"));
+
+    MasterServices services = Mockito.mock(MasterServices.class);
+    HMaster master = Mockito.mock(HMaster.class);
+    Mockito.when(master.getConfiguration()).thenReturn(conf);
+
+    ServerName SERVERNAME_A = ServerName.valueOf("mockserverbulk_a.org", 1000, 
8000);
+    ServerName SERVERNAME_B = ServerName.valueOf("mockserverbulk_b.org", 1001, 
8000);
+    ServerName SERVERNAME_C = ServerName.valueOf("mockserverbulk_c.org", 1002, 
8000);
+
+    // We'll start with 2 servers in draining that existed before the
+    // HMaster started.
+    ArrayList<ServerName> drainingServers = new ArrayList<ServerName>();
+    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<ServerName, ServerLoad> onlineServers = new HashMap<ServerName, 
ServerLoad>();
+    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<ServerName, ServerLoad>());
+    Assert.assertEquals(serverManager.getDrainingServersList(),
+        new ArrayList<ServerName>());
+
+    // checkAndRecordNewServer() is how servers are added to the ServerManager.
+    ArrayList<ServerName> onlineDrainingServers = new ArrayList<ServerName>();
+    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);
+  }
 }

Reply via email to