You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by ca...@apache.org on 2022/10/13 10:02:47 UTC

[iotdb] branch beyyes/remove_datanode_improvement updated: fix error about remove-confignode.sh when execute it in non-running node

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

caogaofei pushed a commit to branch beyyes/remove_datanode_improvement
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/beyyes/remove_datanode_improvement by this push:
     new 0f3387048f fix error about remove-confignode.sh when execute it in non-running node
0f3387048f is described below

commit 0f3387048fc93b5758d0f40f4cb18241befa8c88
Author: Beyyes <cg...@foxmail.com>
AuthorDate: Thu Oct 13 18:02:28 2022 +0800

    fix error about remove-confignode.sh when execute it in non-running node
---
 .../confignode/client/ConfigNodeRequestType.java   |  2 +-
 .../client/sync/SyncConfigNodeClientPool.java      |  4 +--
 .../iotdb/confignode/conf/ConfigNodeConstant.java  |  3 +-
 .../confignode/conf/ConfigNodeRemoveCheck.java     | 16 +++++++++--
 .../iotdb/confignode/manager/ProcedureManager.java |  2 +-
 .../procedure/env/ConfigNodeProcedureEnv.java      | 10 +++----
 .../impl/node/AddConfigNodeProcedure.java          | 10 +++----
 .../impl/node/RemoveConfigNodeProcedure.java       | 32 +++++++++++-----------
 .../procedure/state/AddConfigNodeState.java        |  2 +-
 .../procedure/state/RemoveConfigNodeState.java     |  2 +-
 .../thrift/ConfigNodeRPCServiceProcessor.java      | 18 +++++++++++-
 .../apache/iotdb/db/client/ConfigNodeClient.java   |  2 +-
 .../db/service/DataNodeServerCommandLine.java      |  5 ++--
 .../src/main/thrift/confignode.thrift              |  6 ++--
 14 files changed, 72 insertions(+), 42 deletions(-)

diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/client/ConfigNodeRequestType.java b/confignode/src/main/java/org/apache/iotdb/confignode/client/ConfigNodeRequestType.java
index cff85aff72..d9ee56ae25 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/client/ConfigNodeRequestType.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/client/ConfigNodeRequestType.java
@@ -24,6 +24,6 @@ public enum ConfigNodeRequestType {
   NOTIFY_REGISTER_SUCCESS,
   REGISTER_CONFIG_NODE,
   REMOVE_CONFIG_NODE,
-  REMOVE_CONSENSUS_GROUP,
+  DELETE_CONFIG_NODE_PEER,
   STOP_CONFIG_NODE;
 }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/client/sync/SyncConfigNodeClientPool.java b/confignode/src/main/java/org/apache/iotdb/confignode/client/sync/SyncConfigNodeClientPool.java
