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 2020/10/24 05:46:35 UTC

[GitHub] [iotdb] mychaow opened a new pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

mychaow opened a new pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854


   https://issues.apache.org/jira/browse/IOTDB-884


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] jt2594838 merged pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
jt2594838 merged pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] mychaow commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r511362737



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1505,6 +1511,48 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
     return status;
   }
 
+  /**
+   * Forward each sub-plan to its belonging data group, and combine responses from the groups.
+   *
+   * @param planGroupMap sub-plan -> data group pairs
+   */
+  private TSStatus forwardCreateMultiTimeSeriesPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap,
+                                           CreateMultiTimeSeriesPlan plan) {
+    List<String> errorCodePartitionGroups = new ArrayList<>();
+    TSStatus tmpStatus;
+    CreateMultiTimeSeriesPlan subPlan;
+    Map<Integer, Exception> results = new HashMap<>();
+    boolean noFailure = true;
+    boolean isBatchFailure = false;
+    // send sub-plans to each belonging data group and collect results
+    for (Map.Entry<PhysicalPlan, PartitionGroup> entry : planGroupMap.entrySet()) {
+      tmpStatus = forwardToSingleGroup(entry);
+      subPlan = (CreateMultiTimeSeriesPlan) entry.getKey();
+      logger.debug("{}: from {},{},{}", name, entry.getKey(), entry.getValue(), tmpStatus);
+      noFailure =
+        (tmpStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) && noFailure;
+      isBatchFailure = (tmpStatus.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode())
+        || isBatchFailure;
+      if (tmpStatus.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode()) {
+        results.putAll(subPlan.getResults());
+      }
+      if (tmpStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+        // execution failed, record the error message
+        errorCodePartitionGroups.add(String.format("[%s@%s:%s:%s]",
+          tmpStatus.getCode(), entry.getValue().getHeader(),
+          tmpStatus.getMessage(), tmpStatus.subStatus));
+      }
+    }
+
+    plan.setResults(results);
+    if (noFailure || isBatchFailure) {
+      return RpcUtils.getStatus(TSStatusCode.SUCCESS_STATUS);

Review comment:
       got it!




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] LebronAl commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r511329759



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1478,6 +1479,11 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
         // belongs to NodeB, when NodeA returns a success while NodeB returns a failure, the
         // failure and success should be placed into proper positions in TSStatus.subStatus
         status = forwardInsertTabletPlan(planGroupMap, (InsertTabletPlan) plan);
+      } else if (plan instanceof CreateMultiTimeSeriesPlan) {
+        // CreateMultiTimeSeriesPlans contain many rows, each will correspond to a TSStatus as its
+        // execution result, as the plan is split and the sub-plans may have interleaving ranges,
+        // we must assure that each TSStatus is placed to the right position

Review comment:
       Minor Suggesion:
   ```
   // CreateMultiTimeSeriesPlan contains many rows, each will correspond to a TSStatus as its
   // execution result, as the plan is splited and the sub-plans may have interleaving ranges,
   // we must assure that each TSStatus is placed to the right position
   ```

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1505,6 +1511,48 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
     return status;
   }
 
