chihsuan commented on code in PR #11528:
URL: https://github.com/apache/ozone/pull/11528#discussion_r4305809821
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java:
##########
@@ -1799,7 +1799,7 @@ private void stopContainerExportManager() {
@Override
public void shutDown(String message) {
stop();
- ExitUtils.terminate(0, message, LOG);
+ ExitUtils.terminate(1, message, LOG);
Review Comment:
I noticed a decommissioned SCM also reaches this through
`SCMStateMachine.close()` and may exit with 1. Could we keep status 0 for that
expected shutdown?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestStorageContainerManager.java:
##########
@@ -625,6 +627,39 @@ public void testScmInfo(@TempDir Path tempDir) throws
Exception {
}
}
+ @Test
+ void testStateMachineClosedByRatisTerminatesWithFailureStatus(@TempDir Path
tempDir) throws Exception {
Review Comment:
Since this calls `close()` directly, could we rename it to
`testStateMachineCloseTerminatesWithFailureStatus`?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestStorageContainerManager.java:
##########
@@ -625,6 +627,39 @@ public void testScmInfo(@TempDir Path tempDir) throws
Exception {
}
}
+ @Test
+ void testStateMachineClosedByRatisTerminatesWithFailureStatus(@TempDir Path
tempDir) throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ conf.set(HddsConfigKeys.OZONE_METADATA_DIRS,
tempDir.resolve("scm-meta").toString());
+ StorageContainerManager.scmInit(conf, UUID.randomUUID().toString());
+ ExitUtils.disableSystemExit();
+ StorageContainerManager scm = HddsTestUtils.getScmSimple(conf);
+ try {
+ scm.start();
+ waitForStateMachineReady(scm);
+ scm.stop();
+ scm.join();
+ assertThat(ExitUtils.isTerminated()).isFalse();
+
+ scm = HddsTestUtils.getScmSimple(conf);
+ scm.start();
+ waitForStateMachineReady(scm);
+ SCMStateMachine stateMachine =
scm.getScmHAManager().getRatisServer().getSCMStateMachine();
+ ExitUtils.ExitException exit =
assertThrows(ExitUtils.ExitException.class, stateMachine::close);
+ assertThat(exit.getStatus()).isEqualTo(1);
+ assertThat(scm.isStopped()).isTrue();
+ } finally {
+ scm.stop();
+ ExitUtils.clear();
+ }
+ }
+
+ /** Waits until the datanode RPC server is started, so that stopping SCM
releases its port. */
Review Comment:
Nit: `isStateMachineReady` flips before the datanode server starts. Could
the comment say it waits for the state machine to be ready?
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestStorageContainerManager.java:
##########
@@ -625,6 +627,39 @@ public void testScmInfo(@TempDir Path tempDir) throws
Exception {
}
}
+ @Test
+ void testStateMachineClosedByRatisTerminatesWithFailureStatus(@TempDir Path
tempDir) throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ conf.set(HddsConfigKeys.OZONE_METADATA_DIRS,
tempDir.resolve("scm-meta").toString());
+ StorageContainerManager.scmInit(conf, UUID.randomUUID().toString());
+ ExitUtils.disableSystemExit();
+ StorageContainerManager scm = HddsTestUtils.getScmSimple(conf);
+ try {
+ scm.start();
+ waitForStateMachineReady(scm);
+ scm.stop();
+ scm.join();
+ assertThat(ExitUtils.isTerminated()).isFalse();
+
+ scm = HddsTestUtils.getScmSimple(conf);
+ scm.start();
+ waitForStateMachineReady(scm);
+ SCMStateMachine stateMachine =
scm.getScmHAManager().getRatisServer().getSCMStateMachine();
+ ExitUtils.ExitException exit =
assertThrows(ExitUtils.ExitException.class, stateMachine::close);
Review Comment:
Could we also add a decommission case to `TestStorageContainerManagerHA`? It
could wait for the removed SCM to terminate and check the status.
--
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]