133tosakarin commented on code in PR #1168:
URL: https://github.com/apache/ratis/pull/1168#discussion_r1805785798


##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -613,24 +606,33 @@ private synchronized CompletableFuture<Void> 
changeToFollowerAsync(
     return future;
   }
 
-  synchronized void changeToFollowerAndPersistMetadata(
+  synchronized CompletableFuture<Void> changeToFollowerAndPersistMetadata(
       long newTerm,
       boolean allowListener,
       Object reason) throws IOException {
-    if (changeToFollower(newTerm, false, allowListener, reason)) {
-      state.persistMetadata();
+    final AtomicBoolean metadataUpdated = new AtomicBoolean();
+    final CompletableFuture<Void> future = changeToFollower(newTerm, false, 
allowListener, reason, metadataUpdated);
+    try {
+      if (metadataUpdated.get()) {
+        state.persistMetadata();
+      }
+    } catch (IOException e) {
+      CompletableFuture.runAsync(future::join);

Review Comment:
   Assuming we encounter an exception here, we either do not handle the future 
or need another thread to handle it. If we do not use another thread to handle 
the future, there is still a deadlock problem of holding the lock and waiting 
for the future.
   
   The same consideration applies to using runAsync elsewhere



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