+  /**
+   * Forward each sub-plan to its belonging data group, and combine responses from the groups.
+   *
+   * @param planGroupMap sub-plan -> data group pairs
+   */
+  private TSStatus forwardCreateMultiTimeSeriesPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap,
+                                           CreateMultiTimeSeriesPlan plan) {
+    List<String> errorCodePartitionGroups = new ArrayList<>();
+    TSStatus tmpStatus;
+    CreateMultiTimeSeriesPlan subPlan;
+    Map<Integer, Exception> results = new HashMap<>();
+    boolean noFailure = true;
+    boolean isBatchFailure = false;
+    // send sub-plans to each belonging data group and collect results
+    for (Map.Entry<PhysicalPlan, PartitionGroup> entry : planGroupMap.entrySet()) {
+      tmpStatus = forwardToSingleGroup(entry);
+      subPlan = (CreateMultiTimeSeriesPlan) entry.getKey();
+      logger.debug("{}: from {},{},{}", name, entry.getKey(), entry.getValue(), tmpStatus);
+      noFailure =
+        (tmpStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) && noFailure;
+      isBatchFailure = (tmpStatus.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode())

Review comment:
       When will `forwardToSingleGroup` return a TSStatusCode.MULTIPLE_ERROR during creating multi timeseries? It seems that this TSStatus  will only be returned by BatchInsertionException, which is only thrown during 'insertTablet' interface currently~

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1505,6 +1511,48 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
     return status;
   }
 
+  /**
+   * Forward each sub-plan to its belonging data group, and combine responses from the groups.
+   *
+   * @param planGroupMap sub-plan -> data group pairs
+   */
+  private TSStatus forwardCreateMultiTimeSeriesPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap,
+                                           CreateMultiTimeSeriesPlan plan) {
+    List<String> errorCodePartitionGroups = new ArrayList<>();
+    TSStatus tmpStatus;
+    CreateMultiTimeSeriesPlan subPlan;
+    Map<Integer, Exception> results = new HashMap<>();
+    boolean noFailure = true;
+    boolean isBatchFailure = false;
+    // send sub-plans to each belonging data group and collect results
+    for (Map.Entry<PhysicalPlan, PartitionGroup> entry : planGroupMap.entrySet()) {
+      tmpStatus = forwardToSingleGroup(entry);
+      subPlan = (CreateMultiTimeSeriesPlan) entry.getKey();
+      logger.debug("{}: from {},{},{}", name, entry.getKey(), entry.getValue(), tmpStatus);
+      noFailure =
+        (tmpStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) && noFailure;
+      isBatchFailure = (tmpStatus.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode())
+        || isBatchFailure;
+      if (tmpStatus.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode()) {
+        results.putAll(subPlan.getResults());
+      }
+      if (tmpStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
+        // execution failed, record the error message
+        errorCodePartitionGroups.add(String.format("[%s@%s:%s:%s]",
+          tmpStatus.getCode(), entry.getValue().getHeader(),
+          tmpStatus.getMessage(), tmpStatus.subStatus));
+      }
+    }
+
+    plan.setResults(results);
+    if (noFailure || isBatchFailure) {
+      return RpcUtils.getStatus(TSStatusCode.SUCCESS_STATUS);

Review comment:
       Why not use `StatusUtils.OK`? then we can reduce a creation of a TSStatus object with the same meaning.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] mychaow commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r512441727



##########
File path: server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java
##########
@@ -1070,11 +1075,19 @@ private boolean createMultiTimeSeries(CreateMultiTimeSeriesPlan createMultiTimeS
       try {
         createTimeSeries(plan);
       } catch (QueryProcessException e) {
-        results.put(createMultiTimeSeriesPlan.getIndexes().get(i), e);
+        if (results == null) {
+          results = new TSStatus[createMultiTimeSeriesPlan.getIndexes().size()];
+          Arrays.fill(results, RpcUtils.SUCCESS_STATUS);
+        }
+        results[i] = RpcUtils.getStatus(TSStatusCode.INTERNAL_SERVER_ERROR, e.getMessage());
+        hasFailed = true;
         logger.debug("meet error while processing create timeseries. ", e);
       }
     }
-    createMultiTimeSeriesPlan.setResults(results);
+
+    if (hasFailed) {
+      throw new BatchInsertionException(results);

Review comment:
       ok




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] mychaow commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r511353495



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1478,6 +1479,11 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
         // belongs to NodeB, when NodeA returns a success while NodeB returns a failure, the
         // failure and success should be placed into proper positions in TSStatus.subStatus
         status = forwardInsertTabletPlan(planGroupMap, (InsertTabletPlan) plan);
+      } else if (plan instanceof CreateMultiTimeSeriesPlan) {
+        // CreateMultiTimeSeriesPlans contain many rows, each will correspond to a TSStatus as its
+        // execution result, as the plan is split and the sub-plans may have interleaving ranges,
+        // we must assure that each TSStatus is placed to the right position

Review comment:
       Thank you!




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] mychaow commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r511362821



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1505,6 +1511,48 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
     return status;
   }
 
