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/07 15:11:13 UTC

[GitHub] [iotdb] MiniSho opened a new pull request, #7264: [IOTDB-4361] Add a precheck in removing datanode

MiniSho opened a new pull request, #7264:
URL: https://github.com/apache/iotdb/pull/7264

   We should add a precheck when the confignode executes removing datanode :
   1. Make sure the node which execute addRegionPeer is in running
   2. Throw an error which says it's not supported to remove the datanode because the configuration of the cluster is only one replication.
   


-- 
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 #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
Beyyes commented on code in PR #7264:
URL: https://github.com/apache/iotdb/pull/7264#discussion_r966862872


##########
confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java:
##########
@@ -492,4 +524,21 @@ private Optional<TDataNodeLocation> filterDataNodeWithOtherRegionReplica(
     // TODO replace findAny() by select the low load node.
     return regionReplicaNodes.stream().filter(e -> !e.equals(filterLocation)).findAny();
   }
+
+  /**
+   * Check the protocol of the cluster, standalone is not supported to remove data node currently
+   *
+   * @return SUCCEED_STATUS if the Cluster is not standalone protocol, REMOVE_DATANODE_FAILED
+   *     otherwise
+   */
+  private TSStatus checkClusterProtocol() {
+    TSStatus status = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
+    if (CONF.getDataRegionConsensusProtocolClass().equals(ConsensusFactory.StandAloneConsensus)
+        && CONF.getSchemaRegionConsensusProtocolClass()

Review Comment:
   ```suggestion
           || CONF.getSchemaRegionConsensusProtocolClass()
   ```



-- 
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] MiniSho commented on pull request #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
MiniSho commented on PR #7264:
URL: https://github.com/apache/iotdb/pull/7264#issuecomment-1244958284

   when remove a datanode whic is not in running state and the configuration of cluster is one replication, it throws an error
   ![289a747043fd6a5108313d80ce46ee0](https://user-images.githubusercontent.com/42286868/189826063-c8a99717-e372-46bd-93a3-8a46d1711618.png)
   


-- 
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 #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
Beyyes merged PR #7264:
URL: https://github.com/apache/iotdb/pull/7264


-- 
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] MiniSho commented on a diff in pull request #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
MiniSho commented on code in PR #7264:
URL: https://github.com/apache/iotdb/pull/7264#discussion_r968087852


##########
confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java:
##########
@@ -433,8 +445,28 @@ private TSStatus checkDataNodeExist(RemoveDataNodePlan removeDataNodePlan) {
    */
   private TSStatus checkRegionReplication(RemoveDataNodePlan removeDataNodePlan) {
     TSStatus status = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
-    int removedDataNodeSize = removeDataNodePlan.getDataNodeLocations().size();
+    List<TDataNodeLocation> removedDataNodes = removeDataNodePlan.getDataNodeLocations();
     int allDataNodeSize = configManager.getNodeManager().getRegisteredDataNodeCount();
+    if (NodeInfo.getMinimumDataNode() == 1) {

Review Comment:
   done



##########
confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java:
##########
@@ -433,8 +445,28 @@ private TSStatus checkDataNodeExist(RemoveDataNodePlan removeDataNodePlan) {
    */
   private TSStatus checkRegionReplication(RemoveDataNodePlan removeDataNodePlan) {
     TSStatus status = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
-    int removedDataNodeSize = removeDataNodePlan.getDataNodeLocations().size();
+    List<TDataNodeLocation> removedDataNodes = removeDataNodePlan.getDataNodeLocations();
     int allDataNodeSize = configManager.getNodeManager().getRegisteredDataNodeCount();
+    if (NodeInfo.getMinimumDataNode() == 1) {

Review Comment:
   done



-- 
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] MiniSho commented on a diff in pull request #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
MiniSho commented on code in PR #7264:
URL: https://github.com/apache/iotdb/pull/7264#discussion_r968087648


##########
confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java:
##########
@@ -492,4 +524,21 @@ private Optional<TDataNodeLocation> filterDataNodeWithOtherRegionReplica(
     // TODO replace findAny() by select the low load node.
     return regionReplicaNodes.stream().filter(e -> !e.equals(filterLocation)).findAny();
   }
+
+  /**
+   * Check the protocol of the cluster, standalone is not supported to remove data node currently
+   *
+   * @return SUCCEED_STATUS if the Cluster is not standalone protocol, REMOVE_DATANODE_FAILED
+   *     otherwise
+   */
+  private TSStatus checkClusterProtocol() {
+    TSStatus status = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
+    if (CONF.getDataRegionConsensusProtocolClass().equals(ConsensusFactory.StandAloneConsensus)
+        && CONF.getSchemaRegionConsensusProtocolClass()

Review Comment:
   done



-- 
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] MiniSho commented on pull request #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
MiniSho commented on PR #7264:
URL: https://github.com/apache/iotdb/pull/7264#issuecomment-1243942088

   when set the configuration as Standalone protocol, it throws an error
   ![ba8f3fe60cbb242de3869c697f25121](https://user-images.githubusercontent.com/42286868/189699089-46e518db-4467-4e00-9bf0-ab7ffffcd8bd.png)
   


-- 
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] MiniSho closed pull request #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
MiniSho closed pull request #7264: [IOTDB-4361] Add a precheck in removing datanode
URL: https://github.com/apache/iotdb/pull/7264


-- 
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 #7264: [IOTDB-4361] Add a precheck in removing datanode

Posted by GitBox <gi...@apache.org>.
CRZbulabula commented on code in PR #7264:
URL: https://github.com/apache/iotdb/pull/7264#discussion_r967624081


##########
confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java:
##########
@@ -433,8 +445,28 @@ private TSStatus checkDataNodeExist(RemoveDataNodePlan removeDataNodePlan) {
    */
   private TSStatus checkRegionReplication(RemoveDataNodePlan removeDataNodePlan) {
     TSStatus status = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
-    int removedDataNodeSize = removeDataNodePlan.getDataNodeLocations().size();
+    List<TDataNodeLocation> removedDataNodes = removeDataNodePlan.getDataNodeLocations();
     int allDataNodeSize = configManager.getNodeManager().getRegisteredDataNodeCount();
+    if (NodeInfo.getMinimumDataNode() == 1) {

Review Comment:
   Add annotation in front of this block of code in order to explain the usage.



##########
confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/DataNodeRemoveHandler.java:
##########
@@ -433,8 +445,28 @@ private TSStatus checkDataNodeExist(RemoveDataNodePlan removeDataNodePlan) {
    */
   private TSStatus checkRegionReplication(RemoveDataNodePlan removeDataNodePlan) {
     TSStatus status = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
-    int removedDataNodeSize = removeDataNodePlan.getDataNodeLocations().size();
+    List<TDataNodeLocation> removedDataNodes = removeDataNodePlan.getDataNodeLocations();
     int allDataNodeSize = configManager.getNodeManager().getRegisteredDataNodeCount();
+    if (NodeInfo.getMinimumDataNode() == 1) {

Review Comment:
   We should step into this if statement as long as the schemaReplicationFactor or the dataReplicationFactor equals to 1



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