zaynt4606 commented on code in PR #3838:
URL: https://github.com/apache/celeborn/pull/3838#discussion_r4236259886


##########
master/src/main/java/org/apache/celeborn/service/deploy/master/clustermeta/ha/HARaftServer.java:
##########
@@ -311,6 +319,34 @@ public void stop(boolean transferLeadership) {
     }
   }
 
+  private void checkRaftServerState() {
+    if (stopped.get() || unexpectedCloseHandled.get()) {

Review Comment:
   There is a race between this guard and a normal `stop()`. The scheduled 
checker can read `stopped == false` here; another thread can then set `stopped` 
to true and close the Raft server; this checker subsequently observes `CLOSED` 
and runs the default `System.exit(1)` handler. This can turn an intentional 
shutdown into a failure exit and trigger a restart under a restart-on-failure 
supervisor. Please recheck `stopped` after reading the lifecycle state, before 
claiming `unexpectedCloseHandled`, and add a test for this interleaving.



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

Reply via email to