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 2021/09/07 03:32:39 UTC

[GitHub] [iotdb] LebronAl commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster

LebronAl commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r703152094



##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertRowsOfOneDevicePlan.java
##########
@@ -29,26 +29,52 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 public class InsertRowsOfOneDevicePlan extends InsertPlan implements BatchPlan {
 
+  /**
+   * This class has some duplicated codes with InsertRowsPlan, they should be refined in the future
+   */
   boolean[] isExecuted;
+
   private InsertRowPlan[] rowPlans;
 
+  /**
+   * Suppose there is an InsertRowsOfOneDevicePlan, which contains 5 InsertRowPlans,
+   * rowPlans={InsertRowPlan_0, InsertRowPlan_1, InsertRowPlan_2, InsertRowPlan_3, InsertRowPlan_4},
+   * then the rowPlanIndexList={0, 1, 2, 3, 4} respectively. But when the InsertRowsOfOneDevicePlan
+   * is split into two InsertRowsOfOneDevicePlans according to different storage group in cluster

Review comment:
       All `InsertRowPlan` in `InsertRowsOfOneDevicePlan` belong to the same device, so they must belong to the same storage group, but they may be distributed into different data groups due to different time partition. You need to rephrase the comments here

##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertRowsOfOneDevicePlan.java
##########
@@ -29,26 +29,52 @@
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 public class InsertRowsOfOneDevicePlan extends InsertPlan implements BatchPlan {
 
+  /**
+   * This class has some duplicated codes with InsertRowsPlan, they should be refined in the future
+   */
   boolean[] isExecuted;
+
   private InsertRowPlan[] rowPlans;
 
+  /**
+   * Suppose there is an InsertRowsOfOneDevicePlan, which contains 5 InsertRowPlans,
+   * rowPlans={InsertRowPlan_0, InsertRowPlan_1, InsertRowPlan_2, InsertRowPlan_3, InsertRowPlan_4},
+   * then the rowPlanIndexList={0, 1, 2, 3, 4} respectively. But when the InsertRowsOfOneDevicePlan
+   * is split into two InsertRowsOfOneDevicePlans according to different storage group in cluster
+   * version, suppose that the InsertRowsOfOneDevicePlan_1's rowPlanIndexList = {InsertRowPlan_0,
+   * InsertRowPlan_3, InsertRowPlan_4}, then InsertRowsOfOneDevicePlan_1's rowPlanIndexList = {0, 3,
+   * 4}; InsertRowsOfOneDevicePlan_2's rowPlanIndexList = {InsertRowPlan_1, InsertRowPlan_2} then
+   * InsertRowsOfOneDevicePlan_2's rowPlanIndexList = {1, 2} respectively;
+   */
+  private int[] rowPlanIndexList;
+
+  /** record the result of insert rows */
+  private Map<Integer, TSStatus> results = new HashMap<>();
+
+  public InsertRowsOfOneDevicePlan() {
+    super(OperatorType.BATCH_INSERT_ONE_DEVICE);
+  }
+
   public InsertRowsOfOneDevicePlan(
       PartialPath deviceId,
       Long[] insertTimes,
       List<List<String>> measurements,
       ByteBuffer[] insertValues)
       throws QueryProcessException {
-    super(OperatorType.BATCH_INSERT_ONE_DEVICE);
+    this();
     this.prefixPath = deviceId;
     rowPlans = new InsertRowPlan[insertTimes.length];
+    rowPlanIndexList = new int[insertTimes.length];
+    results = new HashMap<>(insertTimes.length);

Review comment:
       I think we'd better set the default size of this map to 0 to reduce the memory footprint in most cases




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