xBis7 commented on code in PR #4521:
URL: https://github.com/apache/ozone/pull/4521#discussion_r1156101037


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestStorageContainerManagerHA.java:
##########
@@ -61,13 +58,14 @@
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.TimeoutException;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.hadoop.hdds.client.ReplicationFactor.ONE;
 import static org.apache.hadoop.hdds.client.ReplicationType.RATIS;
 
 /**
- * Base class for Ozone Manager HA tests.
+ * Base class for SCM HA tests.
  */
 public class TestStorageContainerManagerHA {

Review Comment:
   You can add the timeout above the class and it will be applied to every 
method.
   
   ```suggestion
   @Timeout(value = 300)
   public class TestStorageContainerManagerHA {
   ```



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestStorageContainerManagerHA.java:
##########
@@ -284,36 +280,80 @@ public void testInvalidHAConfig(boolean isRatisEnabled) 
throws Exception {
             () -> StorageContainerManager.scmInit(conf, clusterId));
   }
 
-
-
   @Test
+  @Timeout(300)
   public void testBootStrapSCM() throws Exception {
     StorageContainerManager scm2 = 
cluster.getStorageContainerManagers().get(1);
     OzoneConfiguration conf2 = scm2.getConfiguration();
     boolean isDeleted = scm2.getScmStorageConfig().getVersionFile().delete();
-    Assert.assertTrue(isDeleted);
+    Assertions.assertTrue(isDeleted);
     final SCMStorageConfig scmStorageConfig = new SCMStorageConfig(conf2);
     scmStorageConfig.setClusterId(UUID.randomUUID().toString());
     scmStorageConfig.getCurrentDir().delete();
     scmStorageConfig.setSCMHAFlag(true);
     scmStorageConfig.initialize();
     conf2.setBoolean(ScmConfigKeys.OZONE_SCM_SKIP_BOOTSTRAP_VALIDATION_KEY,
         false);
-    Assert.assertFalse(StorageContainerManager.scmBootstrap(conf2));
+    Assertions.assertFalse(StorageContainerManager.scmBootstrap(conf2));
     conf2.setBoolean(ScmConfigKeys.OZONE_SCM_SKIP_BOOTSTRAP_VALIDATION_KEY,
         true);
-    Assert.assertTrue(StorageContainerManager.scmBootstrap(conf2));
+    Assertions.assertTrue(StorageContainerManager.scmBootstrap(conf2));
   }
 
   @Test
