HBASE-26172 Deprecated MasterRegistry and allow getBootstrapNodes to return master address instead of region server

Apache9 · Apache9 · commit 2ad504263d45 · 2021-08-05T23:19:46.000+08:00
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterRegistry.java
@@ -49,8 +49,9 @@
  * {@link #MASTER_REGISTRY_HEDGED_REQS_FANOUT_KEY} to a value greater than {@code 1} will enable
  * it(the default value is {@link AbstractRpcBasedConnectionRegistry#HEDGED_REQS_FANOUT_DEFAULT}).
  * <p/>
- * TODO: Handle changes to the configuration dynamically without having to restart the client.
+ * @deprecated Since 2.5.0, will be removed in 4.0.0. Use {@link RpcConnectionRegistry} instead.
  */
+@Deprecated
 @InterfaceAudience.Private
 public class MasterRegistry extends AbstractRpcBasedConnectionRegistry {
 
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -374,8 +374,6 @@
 import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.ClientMetaService;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetActiveMasterRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetActiveMasterResponse;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetBootstrapNodesRequest;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetBootstrapNodesResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetClusterIdRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetClusterIdResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.RegistryProtos.GetMastersRequest;
@@ -3046,13 +3044,18 @@ public GetMastersResponse getMasters(RpcController rpcController, GetMastersRequ
   }
 
   @Override
-  public GetBootstrapNodesResponse getBootstrapNodes(RpcController controller,
-    GetBootstrapNodesRequest request) throws ServiceException {
-    GetBootstrapNodesResponse.Builder builder = GetBootstrapNodesResponse.newBuilder();
-    for (ServerName sn : master.getServerManager().getOnlineServers().keySet()) {
-      builder.addServerName(ProtobufUtil.toServerName(sn));
+  protected List<ServerName> getRegionServers() {
+    return master.getServerManager().getOnlineServersList();
+  }
+
+  @Override
+  protected List<ServerName> getMasters() {
+    List<ServerName> list = new ArrayList<>();
+    master.getActiveMaster().ifPresent(list::add);
+    for (ServerName backupMaster : master.getBackupMasters()) {
+      list.add(backupMaster);
     }
-    return builder.build();
+    return list;
   }
 
   @Override
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -45,6 +45,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.LongAdder;
+import java.util.stream.Stream;
 import org.apache.commons.lang3.mutable.MutableObject;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
@@ -153,6 +154,7 @@
 import org.apache.hadoop.hbase.wal.WALKey;
 import org.apache.hadoop.hbase.wal.WALSplitUtil;
 import org.apache.hadoop.hbase.wal.WALSplitUtil.MutationReplay;
+import org.apache.hadoop.hbase.zookeeper.MasterAddressTracker;
 import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.zookeeper.KeeperException;
@@ -308,18 +310,35 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
    */
   private static final long DEFAULT_REGION_SERVER_RPC_MINIMUM_SCAN_TIME_LIMIT_DELTA = 10;
 
-  /*
+  /**
    * Whether to reject rows with size > threshold defined by
    * {@link RSRpcServices#BATCH_ROWS_THRESHOLD_NAME}
    */
   private static final String REJECT_BATCH_ROWS_OVER_THRESHOLD =
     "hbase.rpc.rows.size.threshold.reject";
 
-  /*
+  /**
    * Default value of config {@link RSRpcServices#REJECT_BATCH_ROWS_OVER_THRESHOLD}
    */
   private static final boolean DEFAULT_REJECT_BATCH_ROWS_OVER_THRESHOLD = false;
 
+  /**
+   * Determine the bootstrap nodes we want to return to the client connection registry.
+   * <ul>
+   * <li>{@link #MASTER}: return masters as bootstrap nodes.</li>
+   * <li>{@link #REGIONSERVER}: return region servers as bootstrap nodes.</li>
+   * <li>{@link #ALL}: return both masters and region servers as bootstrap nodes.</li>
+   * </ul>
+   */
+  public enum ConnectionRegistryBootstrapNodeType {
+    MASTER, REGIONSERVER, ALL
+  }
+
+  public static final String CLIENT_BOOTSTRAP_NODE_TYPE = "hbase.client.bootstrap.node_type";
+
+  public static final ConnectionRegistryBootstrapNodeType DEFAULT_CLIENT_BOOTSTRAP_NODE_TYPE =
+    ConnectionRegistryBootstrapNodeType.REGIONSERVER;
+
   // Request counter. (Includes requests that are not serviced by regions.)
   // Count only once for requests with multiple actions like multi/caching-scan/replayBatch
   final LongAdder requestCount = new LongAdder();
@@ -4102,21 +4121,17 @@ public GetActiveMasterResponse getActiveMaster(RpcController controller,
   @Override
   public GetMastersResponse getMasters(RpcController controller, GetMastersRequest request)
     throws ServiceException {
-    try {
-      GetMastersResponse.Builder builder = GetMastersResponse.newBuilder();
-      ServerName activeMaster = regionServer.getMasterAddressTracker().getMasterAddress();
-      if (activeMaster != null) {
-        builder.addMasterServers(GetMastersResponseEntry.newBuilder()
-          .setServerName(ProtobufUtil.toServerName(activeMaster)).setIsActive(true));
-      }
-      for (ServerName backupMaster : regionServer.getMasterAddressTracker().getBackupMasters()) {
-        builder.addMasterServers(GetMastersResponseEntry.newBuilder()
-          .setServerName(ProtobufUtil.toServerName(backupMaster)).setIsActive(false));
-      }
-      return builder.build();
-    } catch (IOException e) {
-      throw new ServiceException(e);
+    GetMastersResponse.Builder builder = GetMastersResponse.newBuilder();
+    ServerName activeMaster = regionServer.getMasterAddressTracker().getMasterAddress();
+    if (activeMaster != null) {
+      builder.addMasterServers(GetMastersResponseEntry.newBuilder()
+        .setServerName(ProtobufUtil.toServerName(activeMaster)).setIsActive(true));
     }
+    for (ServerName backupMaster : regionServer.getMasterAddressTracker().getBackupMasters()) {
+      builder.addMasterServers(GetMastersResponseEntry.newBuilder()
+        .setServerName(ProtobufUtil.toServerName(backupMaster)).setIsActive(false));
+    }
+    return builder.build();
   }
 
   @Override
@@ -4130,12 +4145,45 @@ public GetMetaRegionLocationsResponse getMetaRegionLocations(RpcController contr
     return builder.build();
   }
 
+  protected List<ServerName> getRegionServers() {
+    return regionServer.getRegionServerAddressTracker().getRegionServers();
+  }
+
+  protected List<ServerName> getMasters() {
+    List<ServerName> list = new ArrayList<>();
+    MasterAddressTracker tracker = regionServer.getMasterAddressTracker();
+    ServerName activeMaster = tracker.getMasterAddress();
+    if (activeMaster != null) {
+      list.add(activeMaster);
+    }
+    for (ServerName backupMaster : tracker.getBackupMasters()) {
+      list.add(backupMaster);
+    }
+    return list;
+  }
+
   @Override
-  public GetBootstrapNodesResponse getBootstrapNodes(RpcController controller,
+  public final GetBootstrapNodesResponse getBootstrapNodes(RpcController controller,
     GetBootstrapNodesRequest request) throws ServiceException {
+    ConnectionRegistryBootstrapNodeType type =
+      ConnectionRegistryBootstrapNodeType.valueOf(regionServer.getConfiguration()
+        .get(CLIENT_BOOTSTRAP_NODE_TYPE, DEFAULT_CLIENT_BOOTSTRAP_NODE_TYPE.name()));
+    Stream<ServerName> bootstrapNodes;
+    switch (type) {
+      case MASTER:
+        bootstrapNodes = getMasters().stream();
+        break;
+      case REGIONSERVER:
+        bootstrapNodes = getRegionServers().stream();
+        break;
+      case ALL:
+        bootstrapNodes = Stream.concat(getMasters().stream(), getRegionServers().stream());
+        break;
+      default:
+        throw new IllegalArgumentException("Unknown bootstrap node type:" + type);
+    }
     GetBootstrapNodesResponse.Builder builder = GetBootstrapNodesResponse.newBuilder();
-    regionServer.getRegionServerAddressTracker().getRegionServers().stream()
-      .map(ProtobufUtil::toServerName).forEach(builder::addServerName);
+    bootstrapNodes.map(ProtobufUtil::toServerName).forEach(builder::addServerName);
     return builder.build();
   }
 }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcConnectionRegistry.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRpcConnectionRegistry.java
@@ -20,17 +20,22 @@
 import static org.hamcrest.CoreMatchers.hasItems;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtil;
+import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.StartTestingClusterOption;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.master.HMaster;
+import org.apache.hadoop.hbase.regionserver.RSRpcServices;
 import org.apache.hadoop.hbase.testclassification.ClientTests;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.junit.After;
@@ -40,9 +45,14 @@
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 import org.apache.hbase.thirdparty.com.google.common.io.Closeables;
 
+@RunWith(Parameterized.class)
 @Category({ MediumTests.class, ClientTests.class })
 public class TestRpcConnectionRegistry {
 
@@ -54,13 +64,38 @@ public class TestRpcConnectionRegistry {
 
   private RpcConnectionRegistry registry;
 
+  @Parameter
+  public RSRpcServices.ConnectionRegistryBootstrapNodeType bootstrapNodeType;
+
+  @Parameters(name = "{index}: bootstrapNodeType={0}")
+  public static List<Object[]> params() {
+    List<Object[]> params = new ArrayList<>();
+    for (RSRpcServices.ConnectionRegistryBootstrapNodeType type : RSRpcServices.ConnectionRegistryBootstrapNodeType
+      .values()) {
+      params.add(new Object[] { type });
+    }
+    return params;
+  }
+
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
     // allow refresh immediately so we will switch to use region servers soon.
     UTIL.getConfiguration().setLong(RpcConnectionRegistry.PERIODIC_REFRESH_INTERVAL_SECS, 1);
     UTIL.getConfiguration().setLong(RpcConnectionRegistry.MIN_SECS_BETWEEN_REFRESHES, 0);
-    UTIL.startMiniCluster(3);
+    UTIL.startMiniCluster(StartTestingClusterOption.builder().numMasters(1)
+      .numAlwaysStandByMasters(1).numRegionServers(3).build());
     HBaseTestingUtil.setReplicas(UTIL.getAdmin(), TableName.META_TABLE_NAME, 3);
+    try (ConnectionRegistry registry =
+      ConnectionRegistryFactory.getRegistry(UTIL.getConfiguration())) {
+      // Add wait on all replicas being assigned before proceeding w/ test. Failed on occasion
+      // because not all replicas had made it up before test started.
+      RegionReplicaTestHelper.waitUntilAllMetaReplicasAreReady(UTIL, registry);
+    }
+    // set only the active master as bootstrap node, so it is easy to make sure we have refreshed
+    // the bootstrap nodes by check the number of bootstrap nodes.
+    ServerName activeMasterAddr = UTIL.getMiniHBaseCluster().getMaster().getServerName();
+    UTIL.getConfiguration().set(HConstants.MASTER_ADDRS_KEY,
+      activeMasterAddr.getHostname() + ":" + activeMasterAddr.getPort());
   }
 
   @AfterClass
@@ -70,6 +105,12 @@ public static void tearDownAfterClass() throws Exception {
 
   @Before
   public void setUp() throws IOException {
+    UTIL.getMiniHBaseCluster().getMasterThreads().stream()
+      .map(t -> t.getMaster().getConfiguration()).forEach(
+        conf -> conf.set(RSRpcServices.CLIENT_BOOTSTRAP_NODE_TYPE, bootstrapNodeType.name()));
+    UTIL.getMiniHBaseCluster().getRegionServerThreads().stream()
+      .map(t -> t.getRegionServer().getConfiguration()).forEach(
+        conf -> conf.set(RSRpcServices.CLIENT_BOOTSTRAP_NODE_TYPE, bootstrapNodeType.name()));
     registry = new RpcConnectionRegistry(UTIL.getConfiguration());
   }
 
@@ -78,17 +119,39 @@ public void tearDown() throws IOException {
     Closeables.close(registry, true);
   }
 
+  private void assertHasMasters() {
+    assertThat(registry.getParsedServers(), hasItems(UTIL.getMiniHBaseCluster().getMasterThreads()
+      .stream().map(t -> t.getMaster().getServerName()).toArray(ServerName[]::new)));
+  }
+
+  private void assertHasRegionServers() {
+    assertThat(registry.getParsedServers(),
+      hasItems(UTIL.getMiniHBaseCluster().getRegionServerThreads().stream()
+        .map(t -> t.getRegionServer().getServerName()).toArray(ServerName[]::new)));
+  }
+
   @Test
   public void testRegistryRPCs() throws Exception {
     HMaster activeMaster = UTIL.getHBaseCluster().getMaster();
-    // wait until we switch to use region servers
-    UTIL.waitFor(10000, () -> registry.getParsedServers().size() == 3);
-    assertThat(registry.getParsedServers(),
-      hasItems(activeMaster.getServerManager().getOnlineServersList().toArray(new ServerName[0])));
+    // wait until we switch to use refresh bootstrap nodes
+    switch (bootstrapNodeType) {
+      case MASTER:
+        UTIL.waitFor(10000, () -> registry.getParsedServers().size() == 2);
+        assertHasMasters();
+        break;
+      case REGIONSERVER:
+        UTIL.waitFor(10000, () -> registry.getParsedServers().size() == 3);
+        assertHasRegionServers();
+        break;
+      case ALL:
+        UTIL.waitFor(10000, () -> registry.getParsedServers().size() == 5);
+        assertHasMasters();
+        assertHasRegionServers();
+        break;
+      default:
+        fail("Unknown bootstrap node type: " + bootstrapNodeType);
+    }
 
-    // Add wait on all replicas being assigned before proceeding w/ test. Failed on occasion
-    // because not all replicas had made it up before test started.
-    RegionReplicaTestHelper.waitUntilAllMetaReplicasAreReady(UTIL, registry);
 
     assertEquals(registry.getClusterId().get(), activeMaster.getClusterId());
     assertEquals(registry.getActiveMaster().get(), activeMaster.getServerName());
diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MasterAddressTracker.java
@@ -307,7 +307,7 @@ public static boolean deleteIfEquals(ZKWatcher zkw, final String content) {
     return false;
   }
 
-  public List<ServerName> getBackupMasters() throws InterruptedIOException {
+  public List<ServerName> getBackupMasters() {
     return backupMasters;
   }
 

Original file line number	Diff line number	Diff line change
`@@ -307,7 +307,7 @@ public static boolean deleteIfEquals(ZKWatcher zkw, final String content) {`
`307`	`307`	`return false;`
`308`	`308`	`}`
`309`	`309`
`310`		`- public List<ServerName> getBackupMasters() throws InterruptedIOException {`
	`310`	`+ public List<ServerName> getBackupMasters() {`
`311`	`311`	`return backupMasters;`
`312`	`312`	`}`
`313`	`313`