HBASE-23275: Track active master's address in ActiveMasterManager (#812)

Currently we just track whether an active master exists.
It helps to also track the address of the active master in
all the masters to help serve the client RPC requests to
know which master is active.

Signed-off-by: Nick Dimiduk <ndimiduk@apache.org>
Signed-off-by: Andrew Purtell <apurtell@apache.org>
(cherry picked from commit efebb843af)
(cherry picked from commit 742949165f)

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

@@ -1,4 +1,4 @@
-/**
+/*
  *
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
@@ -17,7 +17,6 @@
  * limitations under the License.
  */
 package org.apache.hadoop.hbase.master;
-
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -57,12 +56,18 @@ public class ActiveMasterManager extends ZooKeeperListener {
   final AtomicBoolean clusterHasActiveMaster = new AtomicBoolean(false);
   final AtomicBoolean clusterShutDown = new AtomicBoolean(false);
 
+  // This server's information.
   private final ServerName sn;
   private int infoPort;
   private final Server master;
 
+  // Active master's server name. Invalidated anytime active master changes (based on ZK
+  // notifications) and lazily fetched on-demand.
+  // ServerName is immutable, so we don't need heavy synchronization around it.
+  private volatile ServerName activeMasterServerName;
+
   /**
-   * @param watcher
+   * @param watcher ZK watcher
    * @param sn ServerName
    * @param master In an instance of a Master.
    */
@@ -106,6 +111,33 @@ public class ActiveMasterManager extends ZooKeeperListener {
     }
   }
 
