This is an automated email from the ASF dual-hosted git repository.

tanxinyu pushed a commit to branch consensus_module_refactor
in repository https://gitbox.apache.org/repos/asf/iotdb.git

commit 60f4c247da117f3139fecba009fd3e627f778fdf
Author: BUAAserein <[email protected]>
AuthorDate: Thu Aug 17 18:01:12 2023 +0800

    refactor triggerSnapshot
---
 .../iotdb/consensus/ratis/RatisConsensus.java      | 22 ++++++++--------------
 .../iotdb/consensus/ratis/RatisConsensusTest.java  |  2 +-
 .../consensus/simple/SimpleConsensusTest.java      | 10 ++++++----
 .../apache/iotdb/db/service/IoTDBShutdownHook.java | 12 +++++++++++-
 4 files changed, 26 insertions(+), 20 deletions(-)

diff --git 
a/iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java
 
b/iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java
index df72318f4ee..88c8072630c 100644
--- 
a/iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java
+++ 
b/iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/ratis/RatisConsensus.java
@@ -486,18 +486,18 @@ class RatisConsensus implements IConsensus {
    * change
    */
   @Override
-  public ConsensusGenericResponse removeRemotePeer(ConsensusGroupId groupId, 
Peer peer) {
+  public void removeRemotePeer(ConsensusGroupId groupId, Peer peer) throws 
ConsensusException {
     RaftGroupId raftGroupId = Utils.fromConsensusGroupIdToRaftGroupId(groupId);
     RaftGroup group = getGroupInfo(raftGroupId);
     RaftPeer peerToRemove = Utils.fromPeerAndPriorityToRaftPeer(peer, 
DEFAULT_PRIORITY);
 
     // pre-conditions: group exists and myself in this group
     if (group == null || !group.getPeers().contains(myself)) {
-      return failed(new ConsensusGroupNotExistException(groupId));
+      throw new ConsensusGroupNotExistException(groupId);
     }
     // pre-condition: peer is a member of groupId
     if (!group.getPeers().contains(peerToRemove)) {
-      return failed(new PeerNotInConsensusGroupException(groupId, myself));
+      throw new PeerNotInConsensusGroupException(groupId, myself);
     }
 
     // update group peer information
@@ -510,10 +510,8 @@ class RatisConsensus implements IConsensus {
     try {
       reply = sendReconfiguration(RaftGroup.valueOf(raftGroupId, newConfig));
     } catch (RatisRequestFailedException e) {
-      return failed(e);
+      throw e;
     }
-
-    return 
ConsensusGenericResponse.newBuilder().setSuccess(reply.isSuccess()).build();
   }
 
   /**
@@ -708,15 +706,11 @@ class RatisConsensus implements IConsensus {
             currentDirLength,
             filesCount);
 
-        final ConsensusGenericResponse consensusGenericResponse =
-            
triggerSnapshot(Utils.fromRaftGroupIdToConsensusGroupId(raftGroupId));
-        if (consensusGenericResponse.isSuccess()) {
+        try {
+          
triggerSnapshot(Utils.fromRaftGroupIdToConsensusGroupId(raftGroupId));
           logger.info("Raft group {} took snapshot successfully", raftGroupId);
-        } else {
-          logger.warn(
-              "Raft group {} failed to take snapshot due to",
-              raftGroupId,
-              consensusGenericResponse.getException());
+        } catch (ConsensusException e) {
+          logger.warn("Raft group {} failed to take snapshot due to", 
raftGroupId, e);
         }
       }
     }
diff --git 
a/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/ratis/RatisConsensusTest.java
 
b/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/ratis/RatisConsensusTest.java
index 8fcc5b88461..2a999445a80 100644
--- 
a/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/ratis/RatisConsensusTest.java
+++ 
b/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/ratis/RatisConsensusTest.java
@@ -201,7 +201,7 @@ public class RatisConsensusTest {
     servers.get(0).createLocalPeer(gid, peers.subList(0, 1));
 
     doConsensus(servers.get(0), gid, 10, 10);
-    Assert.assertTrue(servers.get(0).triggerSnapshot(gid).isSuccess());
+    servers.get(0).triggerSnapshot(gid);
 
     servers.get(1).createLocalPeer(gid, Collections.emptyList());
     servers.get(0).addRemotePeer(gid, peers.get(1));
diff --git 
a/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java
 
b/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java
index 6887f25ec9d..84d17212169 100644
--- 
a/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java
+++ 
b/iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/simple/SimpleConsensusTest.java
@@ -34,7 +34,6 @@ import org.apache.iotdb.consensus.common.DataSet;
 import org.apache.iotdb.consensus.common.Peer;
 import org.apache.iotdb.consensus.common.request.ByteBufferConsensusRequest;
 import org.apache.iotdb.consensus.common.request.IConsensusRequest;
-import org.apache.iotdb.consensus.common.response.ConsensusGenericResponse;
 import org.apache.iotdb.consensus.config.ConsensusConfig;
 import org.apache.iotdb.consensus.exception.*;
 
@@ -49,7 +48,6 @@ import java.util.Arrays;
 import java.util.Collections;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -259,8 +257,12 @@ public class SimpleConsensusTest {
 
   @Test
   public void triggerSnapshot() {
-    ConsensusGenericResponse response = 
consensusImpl.triggerSnapshot(dataRegionId);
-    assertFalse(response.isSuccess());
+    try {
+      consensusImpl.triggerSnapshot(dataRegionId);
+      assert false;
+    } catch (ConsensusException e) {
+      assert true;
+    }
   }
 
   @Test
diff --git 
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java
 
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java
index 4a6d2eb5044..f9d461ed727 100644
--- 
a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java
+++ 
b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/IoTDBShutdownHook.java
@@ -24,6 +24,7 @@ import org.apache.iotdb.commons.cluster.NodeStatus;
 import org.apache.iotdb.commons.concurrent.ThreadName;
 import org.apache.iotdb.commons.conf.CommonDescriptor;
 import org.apache.iotdb.consensus.ConsensusFactory;
+import org.apache.iotdb.consensus.exception.ConsensusException;
 import org.apache.iotdb.db.conf.IoTDBDescriptor;
 import org.apache.iotdb.db.consensus.DataRegionConsensusImpl;
 import org.apache.iotdb.db.consensus.SchemaRegionConsensusImpl;
@@ -85,7 +86,16 @@ public class IoTDBShutdownHook extends Thread {
       DataRegionConsensusImpl.getInstance()
           .getAllConsensusGroupIds()
           .parallelStream()
-          .forEach(id -> 
DataRegionConsensusImpl.getInstance().triggerSnapshot(id));
+          .forEach(
+              id -> {
+                try {
+                  DataRegionConsensusImpl.getInstance().triggerSnapshot(id);
+                } catch (ConsensusException e) {
+                  logger.warn(
+                      "Something wrong happened while calling consensus 
layer's triggerSnapshot API.",
+                      e);
+                }
+              });
     }
 
     // close consensusImpl

Reply via email to