HBASE-15856 Don't cache unresolved addresses for connections

hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/AbstractRpcClient.java

@@ -30,6 +30,7 @@ import java.net.ConnectException;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.SocketTimeoutException;
+import java.net.UnknownHostException;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -266,7 +267,7 @@ public abstract class AbstractRpcClient implements RpcClient {
 
   @Override
   public BlockingRpcChannel createBlockingRpcChannel(final ServerName sn, final User ticket,
-      int defaultOperationTimeout) {
+      int defaultOperationTimeout) throws UnknownHostException {
     return new BlockingRpcChannelImplementation(this, sn, ticket, defaultOperationTimeout);
   }
 
@@ -332,8 +333,12 @@ public abstract class AbstractRpcClient implements RpcClient {
      * @param channelOperationTimeout - the default timeout when no timeout is given
      */
     protected BlockingRpcChannelImplementation(final AbstractRpcClient rpcClient,
-        final ServerName sn, final User ticket, int channelOperationTimeout) {
+        final ServerName sn, final User ticket, int channelOperationTimeout)
+        throws UnknownHostException {
       this.isa = new InetSocketAddress(sn.getHostname(), sn.getPort());
+      if (this.isa.isUnresolved()) {
+        throw new UnknownHostException(sn.getHostname());
+      }
       this.rpcClient = rpcClient;
       this.ticket = ticket;
       this.channelOperationTimeout = channelOperationTimeout;

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestClientTimeouts.java

@@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue;
 
 import java.net.SocketAddress;
 import java.net.SocketTimeoutException;
+import java.net.UnknownHostException;
 import java.util.Random;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -142,7 +143,7 @@ public class TestClientTimeouts {
     // Return my own instance, one that does random timeouts
     @Override
     public BlockingRpcChannel createBlockingRpcChannel(ServerName sn,
-        User ticket, int rpcTimeout) {
+        User ticket, int rpcTimeout) throws UnknownHostException {
       return new RandomTimeoutBlockingRpcChannel(this, sn, ticket, rpcTimeout);
     }
   }
@@ -157,7 +158,7 @@ public class TestClientTimeouts {
     private static AtomicInteger invokations = new AtomicInteger();
 
     RandomTimeoutBlockingRpcChannel(final RpcClientImpl rpcClient, final ServerName sn,
-        final User ticket, final int rpcTimeout) {
+        final User ticket, final int rpcTimeout) throws UnknownHostException {
       super(rpcClient, sn, ticket, rpcTimeout);
     }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestConnectionImplementation.java

@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hbase.client;
+
+import static org.junit.Assert.fail;
+
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.testclassification.ClientTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.net.UnknownHostException;
+
+/**
+ * Tests that we fail fast when hostname resolution is not working and do not cache
+ * unresolved InetSocketAddresses.
+ */
+@Category({MediumTests.class, ClientTests.class})
+public class TestConnectionImplementation {
+  private static HBaseTestingUtility testUtil;
+  private static ConnectionImplementation conn;
+
+  @BeforeClass
+  public static void setupBeforeClass() throws Exception {
+    testUtil = HBaseTestingUtility.createLocalHTU();
+    testUtil.startMiniCluster();
+    conn = (ConnectionImplementation) testUtil.getConnection();
+  }
+
+  @AfterClass
+  public static void teardownAfterClass() throws Exception {
+    conn.close();
+    testUtil.shutdownMiniCluster();
+  }
+
+  @Test(expected = UnknownHostException.class)
+  public void testGetAdminBadHostname() throws Exception {
+    // verify that we can get an instance with the cluster hostname
+    ServerName master = testUtil.getHBaseCluster().getMaster().getServerName();
+    try {
+      conn.getAdmin(master);
+    } catch (UnknownHostException uhe) {
+      fail("Obtaining admin to the cluster master should have succeeded");
+    }
+
+    // test that we fail to get a client to an unresolvable hostname, which
+    // means it won't be cached
+    ServerName badHost =
+        ServerName.valueOf("unknownhost.example.com:" + HConstants.DEFAULT_MASTER_PORT,
+        System.currentTimeMillis());
+    conn.getAdmin(badHost);
+    fail("Obtaining admin to unresolvable hostname should have failed");
+  }
+
+  @Test(expected = UnknownHostException.class)
+  public void testGetClientBadHostname() throws Exception {
+    // verify that we can get an instance with the cluster hostname
+    ServerName rs = testUtil.getHBaseCluster().getRegionServer(0).getServerName();
+    try {
+      conn.getClient(rs);
+    } catch (UnknownHostException uhe) {
+      fail("Obtaining client to the cluster regionserver should have succeeded");
+    }
+
+    // test that we fail to get a client to an unresolvable hostname, which
+    // means it won't be cached
+    ServerName badHost =
+        ServerName.valueOf("unknownhost.example.com:" + HConstants.DEFAULT_REGIONSERVER_PORT,
+        System.currentTimeMillis());
+    conn.getAdmin(badHost);
+    fail("Obtaining client to unresolvable hostname should have failed");
+  }
+}

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

@@ -185,6 +185,12 @@ public class TestMasterNoCluster {
     // of the 'remote' mocked up regionservers.
     CoordinatedStateManager cp = CoordinatedStateManagerFactory.getCoordinatedStateManager(
       TESTUTIL.getConfiguration());
+    // Insert a mock for the connection, use TESTUTIL.getConfiguration rather than
+    // the conf from the master; the conf will already have an HConnection
+    // associate so the below mocking of a connection will fail.
+    final ClusterConnection mockedConnection = HConnectionTestingUtility.getMockedConnectionAndDecorate(
+        TESTUTIL.getConfiguration(), rs0, rs0, rs0.getServerName(),
+        HRegionInfo.FIRST_META_REGIONINFO);
     HMaster master = new HMaster(conf, cp) {
       InetAddress getRemoteInetAddress(final int port, final long serverStartCode)
       throws UnknownHostException {
@@ -215,16 +221,12 @@ public class TestMasterNoCluster {
 
       @Override
       public ClusterConnection getConnection() {
-        // Insert a mock for the connection, use TESTUTIL.getConfiguration rather than
+        return mockedConnection;
-        // the conf from the master; the conf will already have an HConnection
+      }
-        // associate so the below mocking of a connection will fail.
+
-        try {
+      @Override
-          return HConnectionTestingUtility.getMockedConnectionAndDecorate(
+      public ClusterConnection getClusterConnection() {
-            TESTUTIL.getConfiguration(), rs0, rs0, rs0.getServerName(),
+        return mockedConnection;
-            HRegionInfo.FIRST_META_REGIONINFO);
-        } catch (IOException e) {
-          return null;
-        }
       }
     };
     master.start();