[GitHub] [iotdb] Beyyes commented on a diff in pull request #7006: [IOTDB-4092] Protecting Region creation process by CreateRegionGroupsProcedure

[GitBox <gi...@apache.org>]

Beyyes commented on code in PR #7006:
URL: https://github.com/apache/iotdb/pull/7006#discussion_r948745969


##########
confignode/src/main/java/org/apache/iotdb/confignode/client/sync/datanode/SyncDataNodeClientPool.java:
##########
@@ -134,14 +134,21 @@ private void deleteRegions(
       List<TConsensusGroupId> regionIds,
       Set<TRegionReplicaSet> deletedRegionSet) {
     for (TConsensusGroupId regionId : regionIds) {
-      LOGGER.debug("Delete region {} ", regionId);
+      LOGGER.info("Try to delete RegionReplica: {} on DataNode: {}", regionId, endPoint);
       final TSStatus status =
           sendSyncRequestToDataNodeWithRetry(
               endPoint, regionId, DataNodeRequestType.DELETE_REGIONS);
+
       if (status.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
-        LOGGER.info("DELETE Region {} successfully", regionId);
-        deletedRegionSet.removeIf(k -> k.getRegionId().equals(regionId));
+        LOGGER.info("Delete RegionReplica: {} on DataNode: {} successfully", regionId, endPoint);
+      } else {
+        LOGGER.warn(
+            "Failed to delete RegionReplica: {} on DataNode: {}. You might need to delete it manually",
+            regionId,
+            endPoint);
       }
+
+      deletedRegionSet.removeIf(k -> k.getRegionId().equals(regionId));

Review Comment:
   is `deletedRegionSet` used out of this method `deleteRegions`?



##########
confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/write/DeleteRegionGroupsPlan.java:
##########
@@ -18,62 +18,60 @@
  */
 package org.apache.iotdb.confignode.consensus.request.write;
 
-import org.apache.iotdb.common.rpc.thrift.TConsensusGroupId;
+import org.apache.iotdb.common.rpc.thrift.TRegionReplicaSet;
 import org.apache.iotdb.commons.utils.BasicStructureSerDeUtil;
 import org.apache.iotdb.commons.utils.ThriftCommonsSerDeUtils;
-import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlan;
 import org.apache.iotdb.confignode.consensus.request.ConfigPhysicalPlanType;
 
 import java.io.DataOutputStream;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
-public class DeleteRegionsPlan extends ConfigPhysicalPlan {
+public class DeleteRegionGroupsPlan extends CreateRegionGroupsPlan {

Review Comment:
   extends `CreateRegionGroupsPlan` only for common use `regionGroupMap`?



##########
confignode/src/main/java/org/apache/iotdb/confignode/procedure/env/ConfigNodeProcedureEnv.java:
##########
@@ -288,6 +297,39 @@ public void broadCastTheLatestConfigNodeGroup() {
             configManager.getNodeManager().getRegisteredConfigNodes());
   }
 
+  /**
+   * Broadcast the CreateRegionGroupsPlan

Review Comment:
   I think the doc is not specific



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