+  /**
+   * Forward each sub-plan to its belonging data group, and combine responses from the groups.
+   *
+   * @param planGroupMap sub-plan -> data group pairs
+   */
+  private TSStatus forwardCreateMultiTimeSeriesPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap,
+                                           CreateMultiTimeSeriesPlan plan) {
+    List<String> errorCodePartitionGroups = new ArrayList<>();
+    TSStatus tmpStatus;
+    CreateMultiTimeSeriesPlan subPlan;
+    Map<Integer, Exception> results = new HashMap<>();
+    boolean noFailure = true;
+    boolean isBatchFailure = false;
+    // send sub-plans to each belonging data group and collect results
+    for (Map.Entry<PhysicalPlan, PartitionGroup> entry : planGroupMap.entrySet()) {
+      tmpStatus = forwardToSingleGroup(entry);
+      subPlan = (CreateMultiTimeSeriesPlan) entry.getKey();
+      logger.debug("{}: from {},{},{}", name, entry.getKey(), entry.getValue(), tmpStatus);
+      noFailure =
+        (tmpStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) && noFailure;
+      isBatchFailure = (tmpStatus.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode())

Review comment:
       yes, you are right! I'll fix it!




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] jt2594838 commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r512365107



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanRouter.java
##########
@@ -292,4 +295,64 @@ private PartitionGroup routePlan(ShowChildPathsPlan plan) {
     }
     return result;
   }
