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();
```
##########
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:
OK, good catch. I'm OK to re-setup cluster after`#shutdown()` first one.
BTW, can you point me to the build link, we can download the build artifact
and check the logs to understand the behavior.
--
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]