+  @Timeout(300)
   public void testGetRatisRolesDetail() throws IOException {
     Set<String> resultSet = new HashSet<>();
     for (StorageContainerManager scm: cluster.getStorageContainerManagers()) {
       resultSet.addAll(scm.getScmHAManager().getRatisServer().getRatisRoles());
     }
     System.out.println(resultSet);
-    Assert.assertEquals(3, resultSet.size());
-    Assert.assertEquals(1,
+    Assertions.assertEquals(3, resultSet.size());
+    Assertions.assertEquals(1,
         resultSet.stream().filter(x -> x.contains("LEADER")).count());
   }
+
+  @Test
+  @Timeout(300)
+  public void testSCMHAMetrics() throws InterruptedException, TimeoutException 
{
+    waitForLeaderToBeReady();
+
+    StorageContainerManager leaderSCM = cluster.getActiveSCM();
+    String leaderSCMId = leaderSCM.getScmId();
+    List<StorageContainerManager> scms =
+        cluster.getStorageContainerManagersList();
+
+    checkSCMHAMetricsForAllSCMs(scms, leaderSCMId);
+  }
+
+  private void checkSCMHAMetricsForAllSCMs(List<StorageContainerManager> scms,
+      String leaderSCMId) {
+    for (StorageContainerManager scm : scms) {
+      String nodeId = scm.getScmId();
+
+      SCMHAMetrics scmHAMetrics = scm.getScmHAMetrics();
+      // If current SCM is leader, state should be 1
+      int expectedState = nodeId.equals(leaderSCMId) ? 1 : 0;
+
+      Assertions.assertEquals(expectedState,
+          scmHAMetrics.getSCMHAMetricsInfoLeaderState());
+      Assertions.assertEquals(nodeId, 
scmHAMetrics.getSCMHAMetricsInfoNodeId());
+    }
+  }
+
+  /**
+   * Some tests are stopping or restarting SCMs.
+   * There are test cases where we might need to
+   * wait for a leader to be elected and ready.
+   */
+  private void waitForLeaderToBeReady()
+      throws InterruptedException, TimeoutException {
+    GenericTestUtils.waitFor(() -> {
+      try {
+        return cluster.getActiveSCM().checkLeader();
+      } catch (Exception e) {
+        return false;
+      }
+    }, 1000, 80000);

Review Comment:
   You can use the default timeout for SCM Ratis node failure.
   
   
https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfigKeys.java#L529-L530
   
   For OMHAMetrics, this timeout turned out to be too short and the test was 
marked as flaky.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestStorageContainerManagerHA.java:
##########
@@ -284,36 +280,80 @@ public void testInvalidHAConfig(boolean isRatisEnabled) 
throws Exception {
             () -> StorageContainerManager.scmInit(conf, clusterId));
   }
 
-
-
   @Test
+  @Timeout(300)
   public void testBootStrapSCM() throws Exception {
     StorageContainerManager scm2 = 
cluster.getStorageContainerManagers().get(1);
     OzoneConfiguration conf2 = scm2.getConfiguration();
     boolean isDeleted = scm2.getScmStorageConfig().getVersionFile().delete();
-    Assert.assertTrue(isDeleted);
+    Assertions.assertTrue(isDeleted);
     final SCMStorageConfig scmStorageConfig = new SCMStorageConfig(conf2);
     scmStorageConfig.setClusterId(UUID.randomUUID().toString());
     scmStorageConfig.getCurrentDir().delete();
     scmStorageConfig.setSCMHAFlag(true);
     scmStorageConfig.initialize();
     conf2.setBoolean(ScmConfigKeys.OZONE_SCM_SKIP_BOOTSTRAP_VALIDATION_KEY,
         false);
-    Assert.assertFalse(StorageContainerManager.scmBootstrap(conf2));
+    Assertions.assertFalse(StorageContainerManager.scmBootstrap(conf2));
     conf2.setBoolean(ScmConfigKeys.OZONE_SCM_SKIP_BOOTSTRAP_VALIDATION_KEY,
         true);
-    Assert.assertTrue(StorageContainerManager.scmBootstrap(conf2));
+    Assertions.assertTrue(StorageContainerManager.scmBootstrap(conf2));
   }
 
   @Test
+  @Timeout(300)
   public void testGetRatisRolesDetail() throws IOException {
     Set<String> resultSet = new HashSet<>();
     for (StorageContainerManager scm: cluster.getStorageContainerManagers()) {
       resultSet.addAll(scm.getScmHAManager().getRatisServer().getRatisRoles());
     }
     System.out.println(resultSet);
-    Assert.assertEquals(3, resultSet.size());
-    Assert.assertEquals(1,
+    Assertions.assertEquals(3, resultSet.size());
+    Assertions.assertEquals(1,
         resultSet.stream().filter(x -> x.contains("LEADER")).count());
   }
+
+  @Test
+  @Timeout(300)
+  public void testSCMHAMetrics() throws InterruptedException, TimeoutException 
{
+    waitForLeaderToBeReady();
+
+    StorageContainerManager leaderSCM = cluster.getActiveSCM();
+    String leaderSCMId = leaderSCM.getScmId();
+    List<StorageContainerManager> scms =
+        cluster.getStorageContainerManagersList();
+
+    checkSCMHAMetricsForAllSCMs(scms, leaderSCMId);
+  }

Review Comment:
   How about using `shutdownStorageContainerManager()` and then 
`restartStorageContainerManager()` to shutdown and restart the leader and then 
check again the metrics?
   
   
https://github.com/apache/ozone/blob/master/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java#L250-L273



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