From f87c91ea90b7335b1c57146ce9f3aca2bcbcd621 Mon Sep 17 00:00:00 2001 From: Huaxiang Sun Date: Sun, 22 Mar 2020 17:43:04 -0700 Subject: [PATCH] HBASE-23957 [flakey test] client.TestMultiParallel fails to read hbase-site.xml (#1310) Signed-off-by: Nick Dimiduk ndimiduk@apache.org Signed-off-by: stack --- .../AbstractTestUpdateConfiguration.java | 116 ++++++++++++++++++ .../hbase/client/TestAsyncAdminBase.java | 2 +- .../client/TestAsyncClusterAdminApi.java | 24 +--- .../hbase/client/TestUpdateConfiguration.java | 45 +++---- ...base-site2.xml => override-hbase-site.xml} | 0 5 files changed, 137 insertions(+), 50 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestUpdateConfiguration.java rename hbase-server/src/test/resources/{hbase-site2.xml => override-hbase-site.xml} (100%) 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 new file mode 100644 index 000000000000..ec6204dbe141 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestUpdateConfiguration.java @@ -0,0 +1,116 @@ +/** + * 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 java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.util.JVMClusterUtil.RegionServerThread; + +/** + * 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 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"; + + private static Path configFileUnderTestDataDir; + private static Path overrideConfigFileUnderTestDataDir; + private static Path backupConfigFileUnderTestDataDir; + + protected static void setUpConfigurationFiles(final HBaseTestingUtility testUtil) + throws Exception { + // Before this change, the test will update hbase-site.xml under target/test-classes and + // trigger a config reload. Since target/test-classes/hbase-site.xml is being used by + // other testing cases at the same time, this update will break other testing cases so it will + // be flakey in nature. + // To avoid this, the change is to make target/test-classes/hbase-site.xml immutable. A new + // hbase-site.xml will be created under its test data directory, i.e, + // hbase-server/target/test-data/UUID, this new file will be added as a resource for the + // config, new update will be applied to this new file and only visible to this specific test + // case. The target/test-classes/hbase-site.xml will not be changed during the test. + + String absoluteDataPath = testUtil.getDataTestDir().toString(); + + // Create test-data directories. + Files.createDirectories(Paths.get(absoluteDataPath)); + + // Copy hbase-site.xml from target/test-class to target/test-data/UUID directory. + Path configFile = Paths.get("target", "test-classes", SERVER_CONFIG); + configFileUnderTestDataDir = Paths.get(absoluteDataPath, SERVER_CONFIG); + Files.copy(configFile, configFileUnderTestDataDir); + + // Copy override config file overrider-hbase-site.xml from target/test-class to + // target/test-data/UUID directory. + Path overrideConfigFile = Paths.get("target", "test-classes", + OVERRIDE_SERVER_CONFIG); + overrideConfigFileUnderTestDataDir = Paths.get(absoluteDataPath, OVERRIDE_SERVER_CONFIG); + Files.copy(overrideConfigFile, overrideConfigFileUnderTestDataDir); + + backupConfigFileUnderTestDataDir = Paths.get(absoluteDataPath, BACKUP_SERVER_CONFIG); + + // Add the new custom config file to Configuration + testUtil.getConfiguration().addResource(testUtil.getDataTestDir(SERVER_CONFIG)); + } + + protected static void addResourceToRegionServerConfiguration(final HBaseTestingUtility testUtil) { + // When RegionServer is created in MiniHBaseCluster, it uses HBaseConfiguration.create(conf) of + // the master Configuration. The create() just copies config params over, it does not do + // a clone for a historic reason. Properties such as resources are lost during this process. + // Exposing a new method in HBaseConfiguration causes confusion. Instead, the new hbase-site.xml + // under test-data directory is added to RegionServer's configuration as a workaround. + for (RegionServerThread rsThread : testUtil.getMiniHBaseCluster().getRegionServerThreads()) { + rsThread.getRegionServer().getConfiguration().addResource( + testUtil.getDataTestDir(SERVER_CONFIG)); + } + } + + /** + * Replace the hbase-site.xml file under this test's data directory with the content of the + * override-hbase-site.xml file. Stashes the current existing file so that it can be restored + * using {@link #restoreHBaseSiteXML()}. + * + * @throws IOException if an I/O error occurs + */ + protected void replaceHBaseSiteXML() throws IOException { + // make a backup of hbase-site.xml + Files.copy(configFileUnderTestDataDir, + backupConfigFileUnderTestDataDir, StandardCopyOption.REPLACE_EXISTING); + // update hbase-site.xml by overwriting it + Files.copy(overrideConfigFileUnderTestDataDir, + configFileUnderTestDataDir, StandardCopyOption.REPLACE_EXISTING); + } + + /** + * Restores the hbase-site.xml file that was stashed by a previous call to + * {@link #replaceHBaseSiteXML()}. + * + * @throws IOException if an I/O error occurs + */ + protected void restoreHBaseSiteXML() throws IOException { + // restore hbase-site.xml + Files.copy(backupConfigFileUnderTestDataDir, + configFileUnderTestDataDir, StandardCopyOption.REPLACE_EXISTING); + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAdminBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAdminBase.java index 73529c084ac9..c57ef8dca7e6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAdminBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncAdminBase.java @@ -46,7 +46,7 @@ /** * Class to test AsyncAdmin. */ -public abstract class TestAsyncAdminBase { +public abstract class TestAsyncAdminBase extends AbstractTestUpdateConfiguration { protected static final Logger LOG = LoggerFactory.getLogger(TestAsyncAdminBase.class); protected final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncClusterAdminApi.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncClusterAdminApi.java index ec6639a5ed3d..d9f828286c9e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncClusterAdminApi.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncClusterAdminApi.java @@ -22,10 +22,6 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.Collection; import java.util.EnumSet; @@ -65,19 +61,19 @@ public class TestAsyncClusterAdminApi extends TestAsyncAdminBase { public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestAsyncClusterAdminApi.class); - private final Path cnfPath = FileSystems.getDefault().getPath("target/test-classes/hbase-site.xml"); - private final Path cnf2Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site2.xml"); - private final Path cnf3Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site3.xml"); - @BeforeClass public static void setUpBeforeClass() throws Exception { + + setUpConfigurationFiles(TEST_UTIL); TEST_UTIL.getConfiguration().setInt(HConstants.MASTER_INFO_PORT, 0); TEST_UTIL.getConfiguration().setInt(HConstants.HBASE_RPC_TIMEOUT_KEY, 60000); TEST_UTIL.getConfiguration().setInt(HConstants.HBASE_CLIENT_OPERATION_TIMEOUT, 120000); TEST_UTIL.getConfiguration().setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 2); TEST_UTIL.getConfiguration().setInt(START_LOG_ERRORS_AFTER_COUNT_KEY, 0); + TEST_UTIL.startMiniCluster(2); ASYNC_CONN = ConnectionFactory.createAsyncConnection(TEST_UTIL.getConfiguration()).get(); + addResourceToRegionServerConfiguration(TEST_UTIL); } @Test @@ -137,18 +133,6 @@ public void testAllClusterOnlineConfigChange() throws IOException { restoreHBaseSiteXML(); } - private void replaceHBaseSiteXML() throws IOException { - // make a backup of hbase-site.xml - Files.copy(cnfPath, cnf3Path, StandardCopyOption.REPLACE_EXISTING); - // update hbase-site.xml by overwriting it - Files.copy(cnf2Path, cnfPath, StandardCopyOption.REPLACE_EXISTING); - } - - private void restoreHBaseSiteXML() throws IOException { - // restore hbase-site.xml - Files.copy(cnf3Path, cnfPath, StandardCopyOption.REPLACE_EXISTING); - } - @Test public void testRollWALWALWriter() throws Exception { setUpforLogRolling(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestUpdateConfiguration.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestUpdateConfiguration.java index eae56fdd53c6..e8036fc4e9af 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestUpdateConfiguration.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestUpdateConfiguration.java @@ -20,10 +20,6 @@ import static org.junit.Assert.assertEquals; import java.io.IOException; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; @@ -32,13 +28,15 @@ import org.apache.hadoop.hbase.testclassification.MediumTests; import org.junit.BeforeClass; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @Category({MediumTests.class}) -public class TestUpdateConfiguration { +public class TestUpdateConfiguration extends AbstractTestUpdateConfiguration { @ClassRule public static final HBaseClassTestRule CLASS_RULE = @@ -49,14 +47,18 @@ public class TestUpdateConfiguration { @BeforeClass public static void setup() throws Exception { - // Set master number and use default values for other options. + setUpConfigurationFiles(TEST_UTIL); StartMiniClusterOption option = StartMiniClusterOption.builder().numMasters(2).build(); TEST_UTIL.startMiniCluster(option); + addResourceToRegionServerConfiguration(TEST_UTIL); } + @Rule + public TestName name = new TestName(); + @Test public void testOnlineConfigChange() throws IOException { - LOG.debug("Starting the test"); + LOG.debug("Starting the test {}", name.getMethodName()); Admin admin = TEST_UTIL.getAdmin(); ServerName server = TEST_UTIL.getHBaseCluster().getRegionServer(0).getServerName(); admin.updateConfiguration(server); @@ -64,42 +66,28 @@ public void testOnlineConfigChange() throws IOException { @Test public void testMasterOnlineConfigChange() throws IOException { - LOG.debug("Starting the test"); - Path cnfPath = FileSystems.getDefault().getPath("target/test-classes/hbase-site.xml"); - Path cnf2Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site2.xml"); - Path cnf3Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site3.xml"); - // make a backup of hbase-site.xml - Files.copy(cnfPath, cnf3Path, StandardCopyOption.REPLACE_EXISTING); - // update hbase-site.xml by overwriting it - Files.copy(cnf2Path, cnfPath, StandardCopyOption.REPLACE_EXISTING); - + LOG.debug("Starting the test {}", name.getMethodName()); + replaceHBaseSiteXML(); Admin admin = TEST_UTIL.getAdmin(); ServerName server = TEST_UTIL.getHBaseCluster().getMaster().getServerName(); admin.updateConfiguration(server); Configuration conf = TEST_UTIL.getMiniHBaseCluster().getMaster().getConfiguration(); int custom = conf.getInt("hbase.custom.config", 0); assertEquals(1000, custom); - // restore hbase-site.xml - Files.copy(cnf3Path, cnfPath, StandardCopyOption.REPLACE_EXISTING); + restoreHBaseSiteXML(); } @Test public void testAllOnlineConfigChange() throws IOException { - LOG.debug("Starting the test"); + LOG.debug("Starting the test {} ", name.getMethodName()); Admin admin = TEST_UTIL.getAdmin(); admin.updateConfiguration(); } @Test public void testAllCustomOnlineConfigChange() throws IOException { - LOG.debug("Starting the test"); - Path cnfPath = FileSystems.getDefault().getPath("target/test-classes/hbase-site.xml"); - Path cnf2Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site2.xml"); - Path cnf3Path = FileSystems.getDefault().getPath("target/test-classes/hbase-site3.xml"); - // make a backup of hbase-site.xml - Files.copy(cnfPath, cnf3Path, StandardCopyOption.REPLACE_EXISTING); - // update hbase-site.xml by overwriting it - Files.copy(cnf2Path, cnfPath, StandardCopyOption.REPLACE_EXISTING); + LOG.debug("Starting the test {}", name.getMethodName()); + replaceHBaseSiteXML(); Admin admin = TEST_UTIL.getAdmin(); admin.updateConfiguration(); @@ -120,7 +108,6 @@ public void testAllCustomOnlineConfigChange() throws IOException { custom = regionServerConfiguration.getInt("hbase.custom.config", 0); assertEquals(1000, custom); - // restore hbase-site.xml - Files.copy(cnf3Path, cnfPath, StandardCopyOption.REPLACE_EXISTING); + restoreHBaseSiteXML(); } } diff --git a/hbase-server/src/test/resources/hbase-site2.xml b/hbase-server/src/test/resources/override-hbase-site.xml similarity index 100% rename from hbase-server/src/test/resources/hbase-site2.xml rename to hbase-server/src/test/resources/override-hbase-site.xml