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/23 09:46:28 UTC
[GitHub] [iotdb] Caideyipi opened a new pull request, #7422: [IOTDB-4482] Master confignode
Caideyipi opened a new pull request, #7422:
URL: https://github.com/apache/iotdb/pull/7422
## IoTDB Distribution: changed the input of node deletion
### 1
Input id instead of IP and port for node deleting;
### 2
Changed some logs;
### 3
Simplified some codes
Test images:
Before deletion:
remove-confignode:
remove-datanode:
After deletion:
<!--
In each section, please describe design decisions made, including:
- Choice of algorithms
- Behavioral aspects. What configuration values are acceptable? How are corner cases and error
conditions handled, such as when there are insufficient resources?
- Class organization and design (how the logic is split between classes, inheritance, composition,
design patterns)
- Method organization and design (how the logic is split between methods, parameters and return types)
- Naming (class, method, API, configuration, HTTP endpoint, names of emitted metrics)
-->
<!-- It's good to describe an alternative design (or mention an alternative name) for every design
(or naming) decision point and compare the alternatives with the designs that you've implemented
(or the names you've chosen) to highlight the advantages of the chosen designs and names. -->
<!-- If there was a discussion of the design of the feature implemented in this PR elsewhere
(e. g. a "Proposal" issue, any other issue, or a thread in the development mailing list),
link to that discussion from this PR description and explain what have changed in your final design
compared to your original proposal or the consensus version in the end of the discussion.
If something hasn't changed since the original discussion, you can omit a detailed discussion of
those aspects of the design here, perhaps apart from brief mentioning for the sake of readability
of this PR description. -->
<!-- Some of the aspects mentioned above may be omitted for simple and small changes. -->
<hr>
This PR has:
- [x] been self-reviewed.
- [ ] concurrent read
- [ ] concurrent write
- [ ] concurrent read and write
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods.
- [ ] added or updated version, __license__, or notice information
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious
for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold
for code coverage.
- [ ] added integration tests.
- [ ] been tested in a test IoTDB cluster.
<!-- Check the items by putting "x" in the brackets for the done things. Not all of these items
apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items
from the checklist above are strictly necessary, but it would be very helpful if you at least
self-review the PR. -->
<hr>
##### Key changed/added classes (or packages if there are too many classes) in this PR
--
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
[GitHub] [iotdb] Caideyipi commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Caideyipi commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r980705802
##########
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:
Solved
--
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
[GitHub] [iotdb] Caideyipi commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Caideyipi commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r980707013
##########
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:
I deem the '-r' somehow redundant, so I removed it in the command
--
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
[GitHub] [iotdb] Beyyes merged pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Beyyes merged PR #7422:
URL: https://github.com/apache/iotdb/pull/7422
--
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
[GitHub] [iotdb] CRZbulabula commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [iotdb] Beyyes commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Beyyes commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r980862180
##########
server/src/main/java/org/apache/iotdb/db/service/DataNodeServerCommandLine.java:
##########
@@ -159,51 +127,67 @@ 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);
- }
-
- 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();
+ // Now support only single datanode deletion
+ if (args.split(",").length > 1) {
+ logger.info("Incorrect input format, usage: <id>/<ip>:<rpc-port>");
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(",")) {
+ if (!isNumeric(id)) {
+ logger.warn("Incorrect id format {}, skipped...", id);
+ continue;
+ }
+ List<TDataNodeLocation> NodeLocationResult =
Review Comment:
```suggestion
List<TDataNodeLocation> nodeLocationResult =
```
--
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
[GitHub] [iotdb] Caideyipi commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Caideyipi commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r980706656
##########
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:
I deem the '-r' somehow redundant, so I removed it in the command
##########
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:
Deleted
--
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
[GitHub] [iotdb] Caideyipi commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Caideyipi commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r980706786
##########
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:
I deem the '-r' somehow redundant, so I removed it in the command
##########
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:
Deleted
--
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
[GitHub] [iotdb] Beyyes commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by 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
[GitHub] [iotdb] Beyyes commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Beyyes commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r979790341
##########
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 `
Same as remove-datanode
--
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
[GitHub] [iotdb] Caideyipi commented on pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Caideyipi commented on PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#issuecomment-1258935898
> hi,I don't think it's a good change. We can easily get the ip and port from the configuration file.
IP and port is re-supported in the current version
--
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
[GitHub] [iotdb] Caideyipi commented on a diff in pull request #7422: [IOTDB-4482] IoTDB distribution: using id to delete nodes
Posted by GitBox <gi...@apache.org>.
Caideyipi commented on code in PR #7422:
URL: https://github.com/apache/iotdb/pull/7422#discussion_r980706271
##########
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:
Reserving multi-node deletion logic is suggested by boss Qiao~
##########
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:
Updated
--
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