HBASE-27075 TestUpdateRSGroupConfiguration.testCustomOnlineConfigChan…

…geInRSGroup is flaky (#4636)

Signed-off-by: Xin Sun <[email protected]>
(cherry picked from commit 31fc97e)

Conflicts:
	hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java
	hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestUpdateRSGroupConfiguration.java

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java

@@ -100,7 +100,7 @@ public static void setUpTestBeforeClass() throws Exception {
       RSGroupAdminEndpoint.class.getName() + "," + CPMasterObserver.class.getName());
     TEST_UTIL.startMiniCluster(NUM_SLAVES_BASE - 1);
     TEST_UTIL.getConfiguration().setInt(ServerManager.WAIT_ON_REGIONSERVERS_MINTOSTART,
-      NUM_SLAVES_BASE - 1);
+      NUM_SLAVES_BASE);
     TEST_UTIL.getConfiguration().setBoolean(SnapshotManager.HBASE_SNAPSHOT_ENABLED, true);
     initialize();
   }

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestUpdateRSGroupConfiguration.java

@@ -18,11 +18,11 @@
 package org.apache.hadoop.hbase.rsgroup;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.assertThrows;
 
-import java.util.stream.Collectors;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
+import org.apache.hadoop.hbase.net.Address;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.util.JVMClusterUtil;
 import org.junit.After;
@@ -36,7 +36,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-@Category(MediumTests.class)
+import org.apache.hbase.thirdparty.com.google.common.collect.Iterables;
+
+@Category({ MediumTests.class })
 public class TestUpdateRSGroupConfiguration extends TestRSGroupsBase {
   protected static final Logger LOG = LoggerFactory.getLogger(TestUpdateRSGroupConfiguration.class);
 
@@ -68,57 +70,47 @@ public void afterMethod() throws Exception {
     tearDownAfterMethod();
   }
 
-  @Test
-  public void testOnlineConfigChangeInRSGroup() throws Exception {
-    addGroup(TEST_GROUP, 1);
-    rsGroupAdmin.updateConfiguration(TEST_GROUP);
-  }
-
   @Test
   public void testNonexistentRSGroup() throws Exception {
-    try {
-      rsGroupAdmin.updateConfiguration(TEST2_GROUP);
-      fail("Group does not exist: test2");
-    } catch (IllegalArgumentException iae) {
-      // expected
-    }
+    assertThrows(IllegalArgumentException.class,
+      () -> rsGroupAdmin.updateConfiguration(TEST2_GROUP));
   }
 
   // This test relies on a disallowed API change in RSGroupInfo and was also found to be
   // flaky. REVERTED from branch-2.5 and branch-2.
   @Test
   @Ignore
   public void testCustomOnlineConfigChangeInRSGroup() throws Exception {
+    RSGroupInfo testRSGroup = addGroup(TEST_GROUP, 1);
+    RSGroupInfo test2RSGroup = addGroup(TEST2_GROUP, 1);
     // Check the default configuration of the RegionServers
     TEST_UTIL.getMiniHBaseCluster().getRegionServerThreads().forEach(thread -> {
       Configuration conf = thread.getRegionServer().getConfiguration();
       assertEquals(0, conf.getInt("hbase.custom.config", 0));
     });
 
     replaceHBaseSiteXML();
-    RSGroupInfo testRSGroup = addGroup(TEST_GROUP, 1);
-    RSGroupInfo test2RSGroup = addGroup(TEST2_GROUP, 1);
-    rsGroupAdmin.updateConfiguration(TEST_GROUP);
-
-    // Check the configuration of the RegionServer in test rsgroup, should be update
-    Configuration regionServerConfiguration = TEST_UTIL.getMiniHBaseCluster()
-      .getLiveRegionServerThreads().stream().map(JVMClusterUtil.RegionServerThread::getRegionServer)
-      .filter(regionServer -> (regionServer.getServerName().getAddress()
-        .equals(testRSGroup.getServers().iterator().next())))
-      .collect(Collectors.toList()).get(0).getConfiguration();
-    int custom = regionServerConfiguration.getInt("hbase.custom.config", 0);
-    assertEquals(1000, custom);
-
-    // Check the configuration of the RegionServer in test2 rsgroup, should not be update
-    regionServerConfiguration = TEST_UTIL.getMiniHBaseCluster().getLiveRegionServerThreads()
-      .stream().map(JVMClusterUtil.RegionServerThread::getRegionServer)
-      .filter(regionServer -> (regionServer.getServerName().getAddress()
-        .equals(test2RSGroup.getServers().iterator().next())))
-      .collect(Collectors.toList()).get(0).getConfiguration();
-    custom = regionServerConfiguration.getInt("hbase.custom.config", 0);
-    assertEquals(0, custom);
-
-    restoreHBaseSiteXML();
+    try {
+      rsGroupAdmin.updateConfiguration(TEST_GROUP);
+
+      Address testServerAddr = Iterables.getOnlyElement(testRSGroup.getServers());
+      LOG.info("Check hbase.custom.config for " + testServerAddr);
+      Configuration testRsConf = TEST_UTIL.getMiniHBaseCluster().getLiveRegionServerThreads()
+        .stream().map(JVMClusterUtil.RegionServerThread::getRegionServer)
+        .filter(rs -> rs.getServerName().getAddress().equals(testServerAddr)).findFirst().get()
+        .getConfiguration();
+      assertEquals(1000, testRsConf.getInt("hbase.custom.config", 0));
+
+      Address test2ServerAddr = Iterables.getOnlyElement(test2RSGroup.getServers());
+      LOG.info("Check hbase.custom.config for " + test2ServerAddr);
+      // Check the configuration of the RegionServer in test2 rsgroup, should not be update
+      Configuration test2RsConf = TEST_UTIL.getMiniHBaseCluster().getLiveRegionServerThreads()
+        .stream().map(JVMClusterUtil.RegionServerThread::getRegionServer)
+        .filter(rs -> rs.getServerName().getAddress().equals(test2ServerAddr)).findFirst().get()
+        .getConfiguration();
+      assertEquals(0, test2RsConf.getInt("hbase.custom.config", 0));
+    } finally {
+      restoreHBaseSiteXML();
+    }
   }
-
 }

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

