From 6f059ecff0b20f4ade13d673a7c415040aca84e7 Mon Sep 17 00:00:00 2001
From: Michael Stack <saintstack@users.noreply.github.com>
Date: Wed, 6 May 2020 09:27:52 -0700
Subject: [PATCH] HBASE-24331 [Flakey Test] TestJMXListener rmi port clash
 (#1657)

Add check that we can make jmx connector in setup. If we can't retry.
---
 .../org/apache/hadoop/hbase/JMXListener.java  |  4 +-
 .../apache/hadoop/hbase/TestJMXListener.java  | 50 ++++++++++++++-----
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/JMXListener.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/JMXListener.java
index 5008354936e..f8fb4bd9a42 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/JMXListener.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/JMXListener.java
@@ -147,9 +147,9 @@ public class JMXListener implements MasterCoprocessor, RegionServerCoprocessor {
         JMX_CS = JMXConnectorServerFactory.newJMXConnectorServer(serviceUrl, jmxEnv, mbs);
         JMX_CS.start();
       }
-      LOG.info("ConnectorServer started!");
+      LOG.info("JMXConnectorServer started!");
     } catch (IOException e) {
-      LOG.error("fail to start connector server!", e);
+      LOG.error("Failed start of JMXConnectorServer!", e);
       // deregister the RMI registry
       if (rmiRegistry != null) {
         UnicastRemoteObject.unexportObject(rmiRegistry, true);
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXListener.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXListener.java
index 8332180b829..f43d390461e 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXListener.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXListener.java
@@ -1,4 +1,4 @@
-/**
+/*
  * 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
@@ -16,15 +16,14 @@
  * limitations under the License.
  */
 package org.apache.hadoop.hbase;
-
 import static org.junit.Assert.fail;
-
 import java.io.IOException;
 import javax.management.MBeanServerConnection;
 import javax.management.remote.JMXConnector;
 import javax.management.remote.JMXConnectorFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
+import org.apache.hadoop.hbase.net.BoundSocketMaker;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.testclassification.MiscTests;
 import org.apache.hadoop.hbase.util.JVMClusterUtil;
@@ -48,17 +47,44 @@ public class TestJMXListener {
 
   private static final Logger LOG = LoggerFactory.getLogger(TestJMXListener.class);
   private static HBaseTestingUtility UTIL = new HBaseTestingUtility();
-  private static int connectorPort = UTIL.randomFreePort();
+  private static int CONNECTOR_PORT;
 
   @BeforeClass
   public static void setupBeforeClass() throws Exception {
+    // Default RMI timeouts are too long. Make them short for test.
+    System.setProperty("sun.rmi.transport.connectionTimeout", Integer.toString(5000));
+    System.setProperty("sun.rmi.transport.tcp.handshakeTimeout", Integer.toString(5000));
+    System.setProperty("sun.rmi.transport.tcp.responseTimeout", Integer.toString(5000));
+    System.setProperty("sun.rmi.transport.tcp.readTimeout", Integer.toString(5000));
     Configuration conf = UTIL.getConfiguration();
-
-    conf.set(CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY,
-      JMXListener.class.getName());
-    conf.setInt("regionserver.rmi.registry.port", connectorPort);
-
-    UTIL.startMiniCluster();
+    // To test what happens when the jmx listener can't put up its port, uncomment the below.
+    BoundSocketMaker bsm = null; // new BoundSocketMaker(HBaseTestingUtility::randomFreePort);
+    conf.set(CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY, JMXListener.class.getName());
+    CONNECTOR_PORT = bsm == null? HBaseTestingUtility.randomFreePort(): bsm.getPort();
+    // Make sure the JMX listener is up before we proceed. If it is not up, retry. It may not
+    // come up if there is a port clash/bind exception except its called something else in rmi.
+    for (int i = 0; i < 10; i++) {
+      conf.setInt("regionserver.rmi.registry.port", CONNECTOR_PORT);
+      UTIL.startMiniCluster();
+      // Make sure we can get make a JMX connection before proceeding. It may have failed setup
+      // because of port clash/bind-exception. Deal with it here.
+      JMXConnector connector = null;
+      try {
+        connector = JMXConnectorFactory.
+          connect(JMXListener.buildJMXServiceURL(CONNECTOR_PORT, CONNECTOR_PORT));
+        break;
+      } catch (IOException ioe) {
+        UTIL.shutdownMiniCluster();
+        CONNECTOR_PORT = HBaseTestingUtility.randomFreePort();
+      } finally {
+        if (connector != null) {
+          connector.close();
+        }
+      }
+    }
+    if (bsm != null) {
+      bsm.close();
+    }
   }
 
   @AfterClass
@@ -69,7 +95,7 @@ public class TestJMXListener {
   @Test
   public void testStart() throws Exception {
     JMXConnector connector = JMXConnectorFactory.connect(
-      JMXListener.buildJMXServiceURL(connectorPort,connectorPort));
+      JMXListener.buildJMXServiceURL(CONNECTOR_PORT, CONNECTOR_PORT));
 
     MBeanServerConnection mb = connector.getMBeanServerConnection();
     String domain = mb.getDefaultDomain();
@@ -91,7 +117,7 @@ public class TestJMXListener {
     cluster.waitUntilShutDown();
 
     JMXConnector connector = JMXConnectorFactory.newJMXConnector(
-      JMXListener.buildJMXServiceURL(connectorPort,connectorPort), null);
+      JMXListener.buildJMXServiceURL(CONNECTOR_PORT, CONNECTOR_PORT), null);
     expectedEx.expect(IOException.class);
     connector.connect();