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/01 09:14:00 UTC
[GitHub] [iotdb] ericpai opened a new pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
ericpai opened a new pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877
## Description
See JIRA https://issues.apache.org/jira/browse/IOTDB-1600 .
This PR has:
- [x] been self-reviewed.
- [ ] been tested in a test IoTDB cluster.
--
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] LebronAl commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r703156136
##########
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:
Although it's trivial, I think `new HashMap<>()` is enough as `new HashMap<>(0)` will do some extra work.
--
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] ericpai edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909878478
--
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] ericpai commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r700698052
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanRouter.java
##########
@@ -504,4 +507,30 @@ private CreateMultiTimeSeriesPlan createSubPlan(CreateMultiTimeSeriesPlan plan)
subPlan.setIndexes(new ArrayList<>());
return subPlan;
}
+
+ /**
+ * @param plan InsertRowsOfOneDevicePlan
+ * @return key is InsertRowsOfOneDevicePlan, value is the partition group the plan belongs to. All
+ * InsertRowPlans in InsertRowsOfOneDevicePlan belong to one same storage group.
+ */
+ private Map<PhysicalPlan, PartitionGroup> splitAndRoutePlan(InsertRowsOfOneDevicePlan plan)
+ throws MetadataException {
+ Map<PhysicalPlan, PartitionGroup> result = new HashMap<>();
+ Map<PartitionGroup, List<InsertRowPlan>> groupPlanMap = new HashMap<>();
+ PartialPath storageGroup = getMManager().getStorageGroupPath(plan.getPrefixPath());
+ for (InsertRowPlan p : plan.getRowPlans()) {
Review comment:
OK, I will add some UT cases. Until now the whole IoTDB project have no IT cases running in cluster, at least I haven't found any...
--
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] ericpai commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r703154590
##########
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:
Thanks, I have updated this comment in the newest commit.
--
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] LebronAl commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [iotdb] coveralls edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42688041)
Coverage increased (+0.1%) to 67.497% when pulling **6dd26e7801b98d0e5a5f16ae2aca6b05a7ff07ba on ericpai:bugfix/iotdb-1600** into **3333f3b2a6a5035b7f42a33dd23b3e841b0768b1 on apache:master**.
--
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] coveralls edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42673584)
Coverage increased (+0.02%) to 67.394% when pulling **b5ba4de147c70eed80f5bc82430cd8f1a321bd39 on ericpai:bugfix/iotdb-1600** into **3333f3b2a6a5035b7f42a33dd23b3e841b0768b1 on apache:master**.
--
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] ericpai commented on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-913521254
> Great work!
> But these changes are lack of `error handling` which you can refer to `InsertRowsPlan`.
> Maybe two more fields `indexList, results` are needed.
> 
> BTW, better to add some e2e test to convince your changes.
Thanks for your hint! I have made the following changes according to your review comments in this [commit](https://github.com/apache/iotdb/pull/3877/commits/b5ba4de147c70eed80f5bc82430cd8f1a321bd39):
1. Add `indexList` and `results` like `InsertRowsPlan` to capture execution results, with their serializing logics.
2. Add unit test for serializing and deserializing `InsertRowsOfOneDevicePlan`.
3. Add IT cases for `insertRecordsOfOneDevice()` in testcontainer.
--
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] wangchao316 commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r700696460
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanRouter.java
##########
@@ -504,4 +507,30 @@ private CreateMultiTimeSeriesPlan createSubPlan(CreateMultiTimeSeriesPlan plan)
subPlan.setIndexes(new ArrayList<>());
return subPlan;
}
+
+ /**
+ * @param plan InsertRowsOfOneDevicePlan
+ * @return key is InsertRowsOfOneDevicePlan, value is the partition group the plan belongs to. All
+ * InsertRowPlans in InsertRowsOfOneDevicePlan belong to one same storage group.
+ */
+ private Map<PhysicalPlan, PartitionGroup> splitAndRoutePlan(InsertRowsOfOneDevicePlan plan)
+ throws MetadataException {
+ Map<PhysicalPlan, PartitionGroup> result = new HashMap<>();
+ Map<PartitionGroup, List<InsertRowPlan>> groupPlanMap = new HashMap<>();
+ PartialPath storageGroup = getMManager().getStorageGroupPath(plan.getPrefixPath());
+ for (InsertRowPlan p : plan.getRowPlans()) {
Review comment:
hi , Good Job.
I have a little... if you could add some UT or IT, this will validation...
--
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] ericpai commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r703154807
##########
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:
Thanks, the map initialization is refined to the class field declaration.
--
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] coveralls commented on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42544741)
Coverage decreased (-0.004%) to 67.371% when pulling **fa55e8fe0337b9f9f5178ade3da07306da243b9f on ericpai:bugfix/iotdb-1600** into **820101c80e4621c9bc150aed9337e76cb2188b06 on apache:master**.
--
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] ericpai commented on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909878478
I have tested this change in 3 cases, in a 3 nodes cluster with 2 replicas.
1. Disable time partition: Write 5 rows with 3 measurements of one device. Data can be queried out in any nodes.
2. Enable time partition: Write 5 rows belonging to 3 different time partitions with 3 measurements of one device. Data can be queried out in any nodes. With observing the partitioned plans, they grouped as our expected.
3. Enable time partition: Write 5 rows belonging to 1 time partitions with 3 measurements of one device. Data can be queried out in any nodes. With observing the partitioned plans, they grouped as our expected.
This PR is ready for reviewing now. BTW, I think it should be backport to rel/0.12.
--
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] coveralls commented on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42544741)
Coverage decreased (-0.004%) to 67.371% when pulling **fa55e8fe0337b9f9f5178ade3da07306da243b9f on ericpai:bugfix/iotdb-1600** into **820101c80e4621c9bc150aed9337e76cb2188b06 on apache:master**.
--
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] ericpai edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909878478
I have tested this change in 3 cases, in a 3 nodes cluster with 2 replicas.
1. Disable time partition: Write 5 rows with 3 measurements of one device. Data can be queried out in any nodes.
2. Enable time partition: Write 5 rows belonging to 3 different time partitions with 3 measurements of one device. Data can be queried out in any nodes. By observing the partitioned plans, they're grouped as our expected.
3. Enable time partition: Write 5 rows belonging to 1 time partitions with 3 measurements of one device. Data can be queried out in any nodes. By observing the partitioned plans, they're grouped as our expected.
This PR is ready for reviewing now. BTW, I think it should be backport to rel/0.12.
--
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] ericpai edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909878478
I have tested this change in 3 cases, in a 3 nodes cluster with 2 replicas.
1. Disable time partition: Write 5 rows with 3 measurements of one device. Data can be queried out in any nodes.
2. Enable time partition: Write 5 rows belonging to 3 different time partitions with 3 measurements of one device. Data can be queried out in any nodes. With observing the partitioned plans, they're grouped as our expected.
3. Enable time partition: Write 5 rows belonging to 1 time partitions with 3 measurements of one device. Data can be queried out in any nodes. With observing the partitioned plans, they grouped as our expected.
This PR is ready for reviewing now. BTW, I think it should be backport to rel/0.12.
--
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] ericpai commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r700701106
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanRouter.java
##########
@@ -504,4 +507,30 @@ private CreateMultiTimeSeriesPlan createSubPlan(CreateMultiTimeSeriesPlan plan)
subPlan.setIndexes(new ArrayList<>());
return subPlan;
}
+
+ /**
+ * @param plan InsertRowsOfOneDevicePlan
+ * @return key is InsertRowsOfOneDevicePlan, value is the partition group the plan belongs to. All
+ * InsertRowPlans in InsertRowsOfOneDevicePlan belong to one same storage group.
+ */
+ private Map<PhysicalPlan, PartitionGroup> splitAndRoutePlan(InsertRowsOfOneDevicePlan plan)
+ throws MetadataException {
+ Map<PhysicalPlan, PartitionGroup> result = new HashMap<>();
+ Map<PartitionGroup, List<InsertRowPlan>> groupPlanMap = new HashMap<>();
+ PartialPath storageGroup = getMManager().getStorageGroupPath(plan.getPrefixPath());
+ for (InsertRowPlan p : plan.getRowPlans()) {
Review comment:
Good, let me have a try~
--
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] ericpai commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r700698052
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanRouter.java
##########
@@ -504,4 +507,30 @@ private CreateMultiTimeSeriesPlan createSubPlan(CreateMultiTimeSeriesPlan plan)
subPlan.setIndexes(new ArrayList<>());
return subPlan;
}
+
+ /**
+ * @param plan InsertRowsOfOneDevicePlan
+ * @return key is InsertRowsOfOneDevicePlan, value is the partition group the plan belongs to. All
+ * InsertRowPlans in InsertRowsOfOneDevicePlan belong to one same storage group.
+ */
+ private Map<PhysicalPlan, PartitionGroup> splitAndRoutePlan(InsertRowsOfOneDevicePlan plan)
+ throws MetadataException {
+ Map<PhysicalPlan, PartitionGroup> result = new HashMap<>();
+ Map<PartitionGroup, List<InsertRowPlan>> groupPlanMap = new HashMap<>();
+ PartialPath storageGroup = getMManager().getStorageGroupPath(plan.getPrefixPath());
+ for (InsertRowPlan p : plan.getRowPlans()) {
Review comment:
OK, I will add some UT cases. Until now the whole IoTDB project has no IT cases running in cluster, at least I haven't found any...
--
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] LebronAl merged pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl merged pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877
--
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] HTHou commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
HTHou commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r700700794
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanRouter.java
##########
@@ -504,4 +507,30 @@ private CreateMultiTimeSeriesPlan createSubPlan(CreateMultiTimeSeriesPlan plan)
subPlan.setIndexes(new ArrayList<>());
return subPlan;
}
+
+ /**
+ * @param plan InsertRowsOfOneDevicePlan
+ * @return key is InsertRowsOfOneDevicePlan, value is the partition group the plan belongs to. All
+ * InsertRowPlans in InsertRowsOfOneDevicePlan belong to one same storage group.
+ */
+ private Map<PhysicalPlan, PartitionGroup> splitAndRoutePlan(InsertRowsOfOneDevicePlan plan)
+ throws MetadataException {
+ Map<PhysicalPlan, PartitionGroup> result = new HashMap<>();
+ Map<PartitionGroup, List<InsertRowPlan>> groupPlanMap = new HashMap<>();
+ PartialPath storageGroup = getMManager().getStorageGroupPath(plan.getPrefixPath());
+ for (InsertRowPlan p : plan.getRowPlans()) {
Review comment:
Actually, we are using testcontainer to run IT cases...
See https://github.com/apache/iotdb/blob/master/testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
--
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] ericpai commented on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909878478
I have tested this change in 3 cases, in a 3 nodes cluster with 2 replicas.
1. Disable time partition: Write 5 rows with 3 measurements of one device. Data can be queried out in any nodes.
2. Enable time partition: Write 5 rows belonging to 3 different time partitions with 3 measurements of one device. Data can be queried out in any nodes. With observing the partitioned plans, they grouped as our expected.
3. Enable time partition: Write 5 rows belonging to 1 time partitions with 3 measurements of one device. Data can be queried out in any nodes. With observing the partitioned plans, they grouped as our expected.
This PR is ready for reviewing now. BTW, I think it should be backport to rel/0.12.
--
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] coveralls commented on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42544741)
Coverage decreased (-0.004%) to 67.371% when pulling **fa55e8fe0337b9f9f5178ade3da07306da243b9f on ericpai:bugfix/iotdb-1600** into **820101c80e4621c9bc150aed9337e76cb2188b06 on apache:master**.
--
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] coveralls edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42688969)
Coverage increased (+0.003%) to 67.38% when pulling **cd303eb6eba8c74b970d81b9b3a94174f70edf18 on ericpai:bugfix/iotdb-1600** into **3333f3b2a6a5035b7f42a33dd23b3e841b0768b1 on apache:master**.
--
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] ericpai commented on a change in pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#discussion_r703183227
##########
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:
Fixed. New knowledge got!
--
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] SteveYurongSu commented on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
SteveYurongSu commented on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-911034193
> I think it should be backport to rel/0.12.
Agree. But don't forget to do that ;D
--
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] coveralls edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42566583)
Coverage increased (+0.001%) to 67.376% when pulling **e4d1ba9938349cc752d9d174049d692ff91027fd on ericpai:bugfix/iotdb-1600** into **820101c80e4621c9bc150aed9337e76cb2188b06 on apache:master**.
--
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] coveralls edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
[](https://coveralls.io/builds/42567137)
Coverage increased (+0.01%) to 67.388% when pulling **e4d1ba9938349cc752d9d174049d692ff91027fd on ericpai:bugfix/iotdb-1600** into **820101c80e4621c9bc150aed9337e76cb2188b06 on apache:master**.
--
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] coveralls edited a comment on pull request #3877: [IOTDB-1600] Support InsertRowsOfOneDevicePlan in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3877:
URL: https://github.com/apache/iotdb/pull/3877#issuecomment-909285384
--
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