adoroszlai commented on a change in pull request #3213:
URL: https://github.com/apache/ozone/pull/3213#discussion_r830448654



##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithData.java
##########
@@ -431,16 +431,16 @@ public void testOMRestart() throws Exception {
         .build();
 
     objectStore.createVolume(volumeName, createVolumeArgs);
-    OzoneVolume retVolumeinfo = objectStore.getVolume(volumeName);
+    OzoneVolume ozoneVolume = objectStore.getVolume(volumeName);

Review comment:
       The new name is indeed better, but this refactoring is not essential for 
the fix.  There are several other improvements that we could make in this test, 
but should not mix it with bugfix.

##########
File path: 
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerHAWithData.java
##########
@@ -450,50 +450,36 @@ public void testOMRestart() throws Exception {
     // the logs corresponding to atleast some of the missed transactions
     // should be purged. This will force the OM to install snapshot when
     // restarted.
-    long minNewTxIndex = lastAppliedTxOnFollowerOM + (getLogPurgeGap() * 10);
-    long leaderOMappliedLogIndex = leaderOM.getOmRatisServer()
-        .getLastAppliedTermIndex().getIndex();
-
-    List<String> missedKeys = new ArrayList<>();
-    while (leaderOMappliedLogIndex < minNewTxIndex) {
-      missedKeys.add(createKey(ozoneBucket));
-      leaderOMappliedLogIndex = leaderOM.getOmRatisServer()
-          .getLastAppliedTermIndex().getIndex();
+    long minNewTxIndex = followerOM1LastAppliedIndex + getLogPurgeGap() * 10L;
+    while (leaderOM.getOmRatisServer().getLastAppliedTermIndex().getIndex()
+        < minNewTxIndex) {
+      createKey(ozoneBucket);
     }
 
-    // Restart the stopped OM.
-    followerOM1.restart();
-
     // Get the latest snapshotIndex from the leader OM.
-    long leaderOMSnaphsotIndex = leaderOM.getRatisSnapshotIndex();
+    final long leaderOMSnaphsotIndex = leaderOM.getRatisSnapshotIndex();
 
-    // The recently started OM should be lagging behind the leader OM.
-    long followerOMLastAppliedIndex =
-        followerOM1.getOmRatisServer().getLastAppliedTermIndex().getIndex();
-    Assert.assertTrue(
-        followerOMLastAppliedIndex < leaderOMSnaphsotIndex);
+    // The stopped OM should be lagging behind the leader OM.
+    Assert.assertTrue(followerOM1LastAppliedIndex < leaderOMSnaphsotIndex);

Review comment:
       I agree that it is better to make this assertion before restarting the 
follower, instead of removing it like I did in #3214.




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