JacksonYao287 commented on code in PR #2953:
URL: https://github.com/apache/ozone/pull/2953#discussion_r845706087


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerStateMachine.java:
##########
@@ -470,8 +469,12 @@ public void close() throws IOException {
     // OM should be shutdown as the StateMachine has shutdown.
     LOG.info("StateMachine has shutdown. Shutdown OzoneManager if not " +
         "already shutdown.");
-    ozoneManager.shutdown(new RaftException("RaftServer called shutdown on " +
-        "StateMachine"));
+    super.close();
+    if (!ozoneManager.isStopped()) {
+      ozoneManager.stop();
+    }
+    ExitUtils.terminate(1, "OM state machine is shutdown by Ratis server",
+        null, false, LOG);

Review Comment:
   @hanishakoneru thanks very much for the review and sorry for the late reply!
   in our production environment, we found that in some case statemachine is 
closed by ratis, but the base process(scm/om) is still running. i don`t think 
this is a good state and sometimes confusing. so i create this patch to make 
sure if statemachine is closed , the base process will be terminated.
   
   >OzoneManager should have two methods - stop() and shutdown(). Stop() would 
just stop the services which is needed for tests. Shutdown can call stop() and 
then terminate the process.
   Critical failures, like the one here is when StateMachine is closed, would 
then call OzoneManager#shutdown to terminate the process.
   
   i totally agree with this, it seems graceful and clear.



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