liyuheng55555 commented on code in PR #12914: URL: https://github.com/apache/iotdb/pull/12914#discussion_r1679104430
########## iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java: ########## @@ -135,57 +165,79 @@ private void doRemoveDataNode(String[] args) */ private List<TDataNodeLocation> buildDataNodeLocations(String args) { List<TDataNodeLocation> dataNodeLocations = new ArrayList<>(); - if (args == null || args.trim().isEmpty()) { - return dataNodeLocations; - } // Now support only single datanode deletion if (args.split(",").length > 1) { - LOGGER.info("Incorrect input format, usage: <id>/<ip>:<rpc-port>"); - return dataNodeLocations; + throw new IllegalArgumentException("Currently only removing single nodes is supported."); } // Below supports multiple datanode deletion, split by ',', and is reserved for extension - try { - List<TEndPoint> endPoints = NodeUrlUtils.parseTEndPointUrls(args); - try (ConfigNodeClient client = - ConfigNodeClientManager.getInstance().borrowClient(ConfigNodeInfo.CONFIG_REGION_ID)) { - dataNodeLocations = - client.getDataNodeConfiguration(-1).getDataNodeConfigurationMap().values().stream() - .map(TDataNodeConfiguration::getLocation) - .filter(location -> endPoints.contains(location.getClientRpcEndPoint())) - .collect(Collectors.toList()); - } catch (TException | ClientManagerException e) { - LOGGER.error("Get data node locations failed", e); + List<NodeCoordinate> endPoints = parseCoordinates(args); Review Comment: Rename this var ? (endpoint has some other meaning in IoTDB) ########## iotdb-core/datanode/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java: ########## @@ -135,57 +165,79 @@ private void doRemoveDataNode(String[] args) */ private List<TDataNodeLocation> buildDataNodeLocations(String args) { List<TDataNodeLocation> dataNodeLocations = new ArrayList<>(); - if (args == null || args.trim().isEmpty()) { - return dataNodeLocations; - } // Now support only single datanode deletion if (args.split(",").length > 1) { - LOGGER.info("Incorrect input format, usage: <id>/<ip>:<rpc-port>"); - return dataNodeLocations; + throw new IllegalArgumentException("Currently only removing single nodes is supported."); } // Below supports multiple datanode deletion, split by ',', and is reserved for extension - try { - List<TEndPoint> endPoints = NodeUrlUtils.parseTEndPointUrls(args); - try (ConfigNodeClient client = - ConfigNodeClientManager.getInstance().borrowClient(ConfigNodeInfo.CONFIG_REGION_ID)) { - dataNodeLocations = - client.getDataNodeConfiguration(-1).getDataNodeConfigurationMap().values().stream() - .map(TDataNodeConfiguration::getLocation) - .filter(location -> endPoints.contains(location.getClientRpcEndPoint())) - .collect(Collectors.toList()); - } catch (TException | ClientManagerException e) { - LOGGER.error("Get data node locations failed", e); + List<NodeCoordinate> endPoints = parseCoordinates(args); + try (ConfigNodeClient client = + configNodeClientManager.borrowClient(ConfigNodeInfo.CONFIG_REGION_ID)) { + dataNodeLocations = + client.getDataNodeConfiguration(-1).getDataNodeConfigurationMap().values().stream() + .map(TDataNodeConfiguration::getLocation) + .filter( + location -> + endPoints.stream() + .anyMatch(nodeCoordinate -> nodeCoordinate.matches(location))) + .collect(Collectors.toList()); + } catch (TException | ClientManagerException e) { + LOGGER.error("Get data node locations failed", e); + } + + return dataNodeLocations; + } + + protected List<NodeCoordinate> parseCoordinates(String coordinatesString) { + // Multiple coordinates are separated by "," + String[] coordinateStrings = coordinatesString.split(","); + List<NodeCoordinate> coordinates = new ArrayList<>(coordinateStrings.length); + for (String coordinate : coordinateStrings) { + // If the string contains a ":" then this is an IP+Port configuration + if (coordinate.contains(":")) { + coordinates.add(new NodeCoordinateIP(UrlUtils.parseTEndPointIpv4AndIpv6Url(coordinate))); } - } catch (BadNodeUrlException e) { - try (ConfigNodeClient client = - ConfigNodeClientManager.getInstance().borrowClient(ConfigNodeInfo.CONFIG_REGION_ID)) { - for (String id : args.split(",")) { - if (!isNumeric(id)) { - LOGGER.warn("Incorrect id format {}, skipped...", id); - continue; - } - List<TDataNodeLocation> nodeLocationResult = - client - .getDataNodeConfiguration(Integer.parseInt(id)) - .getDataNodeConfigurationMap() - .values() - .stream() - .map(TDataNodeConfiguration::getLocation) - .collect(Collectors.toList()); - if (nodeLocationResult.isEmpty()) { - LOGGER.warn("DataNode {} is not in cluster, skipped...", id); - continue; - } - if (!dataNodeLocations.contains(nodeLocationResult.get(0))) { - dataNodeLocations.add(nodeLocationResult.get(0)); - } - } - } catch (TException | ClientManagerException e1) { - LOGGER.error("Get data node locations failed", e); + // In the other case, we expect it to be a numeric value referring to the node-id + else if (NumberUtils.isCreatable(coordinate)) { + coordinates.add(new NodeCoordinateNodeId(Integer.parseInt(coordinate))); } } - return dataNodeLocations; + return coordinates; + } Review Comment: 1. What will UrlUtils.parseTEndPointIpv4AndIpv6Url do if coordinate contains ":" but has some other format problem ? 2. If neither of these two conditions can be met, an error should be reported here. ########## iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceProcessor.java: ########## @@ -244,6 +246,24 @@ public TGetClusterIdResp getClusterId() { @Override public TDataNodeRegisterResp registerDataNode(TDataNodeRegisterReq req) { + // If the remote server is configured to bind to all ip addresses (0.0.0.0), + // update the request to use the remote address we got this request from. + if ((req.dataNodeConfiguration.getLocation() != null) + && req.dataNodeConfiguration.getLocation().isSetClientRpcEndPoint() + && req.dataNodeConfiguration.getLocation().getClientRpcEndPoint().isSetIp() + && "0.0.0.0" Review Comment: Agree -- 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-unsubscr...@iotdb.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org