HBASE-18511 Default no regions on master

Changes the configuration hbase.balancer.tablesOnMaster from list of
table names to instead be a boolean; true if master carries
tables/regions and false if it does not.

Adds a new configuration hbase.balancer.tablesOnMaster.systemTablesOnly.
If true, hbase.balancer.tablesOnMaster is considered true but only
system tables are put on the master.

M hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
  Master was claiming itself active master though it had stopped. Fix
the activeMaster flag. Set it to false on exit.

M hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
 Add new configs and convenience methods for getting current state of
settings.

M hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
 Move configs up into super Interface and now the settings mean
different, remove the no longer needed processing.

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -530,6 +530,17 @@ public HMaster(final Configuration conf, CoordinatedStateManager csm)
     }
   }
 
+  // Main run loop. Calls through to the regionserver run loop.
+  @Override
+  public void run() {
+    try {
+      super.run();
+    } finally {
+      // If on way out, then we are no longer active master.
+      this.activeMaster = false;
+    }
+  }
+
   // return the actual infoPort, -1 means disable info server.
   private int putUpJettyServer() throws IOException {
     if (!conf.getBoolean("hbase.master.infoserver.redirect", true)) {
@@ -604,9 +615,8 @@ protected void login(UserProvider user, String host) throws IOException {
    */
   @Override
   protected void waitForMasterActive(){
-    boolean tablesOnMaster = BaseLoadBalancer.tablesOnMaster(conf);
-    while (!(tablesOnMaster && activeMaster)
-        && !isStopped() && !isAborted()) {
+    boolean tablesOnMaster = LoadBalancer.isTablesOnMaster(conf);
+    while (!(tablesOnMaster && activeMaster) && !isStopped() && !isAborted()) {
       sleeper.sleep();
     }
   }
@@ -644,7 +654,7 @@ protected RSRpcServices createRpcServices() throws IOException {
   protected void configureInfoServer() {
     infoServer.addServlet("master-status", "/master-status", MasterStatusServlet.class);
     infoServer.setAttribute(MASTER, this);
-    if (BaseLoadBalancer.tablesOnMaster(conf)) {
+    if (LoadBalancer.isTablesOnMaster(conf)) {
       super.configureInfoServer();
     }
   }
@@ -796,14 +806,16 @@ private void finishActiveMasterInitialization(MonitoredTask status)
     sleeper.skipSleepCycle();
 
     // Wait for region servers to report in
-    status.setStatus("Wait for region servers to report in");
+    String statusStr = "Wait for region servers to report in";
+    status.setStatus(statusStr);
+    LOG.info(status);
     waitForRegionServers(status);
 
     if (this.balancer instanceof FavoredNodesPromoter) {
       favoredNodesManager = new FavoredNodesManager(this);
     }
     // Wait for regionserver to finish initialization.
-    if (BaseLoadBalancer.tablesOnMaster(conf)) {
+    if (LoadBalancer.isTablesOnMaster(conf)) {
       waitForServerOnline();
     }
 
@@ -1643,11 +1655,11 @@ public void move(final byte[] encodedRegionName, byte[] destServerName) throws H
         LOG.debug("Unable to determine a plan to assign " + hri);
         return;
       }
+      // TODO: What is this? I don't get it.
       if (dest.equals(serverName) && balancer instanceof BaseLoadBalancer
           && !((BaseLoadBalancer)balancer).shouldBeOnMaster(hri)) {
         // To avoid unnecessary region moving later by balancer. Don't put user
-        // regions on master. Regions on master could be put on other region
-        // server intentionally by test however.
+        // regions on master.
         LOG.debug("Skipping move of region " + hri.getRegionNameAsString()
           + " to avoid unnecessary region moving later by load balancer,"
           + " because it should not be on master");

hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java

@@ -18,9 +18,10 @@
  */
 package org.apache.hadoop.hbase.master;
 
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.UnmodifiableIterator;
 import org.apache.hadoop.conf.Configurable;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterStatus;
@@ -33,6 +34,9 @@
 import org.apache.hadoop.hbase.conf.ConfigurationObserver;
 
 import edu.umd.cs.findbugs.annotations.Nullable;
+import org.apache.hadoop.hbase.security.access.AccessControlLists;
+import org.apache.hadoop.hbase.shaded.com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.util.StringUtils;
 
 /**
  * Makes decisions about the placement and movement of Regions across
@@ -50,6 +54,18 @@
  */
 @InterfaceAudience.Private
 public interface LoadBalancer extends Configurable, Stoppable, ConfigurationObserver {
+  /**
+   * Master can carry regions as of hbase-2.0.0.
+   * By default, it carries no tables.
+   * TODO: Add any | system as flags to indicate what it can do.
+   */
+  public static final String TABLES_ON_MASTER = "hbase.balancer.tablesOnMaster";
+
+  /**
+   * Master carries system tables.
+   */
+  public static final String SYSTEM_TABLES_ON_MASTER =
+    "hbase.balancer.tablesOnMaster.systemTablesOnly";
 
   // Used to signal to the caller that the region(s) cannot be assigned
   // We deliberately use 'localhost' so the operation will fail fast
@@ -147,4 +163,15 @@ ServerName randomAssignment(
    * @param conf
    */
   void onConfigurationChange(Configuration conf);
+
+  /**
+   * @return true if Master carries regions
+   */
+  static boolean isTablesOnMaster(Configuration conf) {
+    return conf.getBoolean(TABLES_ON_MASTER, false);
+  }
+
+  static boolean isSystemTablesOnlyOnMaster(Configuration conf) {
+    return conf.getBoolean(SYSTEM_TABLES_ON_MASTER, false);
+  }
 }

hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java

@@ -35,6 +35,7 @@
 import java.util.concurrent.ConcurrentNavigableMap;
 import java.util.concurrent.ConcurrentSkipListMap;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Predicate;
 
 import org.apache.commons.logging.Log;
@@ -109,7 +110,7 @@ public class ServerManager {
   private static final Log LOG = LogFactory.getLog(ServerManager.class);
 
   // Set if we are to shutdown the cluster.
-  private volatile boolean clusterShutdown = false;
+  private AtomicBoolean clusterShutdown = new AtomicBoolean(false);
 
   /**
    * The last flushed sequence id for a region.
@@ -423,7 +424,6 @@ private ServerName findServerWithSameHostnamePortWithLock(
   /**
    * Adds the onlineServers list. onlineServers should be locked.
    * @param serverName The remote servers name.
-   * @param sl
    */
   @VisibleForTesting
   void recordNewServerWithLock(final ServerName serverName, final ServerLoad sl) {
@@ -583,15 +583,15 @@ public synchronized void expireServer(final ServerName serverName) {
 
     // If cluster is going down, yes, servers are going to be expiring; don't
     // process as a dead server
-    if (this.clusterShutdown) {
+    if (this.clusterShutdown.get()) {
       LOG.info("Cluster shutdown set; " + serverName +
         " expired; onlineServers=" + this.onlineServers.size());
       if (this.onlineServers.isEmpty()) {
         master.stop("Cluster shutdown set; onlineServer=0");
       }
       return;
     }
-
+    LOG.info("Processing expiration of " + serverName + " on " + this.master.getServerName());
     master.getAssignmentManager().submitServerCrash(serverName, true);
 
     // Tell our listeners that a server was removed
@@ -790,12 +790,12 @@ public AdminService.BlockingInterface getRsAdmin(final ServerName sn)
   private int getMinToStart() {
     // One server should be enough to get us off the ground.
     int requiredMinToStart = 1;
-    if (BaseLoadBalancer.tablesOnMaster(master.getConfiguration())) {
-      if (!BaseLoadBalancer.userTablesOnMaster(master.getConfiguration())) {
-        // If Master is carrying regions but NOT user-space regions (the current default),
-        // since the Master shows as a 'server', we need at least one more server to check
-        // in before we can start up so up defaultMinToStart to 2.
-        requiredMinToStart = 2;
+    if (LoadBalancer.isTablesOnMaster(master.getConfiguration())) {
+      if (LoadBalancer.isSystemTablesOnlyOnMaster(master.getConfiguration())) {
+        // If Master is carrying regions but NOT user-space regions, it
+        // still shows as a 'server'. We need at least one more server to check
+        // in before we can start up so set defaultMinToStart to 2.
+        requiredMinToStart = requiredMinToStart + 1;
       }
     }
     int minToStart = this.master.getConfiguration().getInt(WAIT_ON_REGIONSERVERS_MINTOSTART, -1);
@@ -944,12 +944,14 @@ public synchronized boolean isServerDead(ServerName serverName) {
   }
 
   public void shutdownCluster() {
-    this.clusterShutdown = true;
-    this.master.stop("Cluster shutdown requested");
+    String statusStr = "Cluster shutdown requested of master=" + this.master.getServerName();
+    LOG.info(statusStr);
+    this.clusterShutdown.set(true);
+    this.master.stop(statusStr);
   }
 
   public boolean isClusterShutdown() {
-    return this.clusterShutdown;
+    return this.clusterShutdown.get();
   }
 
   /**
@@ -973,7 +975,7 @@ public void stop() {
   public List<ServerName> createDestinationServersList(final List<ServerName> serversToExclude){
     final List<ServerName> destServers = getOnlineServersList();
 
-    if (serversToExclude != null){
+    if (serversToExclude != null) {
       destServers.removeAll(serversToExclude);
     }