[GitHub] [hadoop] steveloughran commented on a diff in pull request #4835: HDFS-16740. Mini cluster test flakiness

["steveloughran (via GitHub)" <gi...@apache.org>]

steveloughran commented on code in PR #4835:
URL: https://github.com/apache/hadoop/pull/4835#discussion_r1329984828


##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/MiniJournalCluster.java:
##########
@@ -289,7 +293,8 @@ public void waitActive() throws IOException {
           }
         }, 50, 3000);
       } catch (TimeoutException e) {
-        fail("Time out while waiting for journal node " + index + " to start.");
+        throw new AssertionError("Time out while waiting for journal node " + index +

Review Comment:
   any need to include the inner cause/stack trace? I don't see a compelling one



##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/qjournal/MiniQJMHACluster.java:
##########
@@ -60,6 +61,13 @@ public Builder(Configuration conf) {
       this.dfsBuilder = new MiniDFSCluster.Builder(conf).numDataNodes(0);
     }
 
+    public Builder(Configuration conf, File baseDir) {

Review Comment:
   nit: add a javadoc



##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestBPOfferService.java:
##########
@@ -1254,8 +1259,7 @@ public void testSetIsSlownode() throws Exception {
   @Test(timeout = 15000)
   public void testCommandProcessingThread() throws Exception {
     Configuration conf = new HdfsConfiguration();
-    MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build();
-    try {
+    try (MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf, baseDir.getRoot()).build()) {

Review Comment:
   creating that builder with a new HdfsConfiguration is something which may make sense to factor out. certainly i'd request that in new code. What do you think?



-- 
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.

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org