@@ -24,13 +24,17 @@
 import java.nio.file.StandardCopyOption;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.util.JVMClusterUtil.RegionServerThread;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Base class to test Configuration Update logic. It wraps up things needed to test configuration
  * change and provides utility methods for test cluster setup, updating/restoring configuration
  * file.
  */
 public abstract class AbstractTestUpdateConfiguration {
+  private static final Logger LOG = LoggerFactory.getLogger(AbstractTestUpdateConfiguration.class);
+
   private static final String SERVER_CONFIG = "hbase-site.xml";
   private static final String OVERRIDE_SERVER_CONFIG = "override-hbase-site.xml";
   private static final String BACKUP_SERVER_CONFIG = "backup-hbase-site.xml";
@@ -91,7 +95,9 @@ protected static void addResourceToRegionServerConfiguration(final HBaseTestingU
    * using {@link #restoreHBaseSiteXML()}.
    * @throws IOException if an I/O error occurs
    */
-  protected void replaceHBaseSiteXML() throws IOException {
+  protected final void replaceHBaseSiteXML() throws IOException {
+    LOG.info("Replace hbase config {} with {}", configFileUnderTestDataDir,
+      overrideConfigFileUnderTestDataDir);
     // make a backup of hbase-site.xml
     Files.copy(configFileUnderTestDataDir, backupConfigFileUnderTestDataDir,
       StandardCopyOption.REPLACE_EXISTING);
@@ -105,7 +111,9 @@ protected void replaceHBaseSiteXML() throws IOException {
    * {@link #replaceHBaseSiteXML()}.
    * @throws IOException if an I/O error occurs
    */
-  protected void restoreHBaseSiteXML() throws IOException {
+  protected final void restoreHBaseSiteXML() throws IOException {
+    LOG.info("Restore hbase config {} with {}", configFileUnderTestDataDir,
+      backupConfigFileUnderTestDataDir);
     // restore hbase-site.xml
     Files.copy(backupConfigFileUnderTestDataDir, configFileUnderTestDataDir,
       StandardCopyOption.REPLACE_EXISTING);