HBASE-28333 Refactor TestClientTimeouts to make it more clear that wh…

…at we want to test (#5655)

Signed-off-by: Xin Sun <ddupgs@gmail.com>

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

@@ -17,9 +17,10 @@
  */
 package org.apache.hadoop.hbase.client;
 
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.io.IOException;
 import java.net.SocketAddress;
 import java.net.SocketTimeoutException;
 import java.util.Map;
@@ -31,7 +32,6 @@
 import org.apache.hadoop.hbase.HBaseTestingUtil;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.ServerName;
-import org.apache.hadoop.hbase.exceptions.MasterRegistryFetchException;
 import org.apache.hadoop.hbase.ipc.AbstractRpcClient;
 import org.apache.hadoop.hbase.ipc.BlockingRpcClient;
 import org.apache.hadoop.hbase.ipc.HBaseRpcController;
@@ -67,61 +67,46 @@ public class TestClientTimeouts {
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
     TEST_UTIL.startMiniCluster(SLAVES);
-    // Set the custom RPC client with random timeouts as the client
-    TEST_UTIL.getConfiguration().set(RpcClientFactory.CUSTOM_RPC_CLIENT_IMPL_CONF_KEY,
-      RandomTimeoutRpcClient.class.getName());
   }
 
   @AfterClass
   public static void tearDownAfterClass() throws Exception {
     TEST_UTIL.shutdownMiniCluster();
   }
 
+  private Connection createConnection() {
+    // Ensure the HBaseAdmin uses a new connection by changing Configuration.
+    Configuration conf = HBaseConfiguration.create(TEST_UTIL.getConfiguration());
+    // Set the custom RPC client with random timeouts as the client
+    conf.set(RpcClientFactory.CUSTOM_RPC_CLIENT_IMPL_CONF_KEY,
+      RandomTimeoutRpcClient.class.getName());
+    conf.set(HConstants.HBASE_CLIENT_INSTANCE_ID, String.valueOf(-1));
+    for (;;) {
+      try {
+        return ConnectionFactory.createConnection(conf);
+      } catch (IOException e) {
+        // since we randomly throw SocketTimeoutException, it is possible that we fail when creating
+        // the Connection, but this is not what we want to test here, so just ignore it and try
+        // again
+      }
+    }
+  }
+
   /**
    * Test that a client that fails an RPC to the master retries properly and doesn't throw any
    * unexpected exceptions.
    */
   @Test
   public void testAdminTimeout() throws Exception {
-    boolean lastFailed = false;
-    int initialInvocations = invokations.get();
-    RandomTimeoutRpcClient rpcClient = (RandomTimeoutRpcClient) RpcClientFactory
-      .createClient(TEST_UTIL.getConfiguration(), TEST_UTIL.getClusterKey());
-
-    try {
-      for (int i = 0; i < 5 || (lastFailed && i < 100); ++i) {
-        lastFailed = false;
-        // Ensure the HBaseAdmin uses a new connection by changing Configuration.
-        Configuration conf = HBaseConfiguration.create(TEST_UTIL.getConfiguration());
-        conf.set(HConstants.HBASE_CLIENT_INSTANCE_ID, String.valueOf(-1));
-        Admin admin = null;
-        Connection connection = null;
-        try {
-          connection = ConnectionFactory.createConnection(conf);
-          admin = connection.getAdmin();
-          admin.balancerSwitch(false, false);
-        } catch (MasterRegistryFetchException ex) {
-          // Since we are randomly throwing SocketTimeoutExceptions, it is possible to get
-          // a MasterRegistryFetchException. It's a bug if we get other exceptions.
-          lastFailed = true;
-        } finally {
-          if (admin != null) {
-            admin.close();
-            if (admin.getConnection().isClosed()) {
-              rpcClient = (RandomTimeoutRpcClient) RpcClientFactory
-                .createClient(TEST_UTIL.getConfiguration(), TEST_UTIL.getClusterKey());
-            }
-          }
-          if (connection != null) {
-            connection.close();
-          }
-        }
+    try (Connection conn = createConnection(); Admin admin = conn.getAdmin()) {
+      int initialInvocations = invokations.get();
+      boolean balanceEnabled = admin.isBalancerEnabled();
+      for (int i = 0; i < 5; i++) {
+        assertEquals(balanceEnabled, admin.balancerSwitch(!balanceEnabled, false));
+        balanceEnabled = !balanceEnabled;
       }
       // Ensure the RandomTimeoutRpcEngine is actually being used.
-      assertFalse(lastFailed);
       assertTrue(invokations.get() > initialInvocations);
-    } finally {
-      rpcClient.close();
     }
   }