You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2022/09/26 02:36:37 UTC

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

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


##########
server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java:
##########
@@ -104,52 +98,27 @@ protected int run(String[] args) throws Exception {
   /**
    * remove datanodes from cluster
    *
-   * @param args IPs for removed datanodes, split with ','
+   * @param args ids for removed datanodes, split with ','
    */
-  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-ids>, split with ':'");

Review Comment:
   Don't leave commented codes~



##########
server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java:
##########
@@ -104,52 +98,27 @@ protected int run(String[] args) throws Exception {
   /**
    * remove datanodes from cluster
    *
-   * @param args IPs for removed datanodes, split with ','
+   * @param args ids for removed datanodes, split with ','
    */
-  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-ids>, split with ':'");
+      logger.info("Usage: <node-id>");
+      return;
     }
-  }
 
-  private void removeNodesFromCluster(String[] args)
-      throws BadNodeUrlException, TException, IoTDBException {
-    logger.info("start to remove DataNode from cluster");
+    ConfigNodeInfo.getInstance().loadConfigNodeList();
     List<TDataNodeLocation> dataNodeLocations = buildDataNodeLocations(args[1]);
     if (dataNodeLocations.isEmpty()) {
-      throw new BadNodeUrlException("build DataNode location is empty");
+      throw new BadNodeUrlException("No DataNode to remove");
     }
-    logger.info(
-        "there has data nodes location will be removed. size is: {}, detail: {}",
-        dataNodeLocations.size(),
-        dataNodeLocations);
+    //    logger.info(
+    //        "There are DataNodes to be removed. size is: {}, detail: {}",
+    //        dataNodeLocations.size(),
+    //        dataNodeLocations);

Review Comment:
   Don't leave commented codes~



##########
server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java:
##########
@@ -159,51 +128,52 @@ 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 'ids'
    *
-   * @param endPorts data node ip:port, split with ','
+   * @param ids datanode id, split with ':'
    * @return TDataNodeLocation list
    */
-  private List<TDataNodeLocation> buildDataNodeLocations(String endPorts)
-      throws BadNodeUrlException {
+  private List<TDataNodeLocation> buildDataNodeLocations(String ids) {
     List<TDataNodeLocation> dataNodeLocations = new ArrayList<>();
-    if (endPorts == null || endPorts.trim().isEmpty()) {
+    if (ids == null || ids.trim().isEmpty()) {
       return dataNodeLocations;
     }
 
-    List<TEndPoint> endPoints = NodeUrlUtils.parseTEndPointUrls(endPorts);
+    // Currently support only single id, delete this line when supports multi ones
+    if (ids.split(":").length > 1) {
+      logger.info("Incorrect id format, usage: <node-id>");

Review Comment:
   ```suggestion
         logger.info("Incorrect id format, usage: -r <node-id>");
   ```



##########
confignode/src/main/java/org/apache/iotdb/confignode/service/ConfigNodeCommandLine.java:
##########
@@ -97,18 +95,17 @@ protected int run(String[] args) {
 
   private void doRemoveNode(String[] args) throws IOException {
     LOGGER.info("Starting to remove {}...", ConfigNodeConstant.GLOBAL_NAME);
-    if (args.length != 3) {
-      LOGGER.info("Usage: -r <internal_address>:<internal_port>");
+    if (args.length != 2) {
+      LOGGER.info("Usage: <Node-id>");

Review Comment:
   ```suggestion
         LOGGER.info("Usage: -r <Node-id>");
   ```



##########
server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java:
##########
@@ -104,52 +98,27 @@ protected int run(String[] args) throws Exception {
   /**
    * remove datanodes from cluster
    *
-   * @param args IPs for removed datanodes, split with ','
+   * @param args ids for removed datanodes, split with ','
    */
-  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-ids>, split with ':'");
+      logger.info("Usage: <node-id>");

Review Comment:
   ```suggestion
         logger.info("Usage: -r <node-id>");
   ```



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