[GitHub] [iotdb] Beyyes commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes

[GitBox <gi...@apache.org>]

Beyyes commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r979802334


##########
server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java:
##########
@@ -104,52 +100,22 @@ protected int run(String[] args) throws Exception {
   /**
    * remove datanodes from cluster
    *
-   * @param args IPs for removed datanodes, split with ','
+   * @param args id or ip:rpc_port for removed datanode
    */
-  private void doRemoveNode(String[] args) throws Exception {
-    // throw all exception to ServerCommandLine, it used System.exit
-    removePrepare(args);
-    removeNodesFromCluster(args);
-    removeTail();
-  }
+  private void doRemoveNode(String[] args) throws BadNodeUrlException, TException, IoTDBException {
 
-  private void removePrepare(String[] args) throws BadNodeUrlException, TException {
-    ConfigNodeInfo.getInstance()
-        .updateConfigNodeList(IoTDBDescriptor.getInstance().getConfig().getTargetConfigNodeList());
-    try (ConfigNodeClient configNodeClient = new ConfigNodeClient()) {
-      TDataNodeConfigurationResp resp = configNodeClient.getDataNodeConfiguration(-1);
-      // 1. online Data Node size - removed Data Node size < replication,NOT ALLOW remove
-      //   But replication size is set in Config Node's configuration, so check it in remote Config
-      // Node
-
-      // 2. removed Data Node IP not contained in below map, CAN NOT remove.
-      Map<Integer, TDataNodeConfiguration> nodeIdToNodeConfiguration =
-          resp.getDataNodeConfigurationMap();
-      List<TEndPoint> endPoints = NodeUrlUtils.parseTEndPointUrls(args[1]);
-      List<String> removedDataNodeIps =
-          endPoints.stream().map(TEndPoint::getIp).collect(Collectors.toList());
-
-      List<String> onlineDataNodeIps =
-          nodeIdToNodeConfiguration.values().stream()
-              .map(TDataNodeConfiguration::getLocation)
-              .map(TDataNodeLocation::getInternalEndPoint)
-              .map(TEndPoint::getIp)
-              .collect(Collectors.toList());
-      IoTDBStopCheck.getInstance().checkIpInCluster(removedDataNodeIps, onlineDataNodeIps);
+    logger.info("Start to remove DataNode from cluster");
+    if (args.length != 2) {
+      logger.info("Usage: <node-id>/<ip>:<rpc-port>");

Review Comment:
   Also update the user doc



##########
confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeRemoveCheck.java:
##########
@@ -50,19 +51,34 @@ public ConfigNodeRemoveCheck() {
     systemProperties = new Properties();
   }
 
-  public TConfigNodeLocation removeCheck(TEndPoint endPoint) {
+  public TConfigNodeLocation removeCheck(String args) {
     TConfigNodeLocation nodeLocation = new TConfigNodeLocation();
     if (!systemPropertiesFile.exists()) {
       LOGGER.error("The system properties file is not exists. IoTDB-ConfigNode is shutdown.");
       return nodeLocation;
     }
     try (FileInputStream inputStream = new FileInputStream(systemPropertiesFile)) {
       systemProperties.load(inputStream);
-      nodeLocation =
-          getConfigNodeList().stream()
-              .filter(e -> e.getInternalEndPoint().equals(endPoint))
-              .findFirst()
-              .orElse(null);
+      try {
+        int id = Integer.parseInt(args);
+        nodeLocation =
+            getConfigNodeList().stream()
+                .filter(e -> e.getConfigNodeId() == id)
+                .findFirst()
+                .orElse(null);
+      } catch (NumberFormatException e1) {

Review Comment:
   Using `isNumeric` method to judge but not using `NumberFormatException `



##########
server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java:
##########
@@ -159,51 +125,66 @@ private void removeNodesFromCluster(String[] args)
             removeResp.getStatus().toString(), removeResp.getStatus().getCode());
       }
       logger.info(
-          "Submit remove datanode request successfully, "
+          "Submit remove-datanode request successfully, "
               + "more details are shown in the logs of confignode-leader and removed-datanode, "
-              + "and after the process of remove-datanode is over, "
+              + "and after the process of removing datanode is over, "
               + "you are supposed to delete directory and data of the removed-datanode manually");
     }
   }
 
   /**
-   * fetch all datanode info from ConfigNode, then compare with input 'ips'
+   * fetch all datanode info from ConfigNode, then compare with input 'args'
    *
-   * @param endPorts data node ip:port, split with ','
+   * @param args datanode id or ip:rpc_port
    * @return TDataNodeLocation list
    */
-  private List<TDataNodeLocation> buildDataNodeLocations(String endPorts)
-      throws BadNodeUrlException {
+  private List<TDataNodeLocation> buildDataNodeLocations(String args) {
     List<TDataNodeLocation> dataNodeLocations = new ArrayList<>();
-    if (endPorts == null || endPorts.trim().isEmpty()) {
+    if (args == null || args.trim().isEmpty()) {
       return dataNodeLocations;
     }
 
-    List<TEndPoint> endPoints = NodeUrlUtils.parseTEndPointUrls(endPorts);
-
-    try (ConfigNodeClient client = new ConfigNodeClient()) {
-      dataNodeLocations =
-          client.getDataNodeConfiguration(-1).getDataNodeConfigurationMap().values().stream()
-              .map(TDataNodeConfiguration::getLocation)
-              .filter(location -> endPoints.contains(location.getClientRpcEndPoint()))
-              .collect(Collectors.toList());
-    } catch (TException e) {
-      logger.error("Get data node locations failed", e);
+    // Now support only single datanode deletion
+    if (args.split(",").length > 1) {
+      logger.info("Incorrect input format, usage: <id>/<ip>:<rpc-port>");
+      return dataNodeLocations;
     }
 
-    if (endPoints.size() != dataNodeLocations.size()) {
-      logger.error(
-          "Build DataNode locations error, "
-              + "because number of input DataNode({}) not equals the number of fetched DataNodeLocations({}), "
-              + "please check your IP:RPC_PORT inputs",
-          endPoints.size(),
-          dataNodeLocations.size());
-      dataNodeLocations.clear();
-      return dataNodeLocations;
+    // Below supports multiple datanode deletion, split by ',', and is reserved for extension
+    try {
+      List<TEndPoint> endPoints = NodeUrlUtils.parseTEndPointUrls(args);
+      try (ConfigNodeClient client = new ConfigNodeClient()) {
+        dataNodeLocations =
+            client.getDataNodeConfiguration(-1).getDataNodeConfigurationMap().values().stream()
+                .map(TDataNodeConfiguration::getLocation)
+                .filter(location -> endPoints.contains(location.getClientRpcEndPoint()))
+                .collect(Collectors.toList());
+      } catch (TException e) {
+        logger.error("get data node locations failed", e);
+      }
+    } catch (BadNodeUrlException e) {
+      try (ConfigNodeClient client = new ConfigNodeClient()) {
+        for (String id : args.split(",")) {

Review Comment:
   Now we only support removing one datanode id, if it's necessary to using `args.split(",")`



-- 
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: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org