ndimiduk 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_r395225230
 
 

 ##########
 File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestUpdateConfiguration.java
 ##########
 @@ -47,31 +51,61 @@
   private static final Logger LOG = 
LoggerFactory.getLogger(TestUpdateConfiguration.class);
   private final static HBaseTestingUtility TEST_UTIL = new 
HBaseTestingUtility();
 
+  private static List<Path> newCnfPathes = new ArrayList<>();
+
   @BeforeClass
   public static void setup() throws Exception {
-    // Set master number and use default values for other options.
+    // 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);
+    }
+
+    // Add the new custom config file to Configuration
+    
TEST_UTIL.getConfiguration().addResource(TEST_UTIL.getDataTestDir("hbase-site.xml"));
     StartMiniClusterOption option = 
StartMiniClusterOption.builder().numMasters(2).build();
     TEST_UTIL.startMiniCluster(option);
+
+    // The resources added to TEST_UTIL.conf is lost during Region Server 
Creation, hack it here
+    // again.
+    TEST_UTIL.getMiniHBaseCluster().getRegionServer(0).getConfiguration().
+      addResource(TEST_UTIL.getDataTestDir("hbase-site.xml"));
   }
 
   @Test
   public void testOnlineConfigChange() throws IOException {
-    LOG.debug("Starting the test");
+    LOG.debug("Starting the test testOnlineConfigChange");
     Admin admin = TEST_UTIL.getAdmin();
     ServerName server = 
TEST_UTIL.getHBaseCluster().getRegionServer(0).getServerName();
     admin.updateConfiguration(server);
   }
 
   @Test
   public void testMasterOnlineConfigChange() throws IOException {
-    LOG.debug("Starting the test");
-    Path cnfPath = 
FileSystems.getDefault().getPath("target/test-classes/hbase-site.xml");
 
 Review comment:
   same comment about descriptive variable names.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to