adoroszlai commented on a change in pull request #2953:
URL: https://github.com/apache/ozone/pull/2953#discussion_r800157521
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java
##########
@@ -394,6 +394,10 @@ public void close() throws IOException {
transactionBuffer.close();
HadoopExecutors.
shutdown(installSnapshotExecutor, LOG, 5, TimeUnit.SECONDS);
+ if(!scm.isRunForTest()) {
+ ExitUtils.terminate(1, "ScmStateMachine is closed, shutdown SCM",
+ StateMachine.LOG);
+ }
Review comment:
I don't think we need this new `isRunForTest` flag.
`ExitUtils.disableSystemExit()` serves exactly the same purpose (see 30cb5e764
for similar change in integration tests).
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java
##########
@@ -337,8 +337,8 @@ void reloadSCMState()
public void shutdown() throws IOException {
if (ratisServer != null) {
ratisServer.stop();
- ratisServer.getSCMStateMachine().close();
grpcServer.stop();
+ ratisServer.getSCMStateMachine().close();
Review comment:
Can you please explain this change in order of statements?
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1926,7 +1926,6 @@ public void stop() {
if (omSnapshotProvider != null) {
omSnapshotProvider.stop();
}
- omState = State.STOPPED;
Review comment:
@JacksonYao287 Thanks for the info, I didn't notice the other state
change at the start of the `stop()` method.
@hanishakoneru `omState = State.STOPPED` was set both at the start and end
of `stop()` (added in 839fc94e1b and 410e4d3f54, respectively). Do you recall
any specific reason for that, or was it just a harmless duplication?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
##########
@@ -1507,6 +1509,13 @@ public void stop() {
}
scmSafeModeManager.stop();
+
+ try {
+ LOG.info("Stopping SCM HA services.");
+ scmHAManager.shutdown();
+ } catch (Exception ex) {
+ LOG.error("SCM HA Manager stop failed", ex);
+ }
Review comment:
Can you please explain this change in order of statements?
--
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]
