HBASE-24667 Rename configs that support atypical DNS set ups to put t…

…hem in hbase.unsafe Closes #2542 Signed-off-by: Viraj Jasani <vjasani@apache.org>
apache · Nov 9, 2020 · 5c7432f · 5c7432f
1 parent c36ee4f
commit 5c7432f
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 22 deletions.
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/DNS.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/DNS.java
@@ -35,13 +35,22 @@ public final class DNS {
   // the specification of server hostname is optional. The hostname should be resolvable from
   // both master and region server
   @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
-  public static final String RS_HOSTNAME_KEY = "hbase.regionserver.hostname";
+  public static final String UNSAFE_RS_HOSTNAME_KEY = "hbase.unsafe.regionserver.hostname";
   @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
   public static final String MASTER_HOSTNAME_KEY = "hbase.master.hostname";
 
   private static boolean HAS_NEW_DNS_GET_DEFAULT_HOST_API;
   private static Method GET_DEFAULT_HOST_METHOD;
 
+  /**
+   * @deprecated since 2.4.0 and will be removed in 4.0.0.
+   * Use {@link DNS#UNSAFE_RS_HOSTNAME_KEY} instead.
+   * @see <a href="https://issues.apache.org/jira/browse/HBASE-24667">HBASE-24667</a>
+   */
+  @Deprecated
+  @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
+  public static final String RS_HOSTNAME_KEY = "hbase.regionserver.hostname";
+
   static {
     try {
       GET_DEFAULT_HOST_METHOD = org.apache.hadoop.net.DNS.class
@@ -50,6 +59,7 @@ public final class DNS {
     } catch (Exception e) {
       HAS_NEW_DNS_GET_DEFAULT_HOST_API = false; // FindBugs: Causes REC_CATCH_EXCEPTION. Suppressed
     }
+    Configuration.addDeprecation(RS_HOSTNAME_KEY, UNSAFE_RS_HOSTNAME_KEY);
   }
 
   public enum ServerType {
@@ -106,7 +116,7 @@ public static String getHostname(@NonNull Configuration conf, @NonNull ServerTyp
         hostname = conf.get(MASTER_HOSTNAME_KEY);
         break;
       case REGIONSERVER:
-        hostname = conf.get(RS_HOSTNAME_KEY);
+        hostname = conf.get(UNSAFE_RS_HOSTNAME_KEY);
         break;
       default:
         hostname = null;

diff --git a/hbase-common/src/main/resources/hbase-default.xml b/hbase-common/src/main/resources/hbase-default.xml
@@ -1119,19 +1119,19 @@ possible configurations would overwhelm and obscure the important.
     http://docs.oracle.com/javase/1.5.0/docs/api/java/net/Socket.html#getTcpNoDelay()</description>
   </property>
   <property>
-    <name>hbase.regionserver.hostname</name>
+    <name>hbase.unsafe.regionserver.hostname</name>
     <value></value>
     <description>This config is for experts: don't set its value unless you really know what you are doing.
     When set to a non-empty value, this represents the (external facing) hostname for the underlying server.
     See https://issues.apache.org/jira/browse/HBASE-12954 for details.</description>
   </property>
   <property>
-    <name>hbase.regionserver.hostname.disable.master.reversedns</name>
+    <name>hbase.unsafe.regionserver.hostname.disable.master.reversedns</name>
     <value>false</value>
     <description>This config is for experts: don't set its value unless you really know what you are doing.
     When set to true, regionserver will use the current node hostname for the servername and HMaster will
     skip reverse DNS lookup and use the hostname sent by regionserver instead. Note that this config and
-    hbase.regionserver.hostname are mutually exclusive. See https://issues.apache.org/jira/browse/HBASE-18226
+    hbase.unsafe.regionserver.hostname are mutually exclusive. See https://issues.apache.org/jira/browse/HBASE-18226
     for more details.</description>
   </property>
   <!-- The following properties configure authentication information for

diff --git a/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestSecureRESTServer.java b/hbase-rest/src/test/java/org/apache/hadoop/hbase/rest/TestSecureRESTServer.java
@@ -168,7 +168,7 @@ public static void setupServer() throws Exception {
     HBaseKerberosUtils.setKeytabFileForTesting(serviceKeytab.getAbsolutePath());
     // Why doesn't `setKeytabFileForTesting` do this?
     conf.set("hbase.master.keytab.file", serviceKeytab.getAbsolutePath());
-    conf.set("hbase.regionserver.hostname", "localhost");
+    conf.set("hbase.unsafe.regionserver.hostname", "localhost");
     conf.set("hbase.master.hostname", "localhost");
     HBaseKerberosUtils.setSecuredConfiguration(conf,
         SERVICE_PRINCIPAL+ "@" + KDC.getRealm(), SPNEGO_SERVICE_PRINCIPAL+ "@" + KDC.getRealm());

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
@@ -21,7 +21,7 @@
 import static org.apache.hadoop.hbase.HConstants.DEFAULT_HBASE_SPLIT_WAL_MAX_SPLITTER;
 import static org.apache.hadoop.hbase.HConstants.HBASE_SPLIT_WAL_COORDINATED_BY_ZK;
 import static org.apache.hadoop.hbase.HConstants.HBASE_SPLIT_WAL_MAX_SPLITTER;
-import static org.apache.hadoop.hbase.util.DNS.RS_HOSTNAME_KEY;
+import static org.apache.hadoop.hbase.util.DNS.UNSAFE_RS_HOSTNAME_KEY;
 
 import java.io.IOException;
 import java.lang.management.MemoryType;
@@ -467,13 +467,32 @@ public class HRegionServer extends Thread implements
   protected String useThisHostnameInstead;
 
   /**
-   * HBASE-18226: This config and hbase.regionserver.hostname are mutually exclusive.
-   * Exception will be thrown if both are used.
+   * @deprecated since 2.4.0 and will be removed in 4.0.0.
+   * Use {@link HRegionServer#UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY} instead.
+   * @see <a href="https://issues.apache.org/jira/browse/HBASE-24667">HBASE-24667</a>
    */
+  @Deprecated
   @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
   final static String RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY =
     "hbase.regionserver.hostname.disable.master.reversedns";
 
+  /**
+   * HBASE-18226: This config and hbase.unasfe.regionserver.hostname are mutually exclusive.
+   * Exception will be thrown if both are used.
+   */
+  @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
+  final static String UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY =
+    "hbase.unsafe.regionserver.hostname.disable.master.reversedns";
+
+  /**
+   * HBASE-24667: This config hbase.regionserver.hostname.disable.master.reversedns will be replaced by
+   * hbase.unsafe.regionserver.hostname.disable.master.reversedns. Keep the old config keys here for backward
+   * compatibility.
+   */
+  static {
+    Configuration.addDeprecation(RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY);
+  }
+
   /**
    * This servers startcode.
    */
@@ -691,12 +710,12 @@ private void initNamedQueueRecorder(Configuration conf) {
 
   // HMaster should override this method to load the specific config for master
   protected String getUseThisHostnameInstead(Configuration conf) throws IOException {
-    String hostname = conf.get(RS_HOSTNAME_KEY);
-    if (conf.getBoolean(RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false)) {
+    String hostname = conf.get(UNSAFE_RS_HOSTNAME_KEY);
+    if (conf.getBoolean(UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false)) {
       if (!StringUtils.isBlank(hostname)) {
-        String msg = RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + " and " + RS_HOSTNAME_KEY +
-          " are mutually exclusive. Do not set " + RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY +
-          " to true while " + RS_HOSTNAME_KEY + " is used";
+        String msg = UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + " and " + UNSAFE_RS_HOSTNAME_KEY +
+          " are mutually exclusive. Do not set " + UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY +
+          " to true while " + UNSAFE_RS_HOSTNAME_KEY + " is used";
         throw new IOException(msg);
       } else {
         return rpcServices.isa.getHostName();

diff --git a/...e-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerHostname.java b/...e-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerHostname.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.regionserver;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
@@ -76,7 +77,7 @@ public void teardown() throws Exception {
   @Test
   public void testInvalidRegionServerHostnameAbortsServer() throws Exception {
     String invalidHostname = "hostAddr.invalid";
-    TEST_UTIL.getConfiguration().set(DNS.RS_HOSTNAME_KEY, invalidHostname);
+    TEST_UTIL.getConfiguration().set(DNS.UNSAFE_RS_HOSTNAME_KEY, invalidHostname);
     HRegionServer hrs = null;
     try {
       hrs = new HRegionServer(TEST_UTIL.getConfiguration());
@@ -105,7 +106,7 @@ public void testRegionServerHostname() throws Exception {
         LOG.info("Found " + hostName + " on " + ni + ", addr=" + addr);
 
         TEST_UTIL.getConfiguration().set(DNS.MASTER_HOSTNAME_KEY, hostName);
-        TEST_UTIL.getConfiguration().set(DNS.RS_HOSTNAME_KEY, hostName);
+        TEST_UTIL.getConfiguration().set(DNS.UNSAFE_RS_HOSTNAME_KEY, hostName);
         StartMiniClusterOption option = StartMiniClusterOption.builder()
             .numMasters(NUM_MASTERS).numRegionServers(NUM_RS).numDataNodes(NUM_RS).build();
         TEST_UTIL.startMiniCluster(option);
@@ -127,6 +128,29 @@ public void testRegionServerHostname() throws Exception {
     }
   }
 
+  @Test
+  public void testDeprecatedConfigs() throws Exception {
+    Configuration conf = TEST_UTIL.getConfiguration();
+    new HRegionServer(conf);
+    conf.setBoolean(HRegionServer.RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false);
+    assertFalse(conf.getBoolean(HRegionServer.UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, true));
+    conf.setBoolean(HRegionServer.RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, true);
+    assertTrue(conf.getBoolean(HRegionServer.UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false));
+    conf.setBoolean(HRegionServer.UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, true);
+    assertTrue(conf.getBoolean(HRegionServer.RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false));
+    conf.setBoolean(HRegionServer.UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false);
+    assertFalse(conf.getBoolean(HRegionServer.RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, true));
+
+    conf.setBoolean(DNS.RS_HOSTNAME_KEY, false);
+    assertFalse(conf.getBoolean(DNS.UNSAFE_RS_HOSTNAME_KEY, true));
+    conf.setBoolean(DNS.RS_HOSTNAME_KEY, true);
+    assertTrue(conf.getBoolean(DNS.UNSAFE_RS_HOSTNAME_KEY, false));
+    conf.setBoolean(DNS.UNSAFE_RS_HOSTNAME_KEY, true);
+    assertTrue(conf.getBoolean(DNS.RS_HOSTNAME_KEY, false));
+    conf.setBoolean(DNS.UNSAFE_RS_HOSTNAME_KEY, false);
+    assertFalse(conf.getBoolean(DNS.RS_HOSTNAME_KEY, true));
+  }
+
   @Test
   public void testConflictRegionServerHostnameConfigurationsAbortServer() throws Exception {
     Enumeration<NetworkInterface> netInterfaceList = NetworkInterface.getNetworkInterfaces();
@@ -143,10 +167,10 @@ public void testConflictRegionServerHostnameConfigurationsAbortServer() throws E
         LOG.info("Found " + hostName + " on " + ni);
 
         TEST_UTIL.getConfiguration().set(DNS.MASTER_HOSTNAME_KEY, hostName);
-        // "hbase.regionserver.hostname" and "hbase.regionserver.hostname.disable.master.reversedns"
+        // "hbase.unsafe.regionserver.hostname" and "hbase.unsafe.regionserver.hostname.disable.master.reversedns"
         // are mutually exclusive. Exception should be thrown if both are used.
-        TEST_UTIL.getConfiguration().set(DNS.RS_HOSTNAME_KEY, hostName);
-        TEST_UTIL.getConfiguration().setBoolean(HRegionServer.RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, true);
+        TEST_UTIL.getConfiguration().set(DNS.UNSAFE_RS_HOSTNAME_KEY, hostName);
+        TEST_UTIL.getConfiguration().setBoolean(HRegionServer.UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, true);
         try {
           StartMiniClusterOption option = StartMiniClusterOption.builder()
               .numMasters(NUM_MASTERS).numRegionServers(NUM_RS).numDataNodes(NUM_RS).build();
@@ -155,8 +179,8 @@ public void testConflictRegionServerHostnameConfigurationsAbortServer() throws E
           Throwable t1 = e.getCause();
           Throwable t2 = t1.getCause();
           assertTrue(t1.getMessage()+" - "+t2.getMessage(), t2.getMessage().contains(
-            HRegionServer.RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + " and " +
-                DNS.RS_HOSTNAME_KEY + " are mutually exclusive"));
+            HRegionServer.UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + " and " +
+                DNS.UNSAFE_RS_HOSTNAME_KEY + " are mutually exclusive"));
           return;
         } finally {
           TEST_UTIL.shutdownMiniCluster();
@@ -168,7 +192,7 @@ public void testConflictRegionServerHostnameConfigurationsAbortServer() throws E
 
   @Test
   public void testRegionServerHostnameReportedToMaster() throws Exception {
-    TEST_UTIL.getConfiguration().setBoolean(HRegionServer.RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY,
+    TEST_UTIL.getConfiguration().setBoolean(HRegionServer.UNSAFE_RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY,
     true);
     StartMiniClusterOption option = StartMiniClusterOption.builder()
         .numMasters(NUM_MASTERS).numRegionServers(NUM_RS).numDataNodes(NUM_RS).build();