rakeshadr commented on a change in pull request #2689:
URL: https://github.com/apache/ozone/pull/2689#discussion_r718182900



##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHA.java
##########
@@ -171,22 +173,29 @@ public void init() throws Exception {
      */
     conf.set(OZONE_BLOCK_DELETING_SERVICE_INTERVAL, "10s");
     conf.set(OZONE_KEY_DELETING_LIMIT_PER_TASK, "2");
-    additionalConfiguration();
 
     clusterBuilder = MiniOzoneCluster.newOMHABuilder(conf)
         .setClusterId(clusterId)
         .setScmId(scmId)
         .setOMServiceId(omServiceId)
         .setOmId(omId)
         .setNumOfOzoneManagers(numOfOMs);
-    additionalClusterSettings();
 
-    cluster = (MiniOzoneOMHAClusterImpl)clusterBuilder.build();
+    cluster = (MiniOzoneOMHAClusterImpl) clusterBuilder.build();
     cluster.waitForClusterToBeReady();
     objectStore = OzoneClientFactory.getRpcClient(omServiceId, conf)
         .getObjectStore();
   }
 
+  /**
+   * Apply additional configuration between tests.
+   */
+  @Before
+  public void setupTest() {

Review comment:
       @JyotinderSingh As the cluster is already created before executing the 
`setupTest()` function, there is no impact on adding these configurations. I 
think, we can remove this method safely because this was added to speed up the 
cluster setup for each test case and no functional impact. Now, we are very 
much better by creating cluster only once for all the test cases.
   
   IMHO, we can safely remove #additionalConfiguration() and 
#additionalClusterSettings() methods from everywhere in the test classes.

##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerPrepare.java
##########
@@ -175,7 +184,11 @@ public void testPrepareDownedOM() throws Exception {
 
   @Test
   public void testPrepareWithRestart() throws Exception {
+    // Create fresh cluster for this test to prevent timeout from restarting

Review comment:
       @JyotinderSingh  Do we need a new cluster? I ran 10 times in my laptop 
without a new cluster(without init() and setup() calls ) and passed 
consistently. Please double check it. Also, if we need to setup new cluster 
then please shutdown the previously instantiated cluster by invoking 
`shutdown();` function before `init()`.
   ```
       // Create fresh cluster for this test to prevent timeout from restarting
       // modified cluster.
       init();
       setup();
   ```




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to