From db98fab114974ec7362bd8eca4a026a2631751f9 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Thu, 21 Jul 2022 14:26:52 +0800 Subject: [PATCH] HBASE-27075 TestUpdateRSGroupConfiguration.testCustomOnlineConfigChangeInRSGroup is flaky (#4636) Signed-off-by: Xin Sun (cherry picked from commit 31fc97edeede08cfc9c8dd9fe22c3775ede2eebb) 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/TestRSGroupsBase.java | 2 +- .../TestUpdateRSGroupConfiguration.java | 70 ++++++++----------- .../AbstractTestUpdateConfiguration.java | 12 +++- 3 files changed, 42 insertions(+), 42 deletions(-) diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java index 7b14a72f04f5..445f17a2b48d 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsBase.java +++ b/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(); } diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestUpdateRSGroupConfiguration.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestUpdateRSGroupConfiguration.java index ebb23665fcb4..27f8cbb7eced 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestUpdateRSGroupConfiguration.java +++ b/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,20 +70,10 @@ 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 @@ -89,6 +81,8 @@ public void testNonexistentRSGroup() throws Exception { @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(); @@ -96,29 +90,27 @@ public void testCustomOnlineConfigChangeInRSGroup() throws Exception { }); 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(); + } } - } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestUpdateConfiguration.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestUpdateConfiguration.java index 2af05cce48eb..ea3ec196a63e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestUpdateConfiguration.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestUpdateConfiguration.java @@ -24,6 +24,8 @@ 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 @@ -31,6 +33,8 @@ * 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);