chrisdutz commented on code in PR #12914:
URL: https://github.com/apache/iotdb/pull/12914#discussion_r1679353609
##########
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:
Nothing else than in the original code ... I theory the parser should throw
an exception, but the way it was implemented, it would have been an empty
NodeCoordinate. Exactly this was also the reason, why deleting by node didn't
cause any problems prior to me adding support for deleting by node id. I now
handle one case more that wasn't handled before.
--
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