index 557a00f55f..30d41a1d7b 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/client/sync/SyncConfigNodeClientPool.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/client/sync/SyncConfigNodeClientPool.java
@@ -81,8 +81,8 @@ public class SyncConfigNodeClientPool {
             return null;
           case REMOVE_CONFIG_NODE:
             return removeConfigNode((TConfigNodeLocation) req, client);
-          case REMOVE_CONSENSUS_GROUP:
-            return client.removeConsensusGroup((TConfigNodeLocation) req);
+          case DELETE_CONFIG_NODE_PEER:
+            return client.deleteConfigNodePeer((TConfigNodeLocation) req);
           case STOP_CONFIG_NODE:
             // Only use stopConfigNode when the ConfigNode is removed.
             return client.stopConfigNode((TConfigNodeLocation) req);
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConstant.java b/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConstant.java
index 3a9a9013f5..940de32e5a 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConstant.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeConstant.java
@@ -45,7 +45,8 @@ public class ConfigNodeConstant {
   public static final String METRIC_STATUS_ONLINE = "Online";
   public static final String METRIC_STATUS_UNKNOWN = "Unknown";
 
-  public static final String REMOVE_CONFIGNODE_USAGE = "Executed failed, check usage: <Node-id>/<internal_address>:<internal_port>";
+  public static final String REMOVE_CONFIGNODE_USAGE =
+      "Executed failed, check usage: <Node-id>/<internal_address>:<internal_port>";
 
   public static final String REMOVE_DATANODE_PROCESS = "[REMOVE_DATANODE_PROCESS]";
 
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeRemoveCheck.java b/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeRemoveCheck.java
index 62be003b2c..660595b456 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeRemoveCheck.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeRemoveCheck.java
@@ -34,8 +34,10 @@ import org.slf4j.LoggerFactory;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
 import static org.apache.commons.lang3.StringUtils.isNumeric;
 
@@ -92,7 +94,12 @@ public class ConfigNodeRemoveCheck {
   public void removeConfigNode(TConfigNodeLocation removedNode)
       throws BadNodeUrlException, IOException {
     TSStatus status = new TSStatus();
-    for (TConfigNodeLocation configNodeLocation : getConfigNodeList()) {
+    // Using leader ConfigNode id firstly
+    List<TConfigNodeLocation> configNodeList =
+        getConfigNodeList().stream()
+            .sorted(Comparator.comparing(TConfigNodeLocation::getConfigNodeId))
+            .collect(Collectors.toList());
+    for (TConfigNodeLocation configNodeLocation : configNodeList) {
       status =
           (TSStatus)
               SyncConfigNodeClientPool.getInstance()
@@ -103,10 +110,15 @@ public class ConfigNodeRemoveCheck {
       if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
         break;
       }
+
+      if (status.getCode() == TSStatusCode.REMOVE_CONFIGNODE_FAILED.getStatusCode()) {
+        break;
+      }
     }
+
     if (status.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
       LOGGER.error(status.getMessage());
-      throw new IOException("Remove ConfigNode failed:");
+      throw new IOException("Remove ConfigNode failed: " + status.getMessage());
     }
   }
 
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java b/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java
index ef583c341e..be3d5a01f9 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/manager/ProcedureManager.java
@@ -199,7 +199,7 @@ public class ProcedureManager {
     RemoveConfigNodeProcedure removeConfigNodeProcedure =
         new RemoveConfigNodeProcedure(removeConfigNodePlan.getConfigNodeLocation());
     this.executor.submitProcedure(removeConfigNodeProcedure);
-    LOGGER.info("Submit to remove ConfigNode, {}", removeConfigNodePlan);
+    LOGGER.info("Submit RemoveConfigNodeProcedure successfully: {}", removeConfigNodePlan);
   }
 
   /** Generate RemoveDataNodeProcedures, and serially execute all the RemoveDataNodeProcedure */
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 44d6549f96..2bbab37673 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
@@ -250,18 +250,18 @@ public class ConfigNodeProcedureEnv {
   /**
    * Remove Consensus Group in removed node
    *
-   * @param tConfigNodeLocation config node location
+   * @param removedConfigNode config node location
    * @throws ProcedureException if failed status
    */
-  public void removeConsensusGroup(TConfigNodeLocation tConfigNodeLocation)
+  public void deleteConfigNodePeer(TConfigNodeLocation removedConfigNode)
       throws ProcedureException {
     TSStatus tsStatus =
         (TSStatus)
             SyncConfigNodeClientPool.getInstance()
                 .sendSyncRequestToConfigNodeWithRetry(
-                    tConfigNodeLocation.getInternalEndPoint(),
-                    tConfigNodeLocation,
-                    ConfigNodeRequestType.REMOVE_CONSENSUS_GROUP);
+                    removedConfigNode.getInternalEndPoint(),
+                    removedConfigNode,
+                    ConfigNodeRequestType.DELETE_CONFIG_NODE_PEER);
     if (tsStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
       throw new ProcedureException(tsStatus.getMessage());
     }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/AddConfigNodeProcedure.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/AddConfigNodeProcedure.java
index fe1dd6fc67..7c3e0cd582 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/AddConfigNodeProcedure.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/AddConfigNodeProcedure.java
@@ -58,9 +58,9 @@ public class AddConfigNodeProcedure extends AbstractNodeProcedure<AddConfigNodeS
     try {
       switch (state) {
         case ADD_CONFIG_NODE_PREPARE:
-          setNextState(AddConfigNodeState.ADD_CONSENSUS_GROUP);
+          setNextState(AddConfigNodeState.CREATE_PEER);
           break;
-        case ADD_CONSENSUS_GROUP:
+        case CREATE_PEER:
           env.addConsensusGroup(tConfigNodeLocation);
           setNextState(AddConfigNodeState.ADD_PEER);
           LOG.info("Add consensus group {}", tConfigNodeLocation);
@@ -97,8 +97,8 @@ public class AddConfigNodeProcedure extends AbstractNodeProcedure<AddConfigNodeS
   protected void rollbackState(ConfigNodeProcedureEnv env, AddConfigNodeState state)
       throws ProcedureException {
     switch (state) {
-      case ADD_CONSENSUS_GROUP:
-        env.removeConsensusGroup(tConfigNodeLocation);
+      case CREATE_PEER:
+        env.deleteConfigNodePeer(tConfigNodeLocation);
         LOG.info("Rollback add consensus group:{}", tConfigNodeLocation);
         break;
       case ADD_PEER:
@@ -111,7 +111,7 @@ public class AddConfigNodeProcedure extends AbstractNodeProcedure<AddConfigNodeS
   @Override
   protected boolean isRollbackSupported(AddConfigNodeState state) {
     switch (state) {
-      case ADD_CONSENSUS_GROUP:
+      case CREATE_PEER:
       case ADD_PEER:
         return true;
     }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/RemoveConfigNodeProcedure.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/RemoveConfigNodeProcedure.java
index 5e560ce799..22526504e2 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/RemoveConfigNodeProcedure.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/impl/node/RemoveConfigNodeProcedure.java
@@ -39,38 +39,38 @@ public class RemoveConfigNodeProcedure extends AbstractNodeProcedure<RemoveConfi
   private static final Logger LOG = LoggerFactory.getLogger(RemoveConfigNodeProcedure.class);
   private static final int retryThreshold = 5;
 
-  private TConfigNodeLocation tConfigNodeLocation;
+  private TConfigNodeLocation removedConfigNode;
 
   public RemoveConfigNodeProcedure() {
     super();
   }
 
-  public RemoveConfigNodeProcedure(TConfigNodeLocation tConfigNodeLocation) {
+  public RemoveConfigNodeProcedure(TConfigNodeLocation removedConfigNode) {
     super();
-    this.tConfigNodeLocation = tConfigNodeLocation;
+    this.removedConfigNode = removedConfigNode;
   }
 
   @Override
   protected Flow executeFromState(ConfigNodeProcedureEnv env, RemoveConfigNodeState state) {
-    if (tConfigNodeLocation == null) {
+    if (removedConfigNode == null) {
       return Flow.NO_MORE_STATE;
     }
     try {
       switch (state) {
         case REMOVE_PEER:
-          env.removeConfigNodePeer(tConfigNodeLocation);
-          setNextState(RemoveConfigNodeState.REMOVE_CONSENSUS_GROUP);
-          LOG.info("Remove peer {}", tConfigNodeLocation);
+          env.removeConfigNodePeer(removedConfigNode);
+          setNextState(RemoveConfigNodeState.DELETE_PEER);
+          LOG.info("Remove peer for ConfigNode: {}", removedConfigNode);
           break;
-        case REMOVE_CONSENSUS_GROUP:
-          env.removeConsensusGroup(tConfigNodeLocation);
+        case DELETE_PEER:
+          env.deleteConfigNodePeer(removedConfigNode);
           setNextState(RemoveConfigNodeState.STOP_CONFIG_NODE);
-          LOG.info("Remove Consensus Group {}", tConfigNodeLocation);
+          LOG.info("Delete peer for ConfigNode: {}", removedConfigNode);
           break;
         case STOP_CONFIG_NODE:
           env.broadCastTheLatestConfigNodeGroup();
-          env.stopConfigNode(tConfigNodeLocation);
-          LOG.info("Stop Config Node {}", tConfigNodeLocation);
+          env.stopConfigNode(removedConfigNode);
+          LOG.info("Stop ConfigNode: {}", removedConfigNode);
           return Flow.NO_MORE_STATE;
       }
     } catch (Exception e) {
@@ -79,7 +79,7 @@ public class RemoveConfigNodeProcedure extends AbstractNodeProcedure<RemoveConfi
       } else {
         LOG.error(
             "Retrievable error trying to remove config node {}, state {}",
-            tConfigNodeLocation,
+            removedConfigNode,
             state,
             e);
         if (getCycles() > retryThreshold) {
@@ -118,14 +118,14 @@ public class RemoveConfigNodeProcedure extends AbstractNodeProcedure<RemoveConfi
   public void serialize(DataOutputStream stream) throws IOException {
     stream.writeInt(ProcedureFactory.ProcedureType.REMOVE_CONFIG_NODE_PROCEDURE.ordinal());
     super.serialize(stream);
-    ThriftConfigNodeSerDeUtils.serializeTConfigNodeLocation(tConfigNodeLocation, stream);
+    ThriftConfigNodeSerDeUtils.serializeTConfigNodeLocation(removedConfigNode, stream);
   }
 
   @Override
   public void deserialize(ByteBuffer byteBuffer) {
     super.deserialize(byteBuffer);
     try {
-      tConfigNodeLocation = ThriftConfigNodeSerDeUtils.deserializeTConfigNodeLocation(byteBuffer);
+      removedConfigNode = ThriftConfigNodeSerDeUtils.deserializeTConfigNodeLocation(byteBuffer);
     } catch (ThriftSerDeException e) {
       LOG.error("Error in deserialize RemoveConfigNodeProcedure", e);
     }
@@ -137,7 +137,7 @@ public class RemoveConfigNodeProcedure extends AbstractNodeProcedure<RemoveConfi
       RemoveConfigNodeProcedure thatProc = (RemoveConfigNodeProcedure) that;
       return thatProc.getProcId() == this.getProcId()
           && thatProc.getState() == this.getState()
-          && thatProc.tConfigNodeLocation.equals(this.tConfigNodeLocation);
+          && thatProc.removedConfigNode.equals(this.removedConfigNode);
     }
     return false;
   }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/AddConfigNodeState.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/AddConfigNodeState.java
index 6dbb2ad12e..a7f1912609 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/AddConfigNodeState.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/AddConfigNodeState.java
@@ -21,7 +21,7 @@ package org.apache.iotdb.confignode.procedure.state;
 
 public enum AddConfigNodeState {
   ADD_CONFIG_NODE_PREPARE,
-  ADD_CONSENSUS_GROUP,
+  CREATE_PEER,
   ADD_PEER,
   REGISTER_SUCCESS
 }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/RemoveConfigNodeState.java b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/RemoveConfigNodeState.java
index dc7acbda82..864ee97e0c 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/RemoveConfigNodeState.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/procedure/state/RemoveConfigNodeState.java
@@ -21,6 +21,6 @@ package org.apache.iotdb.confignode.procedure.state;
 
 public enum RemoveConfigNodeState {
   REMOVE_PEER,
-  REMOVE_CONSENSUS_GROUP,
+  DELETE_PEER,
   STOP_CONFIG_NODE
 }
diff --git a/confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java b/confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java
index ace66de37e..33303f057a 100644
--- a/confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java
+++ b/confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java
@@ -442,7 +442,7 @@ public class ConfigNodeRPCServiceProcessor implements IConfigNodeRPCService.Ifac
   }
 
   @Override
-  public TSStatus removeConsensusGroup(TConfigNodeLocation configNodeLocation) {
+  public TSStatus deleteConfigNodePeer(TConfigNodeLocation configNodeLocation) {
     if (!configManager.getNodeManager().getRegisteredConfigNodes().contains(configNodeLocation)) {
       return new TSStatus(TSStatusCode.REMOVE_CONFIGNODE_FAILED.getStatusCode())
           .setMessage(
@@ -457,6 +457,22 @@ public class ConfigNodeRPCServiceProcessor implements IConfigNodeRPCService.Ifac
           .setMessage(
               "remove ConsensusGroup failed because internal failure. See other logs for more details");
     }
+
+    //    List<TConfigNodeLocation> registeredConfigNodes =
+    //            configManager.getNodeManager().getRegisteredConfigNodes();
+    //    registeredConfigNodes.remove(configNodeLocation);
+    //    try {
+    //      // The removed ConfigNode also need to update the confignode-system.properties file
+    //      // Because user may execute remove-confignode.sh in this node
+    //      SystemPropertiesUtils.storeConfigNodeList(new ArrayList<>(registeredConfigNodes));
+    //      LOGGER.info(
+    //              "Updated ConfigNode information files after deleteConfigNodePeer. {}.",
+    //              registeredConfigNodes);
+    //    } catch (IOException e) {
+    //      LOGGER.error("Current ConfigNode to be removed can't update ConfigNode information.",
+    // e);
+    //    }
+
     return new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode())
         .setMessage("remove ConsensusGroup success.");
   }
diff --git a/server/src/main/java/org/apache/iotdb/db/client/ConfigNodeClient.java b/server/src/main/java/org/apache/iotdb/db/client/ConfigNodeClient.java
index 9aeef0be07..65cb925b72 100644
--- a/server/src/main/java/org/apache/iotdb/db/client/ConfigNodeClient.java
+++ b/server/src/main/java/org/apache/iotdb/db/client/ConfigNodeClient.java
@@ -678,7 +678,7 @@ public class ConfigNodeClient
   }
 
   @Override
-  public TSStatus removeConsensusGroup(TConfigNodeLocation configNodeLocation) throws TException {
+  public TSStatus deleteConfigNodePeer(TConfigNodeLocation configNodeLocation) throws TException {
     throw new TException("DataNode to ConfigNode client doesn't support removeConsensusGroup.");
   }
 
diff --git a/server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java b/server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java
index 39b4e875d0..e952779f39 100644
--- a/server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java
+++ b/server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java
@@ -92,7 +92,7 @@ public class DataNodeServerCommandLine extends ServerCommandLine {
     if (MODE_START.equals(mode)) {
       dataNode.doAddNode();
     } else if (MODE_REMOVE.equals(mode)) {
-      doRemoveNode(args);
+      doRemoveDataNode(args);
     } else {
       LOGGER.error("Unrecognized mode {}", mode);
     }
@@ -104,7 +104,8 @@ public class DataNodeServerCommandLine extends ServerCommandLine {
    *
    * @param args id or ip:rpc_port for removed datanode
    */
-  private void doRemoveNode(String[] args) throws BadNodeUrlException, TException, IoTDBException {
+  private void doRemoveDataNode(String[] args)
+      throws BadNodeUrlException, TException, IoTDBException {
 
     if (args.length != 2) {
       LOGGER.info("Usage: <node-id>/<ip>:<rpc-port>");
diff --git a/thrift-confignode/src/main/thrift/confignode.thrift b/thrift-confignode/src/main/thrift/confignode.thrift
index 1b9581a710..5cec6f492d 100644
--- a/thrift-confignode/src/main/thrift/confignode.thrift
+++ b/thrift-confignode/src/main/thrift/confignode.thrift
@@ -699,13 +699,13 @@ service IConfigNodeRPCService {
   common.TSStatus removeConfigNode(common.TConfigNodeLocation configNodeLocation)
 
   /**
-   * Let the specific ConfigNode remove the ConsensusGroup
+   * Let the specific ConfigNode delete the peer
    *
-   * @return SUCCESS_STATUS if remove ConsensusGroup successfully
+   * @return SUCCESS_STATUS if delete peer  successfully
    *         REMOVE_CONFIGNODE_FAILED if the specific ConfigNode doesn't exist in the current cluster
    *                                  or Ratis internal failure
    */
-  common.TSStatus removeConsensusGroup(common.TConfigNodeLocation configNodeLocation)
+  common.TSStatus deleteConfigNodePeer(common.TConfigNodeLocation configNodeLocation)
 
   /** Stop the specific ConfigNode */
   common.TSStatus stopConfigNode(common.TConfigNodeLocation configNodeLocation)