You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "mladjan-gadzic (via GitHub)" <gi...@apache.org> on 2023/04/03 15:01:11 UTC
[GitHub] [ozone] mladjan-gadzic opened a new pull request, #4521: HDDS-8363. Make SCM Ratis roles available in /prom endpoint
mladjan-gadzic opened a new pull request, #4521:
URL: https://github.com/apache/ozone/pull/4521
## What changes were proposed in this pull request?
Add SCM HA metrics to `/jmx` and `/prom` endpoints.
```
{
"name": "Hadoop:service=StorageContainerManager,name=SCMHAMetrics",
"modelerType": "SCMHAMetrics",
"tag.NodeId": "3c8c82f5-524a-41f4-beea-8dcd868fbf91",
"tag.Hostname": "bca1fb37b8aa",
"SCMHALeaderState": 1
}
```
```
# TYPE scmha_metrics_scmha_leader_state gauge
scmha_metrics_scmha_leader_state{nodeid="3c8c82f5-524a-41f4-beea-8dcd868fbf91",hostname="bca1fb37b8aa"} 1
```
Similar PR: https://github.com/apache/ozone/pull/4140.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8363
## How was this patch tested?
- unit
- integration
- manual
Steps for manual test:
- start Docker cluster for HA environment `ozone-ha` and wait for it to be ready
- check if SCM HA metrics are initialized correctly
- stop SCM leader node and wait for new leader to be elected
- check if SCM HA metrics are updated correctly
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #4521: HDDS-8363. Make SCM Ratis roles available in /prom endpoint
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4521:
URL: https://github.com/apache/ozone/pull/4521
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] xBis7 commented on a diff in pull request #4521: HDDS-8363. Make SCM Ratis roles available in /prom endpoint
Posted by "xBis7 (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #4521: HDDS-8363. Make SCM Ratis roles available in /prom endpoint
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4521:
URL: https://github.com/apache/ozone/pull/4521#issuecomment-1510777905
Thanks @mladjan-gadzic for the patch, @xBis7 for the review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] mladjan-gadzic commented on a diff in pull request #4521: HDDS-8363. Make SCM Ratis roles available in /prom endpoint
Posted by "mladjan-gadzic (via GitHub)" <gi...@apache.org>.
mladjan-gadzic commented on code in PR #4521:
URL: https://github.com/apache/ozone/pull/4521#discussion_r1156181117
##########
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:
I'd say there is no need for that because SCMHAMetrics are initialized the way that `leaderId=null`: https://github.com/mladjan-gadzic/ozone/blob/HDDS-8363/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java#L1497
Test checks if there is a leader and there is which means it was updated since it was initialized. Leader is being updated here: https://github.com/mladjan-gadzic/ozone/blob/HDDS-8363/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java#L287
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] mladjan-gadzic commented on pull request #4521: HDDS-8363. Make SCM Ratis roles available in /prom endpoint
Posted by "mladjan-gadzic (via GitHub)" <gi...@apache.org>.
mladjan-gadzic commented on PR #4521:
URL: https://github.com/apache/ozone/pull/4521#issuecomment-1494619018
@xBis7 thanks for the review!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org