qiaoandxiang commented on a change in pull request #1310: HBASE-23957 [flakey test] client.TestMultiParallel fails to read hbas… URL: https://github.com/apache/hbase/pull/1310#discussion_r395308292
########## File path: hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncClusterAdminApi.java ########## @@ -65,19 +67,51 @@ 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"); + private static List<Path> newCnfPathes = new ArrayList<>(); @BeforeClass public static void setUpBeforeClass() 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 = TEST_UTIL.getDataTestDir().toString(); + String dataBasePath = TEST_UTIL.getBaseTestDir().toString(); + String dataPath = absoluteDataPath.substring(absoluteDataPath.indexOf(dataBasePath)); + // Copy Files from target/test-classes to data directory, this is to avoid messing up + // the config file under target/test-class, which is supposed to be immutable. + Files.createDirectories(Paths.get(dataPath)); + + List<String> confs = Arrays.asList("/hbase-site.xml", "/hbase-site2.xml", "/hbase-site3.xml"); + for (String conf : confs) { + Path cnfPath = FileSystems.getDefault().getPath("target/test-classes" + conf); + Path newConfPath = Paths.get(dataPath + conf); + Files.copy(cnfPath, newConfPath); + newCnfPathes.add(newConfPath); + } + 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); + + // Add the new custom config file to Configuration + TEST_UTIL.getConfiguration().addResource(TEST_UTIL.getDataTestDir("hbase-site.xml")); TEST_UTIL.startMiniCluster(2); ASYNC_CONN = ConnectionFactory.createAsyncConnection(TEST_UTIL.getConfiguration()).get(); + + // The resources added to TEST_UTIL.conf is lost during Region Server Creation, hack it here + // again. + for (int i = 0; i < 2; i ++) { Review comment: Hmm, I think it is possible to use 1 region server. The original test uses 2 region servers so I kept that way. For 2, yeah, definitely possible and I will make change in my next patch. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services