+  /**
+   * Fetches the active master's ServerName from zookeeper.
+   */
+  private void fetchAndSetActiveMasterServerName() {
+    LOG.debug("Attempting to fetch active master sn from zk");
+    try {
+      activeMasterServerName = MasterAddressTracker.getMasterAddress(watcher);
+    } catch (IOException | KeeperException e) {
+      // Log and ignore for now and re-fetch later if needed.
+      LOG.error("Error fetching active master information", e);
+    }
+  }
+
+  /**
+   * @return the currently active master as seen by us or null if one does not exist.
+   */
+  public ServerName getActiveMasterServerName() {
+    if (!clusterHasActiveMaster.get()) {
+      return null;
+    }
+    if (activeMasterServerName == null) {
+      fetchAndSetActiveMasterServerName();
+    }
+    // It could still be null, but return whatever we have.
+    return activeMasterServerName;
+  }
+
   /**
    * Handle a change in the master node.  Doesn't matter whether this was called
    * from a nodeCreated or nodeDeleted event because there are no guarantees
@@ -134,6 +166,9 @@ public class ActiveMasterManager extends ZooKeeperListener {
           // Notify any thread waiting to become the active master
           clusterHasActiveMaster.notifyAll();
         }
+        // Reset the active master sn. Will be re-fetched later if needed.
+        // We don't want to make a synchronous RPC under a monitor.
+        activeMasterServerName = null;
       }
     } catch (KeeperException ke) {
       master.abort("Received an unexpected KeeperException, aborting", ke);
@@ -151,8 +186,8 @@ public class ActiveMasterManager extends ZooKeeperListener {
    * @param checkInterval the interval to check if the master is stopped
    * @param startupStatus the monitor status to track the progress
    * @return True if no issue becoming active master else false if another
-   * master was running or if some other problem (zookeeper, stop flag has been
+   *   master was running or if some other problem (zookeeper, stop flag has been
-   * set on this Master)
+   *   set on this Master)
    */
   boolean blockUntilBecomingActiveMaster(
       int checkInterval, MonitoredTask startupStatus) {
@@ -179,9 +214,13 @@ public class ActiveMasterManager extends ZooKeeperListener {
           startupStatus.setStatus("Successfully registered as active master.");
           this.clusterHasActiveMaster.set(true);
           LOG.info("Registered Active Master=" + this.sn);
+          activeMasterServerName = sn;
           return true;
         }
 
+        // Invalidate the active master name so that subsequent requests do not get any stale
+        // master information. Will be re-fetched if needed.
+        activeMasterServerName = null;
         // There is another active master running elsewhere or this is a restart
         // and the master ephemeral node has not expired yet.
         this.clusterHasActiveMaster.set(true);
@@ -208,7 +247,8 @@ public class ActiveMasterManager extends ZooKeeperListener {
             ZKUtil.deleteNode(this.watcher, this.watcher.getMasterAddressZNode());
 
             // We may have failed to delete the znode at the previous step, but
-            //  we delete the file anyway: a second attempt to delete the znode is likely to fail again.
+            //  we delete the file anyway: a second attempt to delete the znode is likely to fail
+            //  again.
             ZNodeClearer.deleteMyEphemeralNodeOnDisk();
           } else {
             msg = "Another master is the active master, " + currentMaster +

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

@@ -3434,6 +3434,10 @@ public class HMaster extends HRegionServer implements MasterServices, Server {
     return replicationLoadSourceMap;
   }
 
+  public ServerName getActiveMaster() {
+    return activeMasterManager.getActiveMasterServerName();
+  }
+
   public String getClusterId() {
     if (activeMaster) {
       return super.getClusterId();

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestActiveMasterManager.java

@@ -18,8 +18,10 @@
  */
 package org.apache.hadoop.hbase.master;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
@@ -84,6 +86,7 @@ public class TestActiveMasterManager {
     ActiveMasterManager activeMasterManager =
       dummyMaster.getActiveMasterManager();
     assertFalse(activeMasterManager.clusterHasActiveMaster.get());
+    assertNull(activeMasterManager.getActiveMasterServerName());
 
     // First test becoming the active master uninterrupted
     MonitoredTask status = Mockito.mock(MonitoredTask.class);
@@ -92,6 +95,7 @@ public class TestActiveMasterManager {
     activeMasterManager.blockUntilBecomingActiveMaster(100, status);
     assertTrue(activeMasterManager.clusterHasActiveMaster.get());
     assertMaster(zk, master);
+    assertMaster(zk, activeMasterManager.getActiveMasterServerName());
 
     // Now pretend master restart
     DummyMaster secondDummyMaster = new DummyMaster(zk,master);
@@ -101,6 +105,8 @@ public class TestActiveMasterManager {
     activeMasterManager.blockUntilBecomingActiveMaster(100, status);
     assertTrue(activeMasterManager.clusterHasActiveMaster.get());
     assertMaster(zk, master);
+    assertMaster(zk, activeMasterManager.getActiveMasterServerName());
+    assertMaster(zk, secondActiveMasterManager.getActiveMasterServerName());
   }
 
   /**
@@ -128,6 +134,7 @@ public class TestActiveMasterManager {
     ActiveMasterManager activeMasterManager =
       ms1.getActiveMasterManager();
     assertFalse(activeMasterManager.clusterHasActiveMaster.get());
+    assertNull(activeMasterManager.getActiveMasterServerName());
 
     // First test becoming the active master uninterrupted
     ClusterStatusTracker clusterStatusTracker =
@@ -137,6 +144,7 @@ public class TestActiveMasterManager {
         Mockito.mock(MonitoredTask.class));
     assertTrue(activeMasterManager.clusterHasActiveMaster.get());
     assertMaster(zk, firstMasterAddress);
+    assertMaster(zk, activeMasterManager.getActiveMasterServerName());
 
     // New manager will now try to become the active master in another thread
     WaitToBeMasterThread t = new WaitToBeMasterThread(zk, secondMasterAddress);
@@ -154,6 +162,8 @@ public class TestActiveMasterManager {
     assertTrue(t.manager.clusterHasActiveMaster.get());
     // But secondary one should not be the active master
     assertFalse(t.isActiveMaster);
+    // Verify the active master ServerName is populated in standby master.
+    assertEquals(firstMasterAddress, t.manager.getActiveMasterServerName());
 
     // Close the first server and delete it's master node
     ms1.stop("stopping first server");
@@ -181,6 +191,7 @@ public class TestActiveMasterManager {
 
     assertTrue(t.manager.clusterHasActiveMaster.get());
     assertTrue(t.isActiveMaster);
+    assertEquals(secondMasterAddress, t.manager.getActiveMasterServerName());
 
     LOG.info("Deleting master node");