From 8addee0bbd5cbd8ea7eee14c9324f6d2daaa0253 Mon Sep 17 00:00:00 2001
From: Itami Sho <42286868+MiniSho@users.noreply.github.com>
Date: Fri, 12 Aug 2022 14:27:03 +0800
Subject: [PATCH] [IOTDB-4102] Refactor config node removal process (#6977)

---
 .../confignode/manager/ConfigManager.java     |  2 +-
 .../iotdb/confignode/manager/NodeManager.java | 28 ++++------------
 .../procedure/env/ConfigNodeProcedureEnv.java | 33 +++++++++++++++++--
 3 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
index 4bb53f4f3c..a7a53a8f6a 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
@@ -781,7 +781,7 @@ public class ConfigManager implements IManager {
     TSStatus status = confirmLeader();
 
     if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
-      status = nodeManager.checkConfigNode(removeConfigNodePlan);
+      status = nodeManager.checkConfigNodeBeforeRemove(removeConfigNodePlan);
       if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
         procedureManager.removeConfigNode(removeConfigNodePlan);
       }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/NodeManager.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/NodeManager.java
index 31c4cadbd8..881f0b29d4 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/NodeManager.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/NodeManager.java
@@ -284,28 +284,12 @@ public class NodeManager {
     nodeInfo.addMetrics();
   }
 
-  public TSStatus removeConfigNodePeer(TConfigNodeLocation tConfigNodeLocation) {
-    removeConfigNodeLock.tryLock();
-    try {
-      // Execute removePeer
-      if (getConsensusManager().removeConfigNodePeer(tConfigNodeLocation)) {
-        configManager
-            .getLoadManager()
-            .removeNodeHeartbeatHandCache(tConfigNodeLocation.getConfigNodeId());
-        return getConsensusManager()
-            .write(new RemoveConfigNodePlan(tConfigNodeLocation))
-            .getStatus();
-      } else {
-        return new TSStatus(TSStatusCode.REMOVE_CONFIGNODE_FAILED.getStatusCode())
-            .setMessage(
-                "Remove ConfigNode failed because update ConsensusGroup peer information failed.");
-      }
-    } finally {
-      removeConfigNodeLock.unlock();
-    }
-  }
-
-  public TSStatus checkConfigNode(RemoveConfigNodePlan removeConfigNodePlan) {
+  /**
+   * Only leader use this interface, check the ConfigNode before remove it
+   *
+   * @param removeConfigNodePlan RemoveConfigNodePlan
+   */
+  public TSStatus checkConfigNodeBeforeRemove(RemoveConfigNodePlan removeConfigNodePlan) {
     removeConfigNodeLock.tryLock();
     try {
       // Check OnlineConfigNodes number
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java
index 0408fe45ea..7a5af9d818 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java
@@ -29,9 +29,11 @@ import org.apache.iotdb.confignode.client.sync.confignode.SyncConfigNodeClientPo
 import org.apache.iotdb.confignode.client.sync.datanode.SyncDataNodeClientPool;
 import org.apache.iotdb.confignode.consensus.request.write.DeleteStorageGroupPlan;
 import org.apache.iotdb.confignode.consensus.request.write.PreDeleteStorageGroupPlan;
+import org.apache.iotdb.confignode.consensus.request.write.RemoveConfigNodePlan;
 import org.apache.iotdb.confignode.exception.AddConsensusGroupException;
 import org.apache.iotdb.confignode.exception.AddPeerException;
 import org.apache.iotdb.confignode.manager.ConfigManager;
+import org.apache.iotdb.confignode.manager.ConsensusManager;
 import org.apache.iotdb.confignode.procedure.exception.ProcedureException;
 import org.apache.iotdb.confignode.procedure.scheduler.LockQueue;
 import org.apache.iotdb.confignode.procedure.scheduler.ProcedureScheduler;
@@ -68,6 +70,8 @@ public class ConfigNodeProcedureEnv {
 
   private static boolean invalidCacheResult = true;
 
+  private final ReentrantLock removeConfigNodeLock;
+
   public static void setSkipForTest(boolean skipForTest) {
     ConfigNodeProcedureEnv.skipForTest = skipForTest;
   }
@@ -80,6 +84,7 @@ public class ConfigNodeProcedureEnv {
     this.configManager = configManager;
     this.scheduler = scheduler;
     this.dataNodeRemoveHandler = new DataNodeRemoveHandler(configManager);
+    this.removeConfigNodeLock = new ReentrantLock();
   }
 
   public ConfigManager getConfigManager() {
@@ -193,9 +198,27 @@ public class ConfigNodeProcedureEnv {
    */
   public void removeConfigNodePeer(TConfigNodeLocation tConfigNodeLocation)
       throws ProcedureException {
-    TSStatus tsStatus = configManager.getNodeManager().removeConfigNodePeer(tConfigNodeLocation);
-    if (tsStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
-      throw new ProcedureException(tsStatus.getMessage());
+    removeConfigNodeLock.tryLock();
+    TSStatus tsStatus;
+    try {
+      // Execute removePeer
+      if (getConsensusManager().removeConfigNodePeer(tConfigNodeLocation)) {
+        configManager
+            .getLoadManager()
+            .removeNodeHeartbeatHandCache(tConfigNodeLocation.getConfigNodeId());
+        tsStatus =
+            getConsensusManager().write(new RemoveConfigNodePlan(tConfigNodeLocation)).getStatus();
+      } else {
+        tsStatus =
+            new TSStatus(TSStatusCode.REMOVE_CONFIGNODE_FAILED.getStatusCode())
+                .setMessage(
+                    "Remove ConfigNode failed because update ConsensusGroup peer information failed.");
+      }
+      if (tsStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+        throw new ProcedureException(tsStatus.getMessage());
+      }
+    } finally {
+      removeConfigNodeLock.unlock();
     }
   }
 
@@ -287,4 +310,8 @@ public class ConfigNodeProcedureEnv {
   public DataNodeRemoveHandler getDataNodeRemoveHandler() {
     return dataNodeRemoveHandler;
   }
+
+  private ConsensusManager getConsensusManager() {
+    return configManager.getConsensusManager();
+  }
 }
-- 
GitLab