+
+  @SuppressWarnings("SuspiciousSystemArraycopy")
+  private Map<PhysicalPlan, PartitionGroup> splitAndRoutePlan(CreateMultiTimeSeriesPlan plan)
+    throws MetadataException {
+    Map<PhysicalPlan, PartitionGroup> result = new HashMap<>();
+    Map<PartitionGroup, PhysicalPlan> groupHoldPlan = new HashMap<>();
+
+    for (int i = 0; i < plan.getPaths().size(); i++) {
+      PartialPath path = plan.getPaths().get(i);
+      PartitionGroup partitionGroup =
+        partitionTable.partitionByPathTime(path, 0);
+      CreateMultiTimeSeriesPlan subPlan = null;
+      if (groupHoldPlan.get(partitionGroup) == null) {

Review comment:
       Assign `groupHoldPlan.get(partitionGroup)` to a local variable so that you will not need to call `get` twice.

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1510,46 +1511,66 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
    *
    * @param planGroupMap sub-plan -> data group pairs
    */
-  private TSStatus forwardInsertTabletPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap,
-      InsertTabletPlan plan) {
+  private TSStatus forwardMultiSubPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap,
+                                           PhysicalPlan plan) {

Review comment:
       Maybe we should rename `plan` to something like `completePlan`, `originalPlan` or `parentPlan`.

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1466,18 +1467,18 @@ public TSStatus processPartitionedPlan(PhysicalPlan plan) throws UnsupportedPlan
    */
   private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, PhysicalPlan plan) {
     // the error codes from the groups that cannot execute the plan
-    TSStatus status;
+    TSStatus status = null;
     if (planGroupMap.size() == 1) {
       status = forwardToSingleGroup(planGroupMap.entrySet().iterator().next());
     } else {
-      if (plan instanceof InsertTabletPlan) {
-        // InsertTabletPlans contain many rows, each will correspond to a TSStatus as its
+      if (plan instanceof InsertTabletPlan || plan instanceof CreateMultiTimeSeriesPlan) {
+        // InsertTabletPlan contains many rows, each will correspond to a TSStatus as its

Review comment:
       Not only `InsertTabletPlan` now, please update the comment.

##########
File path: cluster/src/test/java/org/apache/iotdb/cluster/log/snapshot/DataSnapshotTest.java
##########
@@ -75,6 +75,11 @@ public void readFile(String filePath, long offset, int length,
               }
             }).start();
           }
+
+          @Override
+          public void removeHardLink(String hardLinkPath, AsyncMethodCallback<Void> resultHandler) throws TException {
+            System.out.println("remote hardLinkPath: " + hardLinkPath);

Review comment:
       Actually delete the file using something like `Files.deleteIfExists()`.

##########
File path: server/src/main/java/org/apache/iotdb/db/engine/storagegroup/StorageGroupProcessor.java
##########
@@ -1937,6 +1937,7 @@ private void removeFullyOverlapFiles(TsFileResource newTsFile, Iterator<TsFileRe
     while (iterator.hasNext()) {
       TsFileResource existingTsFile = iterator.next();
       if (newTsFile.getHistoricalVersions().containsAll(existingTsFile.getHistoricalVersions())
+          && !newTsFile.getHistoricalVersions().equals(existingTsFile.getHistoricalVersions())

Review comment:
       This may skip the unclosed TsFile that should be replaced by the pulled file, and I think cluster_new now has properly handled this, so please abort the change.

##########
File path: server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java
##########
@@ -1070,11 +1075,19 @@ private boolean createMultiTimeSeries(CreateMultiTimeSeriesPlan createMultiTimeS
       try {
         createTimeSeries(plan);
       } catch (QueryProcessException e) {
-        results.put(createMultiTimeSeriesPlan.getIndexes().get(i), e);
+        if (results == null) {
+          results = new TSStatus[createMultiTimeSeriesPlan.getIndexes().size()];
+          Arrays.fill(results, RpcUtils.SUCCESS_STATUS);
+        }
+        results[i] = RpcUtils.getStatus(TSStatusCode.INTERNAL_SERVER_ERROR, e.getMessage());

Review comment:
       Use `e.getErrorCode()` here.

##########
File path: server/src/main/java/org/apache/iotdb/db/qp/executor/PlanExecutor.java
##########
@@ -1070,11 +1075,19 @@ private boolean createMultiTimeSeries(CreateMultiTimeSeriesPlan createMultiTimeS
       try {
         createTimeSeries(plan);
       } catch (QueryProcessException e) {
-        results.put(createMultiTimeSeriesPlan.getIndexes().get(i), e);
+        if (results == null) {
+          results = new TSStatus[createMultiTimeSeriesPlan.getIndexes().size()];
+          Arrays.fill(results, RpcUtils.SUCCESS_STATUS);
+        }
+        results[i] = RpcUtils.getStatus(TSStatusCode.INTERNAL_SERVER_ERROR, e.getMessage());
+        hasFailed = true;
         logger.debug("meet error while processing create timeseries. ", e);
       }
     }
-    createMultiTimeSeriesPlan.setResults(results);
+
+    if (hasFailed) {
+      throw new BatchInsertionException(results);

Review comment:
       I think we should rename this to `BatchProcessException` now.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] mychaow commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r511362821



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/MetaGroupMember.java
##########
@@ -1505,6 +1511,48 @@ private TSStatus forwardPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap, Phy
     return status;
   }
 
+  /**
+   * Forward each sub-plan to its belonging data group, and combine responses from the groups.
+   *
+   * @param planGroupMap sub-plan -> data group pairs
+   */
+  private TSStatus forwardCreateMultiTimeSeriesPlan(Map<PhysicalPlan, PartitionGroup> planGroupMap,
+                                           CreateMultiTimeSeriesPlan plan) {
+    List<String> errorCodePartitionGroups = new ArrayList<>();
+    TSStatus tmpStatus;
+    CreateMultiTimeSeriesPlan subPlan;
+    Map<Integer, Exception> results = new HashMap<>();
+    boolean noFailure = true;
+    boolean isBatchFailure = false;
+    // send sub-plans to each belonging data group and collect results
+    for (Map.Entry<PhysicalPlan, PartitionGroup> entry : planGroupMap.entrySet()) {
+      tmpStatus = forwardToSingleGroup(entry);
+      subPlan = (CreateMultiTimeSeriesPlan) entry.getKey();
+      logger.debug("{}: from {},{},{}", name, entry.getKey(), entry.getValue(), tmpStatus);
+      noFailure =
+        (tmpStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()) && noFailure;
+      isBatchFailure = (tmpStatus.getCode() == TSStatusCode.MULTIPLE_ERROR.getStatusCode())

Review comment:
       yes, you are right!




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [iotdb] mychaow commented on a change in pull request #1854: [IOTDB-884] group createMultiTimeseriesPlan by partitionGroup

Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #1854:
URL: https://github.com/apache/iotdb/pull/1854#discussion_r512441846



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanRouter.java
##########
@@ -292,4 +295,64 @@ private PartitionGroup routePlan(ShowChildPathsPlan plan) {
     }
     return result;
   }
+
+  @SuppressWarnings("SuspiciousSystemArraycopy")
+  private Map<PhysicalPlan, PartitionGroup> splitAndRoutePlan(CreateMultiTimeSeriesPlan plan)
+    throws MetadataException {
+    Map<PhysicalPlan, PartitionGroup> result = new HashMap<>();
+    Map<PartitionGroup, PhysicalPlan> groupHoldPlan = new HashMap<>();
+
+    for (int i = 0; i < plan.getPaths().size(); i++) {
+      PartialPath path = plan.getPaths().get(i);
+      PartitionGroup partitionGroup =
+        partitionTable.partitionByPathTime(path, 0);
+      CreateMultiTimeSeriesPlan subPlan = null;
+      if (groupHoldPlan.get(partitionGroup) == null) {

Review comment:
       good




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org