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:35:33 UTC
[GitHub] [iotdb] MrQuansy opened a new pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
MrQuansy opened a new pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879
## Description
Auto create schema functionality in cluster requires careful design combined with the stand-alone partialInsert functionality and the BatchPlan interface, you can refer to the design https://shimo.im/docs/TTJDkQPkX3YYjpyR .
After the reorganization, the corresponding E2E tests need to be added for continuous integration.
<!--
In each section, please describe design decisions made, including:
- Choice of algorithms
- Behavioral aspects. What configuration values are acceptable? How are corner cases and error
conditions handled, such as when there are insufficient resources?
- Class organization and design (how the logic is split between classes, inheritance, composition,
design patterns)
- Method organization and design (how the logic is split between methods, parameters and return types)
- Naming (class, method, API, configuration, HTTP endpoint, names of emitted metrics)
-->
<!-- It's good to describe an alternative design (or mention an alternative name) for every design
(or naming) decision point and compare the alternatives with the designs that you've implemented
(or the names you've chosen) to highlight the advantages of the chosen designs and names. -->
<!-- If there was a discussion of the design of the feature implemented in this PR elsewhere
(e. g. a "Proposal" issue, any other issue, or a thread in the development mailing list),
link to that discussion from this PR description and explain what have changed in your final design
compared to your original proposal or the consensus version in the end of the discussion.
If something hasn't changed since the original discussion, you can omit a detailed discussion of
those aspects of the design here, perhaps apart from brief mentioning for the sake of readability
of this PR description. -->
<!-- Some of the aspects mentioned above may be omitted for simple and small changes. -->
<hr>
This PR has:
- [ ] been self-reviewed.
- [ ] concurrent read
- [ ] concurrent write
- [ ] concurrent read and write
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods.
- [ ] added or updated version, __license__, or notice information
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious
for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold
for code coverage.
- [x] added integration tests.
- [ ] been tested in a test IoTDB cluster.
<!-- Check the items by putting "x" in the brackets for the done things. Not all of these items
apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items
from the checklist above are strictly necessary, but it would be very helpful if you at least
self-review the PR. -->
<hr>
##### Key changed/added classes (or packages if there are too many classes) in this PR
--
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] MrQuansy commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
MrQuansy commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706859277
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -776,14 +774,28 @@ private TSStatus executeNonQueryPlanWithKnownLeader(PhysicalPlan plan) {
boolean hasCreated = false;
try {
if (plan instanceof InsertPlan
- && status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()
&& ClusterDescriptor.getInstance().getConfig().isEnableAutoCreateSchema()) {
- hasCreated = createTimeseriesForFailedInsertion(((InsertPlan) plan));
+ if (plan instanceof InsertRowsPlan || plan instanceof InsertMultiTabletPlan) {
Review comment:
Actually, there is only 5 lines to put together as a function, I think it is of no need.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706946931
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -816,6 +828,22 @@ private boolean createTimeseriesForFailedInsertion(InsertPlan plan)
}
}
+ if (plan instanceof InsertRowsPlan) {
+ for (InsertRowPlan insertPlan : ((InsertRowsPlan) plan).getInsertRowPlanList()) {
+ if (insertPlan.getFailedMeasurements() != null) {
+ insertPlan.getPlanFromFailed();
+ }
+ }
+ }
+
+ if (plan instanceof InsertRowsOfOneDevicePlan) {
+ for (InsertRowPlan insertPlan : ((InsertRowsOfOneDevicePlan) plan).getRowPlans()) {
+ if (insertPlan.getFailedMeasurements() != null) {
+ insertPlan.getPlanFromFailed();
Review comment:
Can you be sure to return the correct error message(the correct index for a column) when a column is recovered with getPlanFromFailed and the execution fails again?
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42704597)
Coverage increased (+0.08%) to 67.501% when pulling **553d9ef4b1e2c2525fc71a7c16b345c9bf8e2c80 on MrQuansy:auto_create_schema** into **aaa96b974becf7cc57ef5ac36c3fa9e199423010 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] MrQuansy commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
MrQuansy commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706857435
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -705,6 +700,22 @@ public boolean createTimeseries(InsertRowsPlan insertRowsPlan)
return allSuccess;
}
+ public boolean createTimeseries(InsertRowsOfOneDevicePlan insertRowsOfOneDevicePlan)
Review comment:
Each InsertRowPlan in InsertRowsOfOneDevicePlan is related with different measurements. It is necessary to create time series for each InsertRowPlan.
--
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] MrQuansy commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
MrQuansy commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r708770006
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -204,11 +202,11 @@ public DataGroupMember(PartitionGroup nodes) {
allNodes = nodes;
setQueryManager(new ClusterQueryManager());
slotManager = new SlotManager(ClusterConstant.SLOT_NUM, getMemberDir(), getName());
+ singleReplicaApplier = new DataLogApplier(metaGroupMember, this);
Review comment:
I extract the process of executing physical plan as a function. We can directly call the DataLogApplier.exexcutePhysicalPlan to skip log processing.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r705829733
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/SingleReplicaDataApplier.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.cluster.log.applier;
+
+import org.apache.iotdb.cluster.log.Log;
+import org.apache.iotdb.cluster.server.member.DataGroupMember;
+import org.apache.iotdb.cluster.server.member.MetaGroupMember;
+import org.apache.iotdb.db.exception.StorageEngineException;
+import org.apache.iotdb.db.exception.metadata.StorageGroupNotSetException;
+import org.apache.iotdb.db.exception.query.QueryProcessException;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+import org.apache.iotdb.db.qp.physical.crud.InsertMultiTabletPlan;
+import org.apache.iotdb.db.qp.physical.crud.InsertPlan;
+import org.apache.iotdb.db.qp.physical.crud.InsertRowsPlan;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SingleReplicaDataApplier extends DataLogApplier {
+ private static final Logger logger = LoggerFactory.getLogger(SingleReplicaDataApplier.class);
+
+ public SingleReplicaDataApplier(
+ MetaGroupMember metaGroupMember, DataGroupMember dataGroupMember) {
+ super(metaGroupMember, dataGroupMember);
+ }
+
+ @Override
+ public void apply(Log log) {}
+
+ public void applyPhysicalPlan(PhysicalPlan plan)
+ throws QueryProcessException, StorageGroupNotSetException, StorageEngineException {
+ if (plan instanceof InsertMultiTabletPlan) {
Review comment:
- Enhancement : You'd better add this bug-fix logic in [PR3439](https://github.com/apache/iotdb/pull/3439) for SingleReplicaDataApplier.You can refer to [here](https://github.com/apache/iotdb/blob/master/cluster/src/main/java/org/apache/iotdb/cluster/log/applier/DataLogApplier.java#L84) for related code. 5~6 lines code is enough
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42567677)
Coverage increased (+0.02%) to 67.394% when pulling **bc312d889e5841cb6ce0c699f009c73d822dffa2 on MrQuansy:auto_create_schema** into **79f29f4af3db8ee6815eb72974092b31147e8358 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] MrQuansy commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
MrQuansy commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706858143
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/CreateMultiTimeSeriesPlan.java
##########
@@ -144,6 +144,11 @@ public void setIndexes(List<Integer> indexes) {
return results;
}
+ @Override
+ public List<PartialPath> getPrefixPaths() {
+ return null;
Review comment:
The method get prefixPaths is defined for batch plan. But CreateMultiTimeSeriesPlan stores each path of time series, I think it is no need to implement 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.
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42655253)
Coverage increased (+0.01%) to 67.24% when pulling **1caa1cb78d3e3d1514310856286eec9e1199b661 on MrQuansy:auto_create_schema** into **9931149fb2e8e7ddec8187539d9940869edd1a7a 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 commented on pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42567677)
Coverage increased (+0.02%) to 67.394% when pulling **bc312d889e5841cb6ce0c699f009c73d822dffa2 on MrQuansy:auto_create_schema** into **79f29f4af3db8ee6815eb72974092b31147e8358 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] LebronAl commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r704245791
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
+ List<String> measurement_list = new ArrayList<>();
+ measurement_list.add("s1");
+ measurement_list.add("s2");
+ measurement_list.add("s3");
+
+ List<TSDataType> type_list = new ArrayList<>();
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+
+ List<Object> value_list = new ArrayList<>();
+ value_list.add(1L);
+ value_list.add(2L);
+ value_list.add(3L);
+
+ for (int i = 0; i < 5; i++) {
+ String sg = "root.sg" + String.valueOf(i);
+ session.setStorageGroup(sg);
+ for (int j = 0; j < 10; j++) {
+ session.createTimeseries(
+ String.format("%s.d1.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d2.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d3.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d4.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ }
+ }
+
+ // step 1: insert into existing time series.
+ for (int i = 0; i < 5; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ List<List<String>> measurements_list = new ArrayList<>();
+ List<List<Object>> values_list = new ArrayList<>();
+ List<List<TSDataType>> types_List = new ArrayList<>();
+ List<String> device_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ types_List.add(type_list);
+ measurements_list.add(measurement_list);
+ values_list.add(value_list);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
Review comment:
Why doing same work 5 times?
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706947289
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/CreateMultiTimeSeriesPlan.java
##########
@@ -144,6 +144,11 @@ public void setIndexes(List<Integer> indexes) {
return results;
}
+ @Override
+ public List<PartialPath> getPrefixPaths() {
+ return null;
Review comment:
So please return `Collections.emptyList()` rather than `null`.There is no doubt that returning`null` is not elegant.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r708764828
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -219,10 +219,9 @@ private void processPlanWithTolerance(InsertPlan plan, DataGroupMember dataGroup
pullTimeseriesSchema(plan, dataGroupMember.getHeader());
plan.recoverFromFailure();
getQueryExecutor().processNonQuery(plan);
- } else {
- throw e;
}
- } else throw e;
+ }
+ throw e;
Review comment:
Maybe these two `throws` is difficult to merge, but there's definitely something wrong with this `throws` ? so will it be thrown again when it executes correctly?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -204,11 +202,11 @@ public DataGroupMember(PartitionGroup nodes) {
allNodes = nodes;
setQueryManager(new ClusterQueryManager());
slotManager = new SlotManager(ClusterConstant.SLOT_NUM, getMemberDir(), getName());
+ singleReplicaApplier = new DataLogApplier(metaGroupMember, this);
Review comment:
can we use `applier` directly? What's their difference?
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42879330)
Coverage increased (+0.2%) to 67.533% when pulling **45ae0b99ac957f1e5298dac2184060b871b9f50d on MrQuansy:auto_create_schema** into **184fd6fc4176987bb8bdb1426644f00d126f0ed6 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] MrQuansy commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
MrQuansy commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706855749
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -816,6 +828,22 @@ private boolean createTimeseriesForFailedInsertion(InsertPlan plan)
}
}
+ if (plan instanceof InsertRowsPlan) {
+ for (InsertRowPlan insertPlan : ((InsertRowsPlan) plan).getInsertRowPlanList()) {
+ if (insertPlan.getFailedMeasurements() != null) {
+ insertPlan.getPlanFromFailed();
+ }
+ }
+ }
+
+ if (plan instanceof InsertRowsOfOneDevicePlan) {
+ for (InsertRowPlan insertPlan : ((InsertRowsOfOneDevicePlan) plan).getRowPlans()) {
+ if (insertPlan.getFailedMeasurements() != null) {
+ insertPlan.getPlanFromFailed();
Review comment:
The method getPlanFromFailed can only get failed measurements without successful ones, so fewer time series will be processed afterwards.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706944351
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -705,6 +700,22 @@ public boolean createTimeseries(InsertRowsPlan insertRowsPlan)
return allSuccess;
}
+ public boolean createTimeseries(InsertRowsOfOneDevicePlan insertRowsOfOneDevicePlan)
Review comment:
Sorry, My mistake. I mean you can use `CreateMultiTimeSeries` once rather than `createTimeSeries` many times
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42686621)
Coverage increased (+0.01%) to 67.387% when pulling **383a0705f552254a9b1f0e6a2aa587754e76e624 on MrQuansy:auto_create_schema** 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] LebronAl commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706952503
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertRowPlan.java
##########
@@ -414,27 +414,31 @@ private void putValues(DataOutputStream outputStream) throws QueryProcessExcepti
ReadWriteIOUtils.write((String) values[i], outputStream);
} else {
ReadWriteIOUtils.write(dataTypes[i], outputStream);
- switch (dataTypes[i]) {
- case BOOLEAN:
- ReadWriteIOUtils.write((Boolean) values[i], outputStream);
- break;
- case INT32:
- ReadWriteIOUtils.write((Integer) values[i], outputStream);
- break;
- case INT64:
- ReadWriteIOUtils.write((Long) values[i], outputStream);
- break;
- case FLOAT:
- ReadWriteIOUtils.write((Float) values[i], outputStream);
- break;
- case DOUBLE:
- ReadWriteIOUtils.write((Double) values[i], outputStream);
- break;
- case TEXT:
- ReadWriteIOUtils.write((Binary) values[i], outputStream);
- break;
- default:
- throw new QueryProcessException("Unsupported data type:" + dataTypes[i]);
+ if (isNeedInferType) {
+ ReadWriteIOUtils.write(values[i].toString(), outputStream);
Review comment:
This is an optimization that can be done on our standalone version as well, otherwise it would be a waste of time to have to refer it again after restarting from WAL. In that case, that doesn't have much to do with the PR. You can do this logic for now, but you need to make it complete. For example, implement the same logic for "private void putValues(ByteBuffer buffer)", plus serialize/deserialize UT for your case.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42681943)
Coverage increased (+0.003%) to 67.379% when pulling **318d6c00356b28e1493d6495ee6171bcacf80afb on MrQuansy:auto_create_schema** 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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42628995)
Coverage decreased (-0.1%) to 67.205% when pulling **1111c07e69b3d1d3cd68163dd3c7952d2c814991 on MrQuansy:auto_create_schema** into **11d64b8c601b3bd4e219542905d74546999e8399 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] LebronAl commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706949702
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -776,14 +774,28 @@ private TSStatus executeNonQueryPlanWithKnownLeader(PhysicalPlan plan) {
boolean hasCreated = false;
try {
if (plan instanceof InsertPlan
- && status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()
&& ClusterDescriptor.getInstance().getConfig().isEnableAutoCreateSchema()) {
- hasCreated = createTimeseriesForFailedInsertion(((InsertPlan) plan));
+ if (plan instanceof InsertRowsPlan || plan instanceof InsertMultiTabletPlan) {
Review comment:
It doesn't matter how many lines of code you have. In my opinion, these ten or so lines of code are lower than processPlanLocally, createTimeseries, etc. If you don't think so, then it's okay with me. It's about everyone's code style.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r704535576
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
+ List<String> measurement_list = new ArrayList<>();
+ measurement_list.add("s1");
+ measurement_list.add("s2");
+ measurement_list.add("s3");
+
+ List<TSDataType> type_list = new ArrayList<>();
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+
+ List<Object> value_list = new ArrayList<>();
+ value_list.add(1L);
+ value_list.add(2L);
+ value_list.add(3L);
+
+ for (int i = 0; i < 5; i++) {
+ String sg = "root.sg" + String.valueOf(i);
+ session.setStorageGroup(sg);
+ for (int j = 0; j < 10; j++) {
+ session.createTimeseries(
+ String.format("%s.d1.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d2.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d3.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d4.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ }
+ }
+
+ // step 1: insert into existing time series.
+ for (int i = 0; i < 5; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ List<List<String>> measurements_list = new ArrayList<>();
+ List<List<Object>> values_list = new ArrayList<>();
+ List<List<TSDataType>> types_List = new ArrayList<>();
+ List<String> device_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ types_List.add(type_list);
+ measurements_list.add(measurement_list);
+ values_list.add(value_list);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ List<IMeasurementSchema> schemaList = new ArrayList<>();
+ schemaList.add(new MeasurementSchema("s1", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s2", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s3", TSDataType.INT64));
+
+ Map<String, Tablet> tabletMap = new HashMap<>();
+ for (int i = 0; i < 5; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
+ // step 2: test auto create sg and time series schema
+ for (int i = 5; i < 10; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ device_list.clear();
+ for (int i = 5; i < 10; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ tabletMap.clear();
+ for (int i = 5; i < 10; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
Review comment:
where are the cases that sg exists but device does not exist?
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42567936)
Coverage decreased (-0.009%) to 67.361% when pulling **bc312d889e5841cb6ce0c699f009c73d822dffa2 on MrQuansy:auto_create_schema** into **79f29f4af3db8ee6815eb72974092b31147e8358 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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42567766)
Coverage decreased (-0.009%) to 67.361% when pulling **bc312d889e5841cb6ce0c699f009c73d822dffa2 on MrQuansy:auto_create_schema** into **79f29f4af3db8ee6815eb72974092b31147e8358 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] LebronAl commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r705822910
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/BaseApplier.java
##########
@@ -159,24 +201,28 @@ private void processPlanWithTolerance(InsertPlan plan, DataGroupMember dataGroup
throws QueryProcessException, StorageGroupNotSetException, StorageEngineException {
try {
getQueryExecutor().processNonQuery(plan);
+ } catch (BatchProcessException e) {
+ handleBatchProcessException(e, plan, dataGroupMember);
} catch (QueryProcessException | StorageGroupNotSetException | StorageEngineException e) {
- // check if this is caused by metadata missing, if so, pull metadata and retry
- Throwable metaMissingException = SchemaUtils.findMetaMissingException(e);
- boolean causedByPathNotExist = metaMissingException instanceof PathNotExistException;
-
- if (causedByPathNotExist) {
- if (logger.isDebugEnabled()) {
- logger.debug(
- "Timeseries is not found locally[{}], try pulling it from another group: {}",
- metaGroupMember.getName(),
- e.getCause().getMessage());
+ if (IoTDBDescriptor.getInstance().getConfig().isEnablePartition()) {
+ // check if this is caused by metadata missing, if so, pull metadata and retry
+ Throwable metaMissingException = SchemaUtils.findMetaMissingException(e);
+ boolean causedByPathNotExist = metaMissingException instanceof PathNotExistException;
+
+ if (causedByPathNotExist) {
+ if (logger.isDebugEnabled()) {
+ logger.debug(
+ "Timeseries is not found locally[{}], try pulling it from another group: {}",
+ metaGroupMember.getName(),
+ e.getCause().getMessage());
+ }
+ pullTimeseriesSchema(plan, dataGroupMember.getHeader());
+ plan.recoverFromFailure();
+ getQueryExecutor().processNonQuery(plan);
+ } else {
+ throw e;
}
- pullTimeseriesSchema(plan, dataGroupMember.getHeader());
- plan.recoverFromFailure();
- getQueryExecutor().processNonQuery(plan);
- } else {
- throw e;
- }
+ } else throw e;
Review comment:
merge these "throw e"
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -705,6 +700,22 @@ public boolean createTimeseries(InsertRowsPlan insertRowsPlan)
return allSuccess;
}
+ public boolean createTimeseries(InsertRowsOfOneDevicePlan insertRowsOfOneDevicePlan)
Review comment:
All device in one `InsertRowsOfOneDevicePlan` are equal, so you only need to createTimeSeries for once~
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertRowPlan.java
##########
@@ -323,7 +323,7 @@ public void markFailedMeasurementInsertion(int index, Exception e) {
}
failedValues.add(values[index]);
values[index] = null;
- dataTypes[index] = null;
+ // dataTypes[index] = null;
Review comment:
Do not comment codes.Just remove it
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -696,40 +699,35 @@ public TSStatus executeNonQueryPlan(PhysicalPlan plan) {
}
handleChangeMembershipLogWithoutRaft(log);
} else {
- getLocalExecutor().processNonQuery(plan);
+ singleReplicaApplier.applyPhysicalPlan(plan);
}
return StatusUtils.OK;
} catch (Exception e) {
Throwable cause = IOUtils.getRootCause(e);
- if (cause instanceof StorageGroupNotSetException
- || cause instanceof UndefinedTemplateException) {
- try {
- metaGroupMember.syncLeaderWithConsistencyCheck(true);
- if (plan instanceof InsertPlan && ((InsertPlan) plan).getFailedMeasurements() != null) {
- ((InsertPlan) plan).recoverFromFailure();
- }
- getLocalExecutor().processNonQuery(plan);
- return StatusUtils.OK;
- } catch (CheckConsistencyException ce) {
- return StatusUtils.getStatus(StatusUtils.CONSISTENCY_FAILURE, ce.getMessage());
- } catch (Exception ne) {
- return handleLogExecutionException(plan, IOUtils.getRootCause(ne));
- }
- }
- if (cause instanceof PathNotExistException) {
- boolean hasCreated = false;
- try {
- if (plan instanceof InsertPlan
- && ClusterDescriptor.getInstance().getConfig().isEnableAutoCreateSchema()) {
+ boolean hasCreated = false;
+ try {
+ if (plan instanceof InsertPlan
+ && ClusterDescriptor.getInstance().getConfig().isEnableAutoCreateSchema()) {
+ if (plan instanceof InsertRowsPlan || plan instanceof InsertMultiTabletPlan) {
+ if (e instanceof BatchProcessException) {
+ for (TSStatus status : ((BatchProcessException) e).getFailingStatus()) {
+ if (status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()) {
+ hasCreated = createTimeseriesForFailedInsertion(((InsertPlan) plan));
+ ((BatchPlan) plan).getResults().clear();
+ break;
+ }
+ }
+ }
+ } else if (cause instanceof PathNotExistException) {
Review comment:
looks like the `isEnableAutoCreateSchema` parameter in this if branch is turned off?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/log/applier/SingleReplicaDataApplier.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.cluster.log.applier;
+
+import org.apache.iotdb.cluster.log.Log;
+import org.apache.iotdb.cluster.server.member.DataGroupMember;
+import org.apache.iotdb.cluster.server.member.MetaGroupMember;
+import org.apache.iotdb.db.exception.StorageEngineException;
+import org.apache.iotdb.db.exception.metadata.StorageGroupNotSetException;
+import org.apache.iotdb.db.exception.query.QueryProcessException;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+import org.apache.iotdb.db.qp.physical.crud.InsertMultiTabletPlan;
+import org.apache.iotdb.db.qp.physical.crud.InsertPlan;
+import org.apache.iotdb.db.qp.physical.crud.InsertRowsPlan;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SingleReplicaDataApplier extends DataLogApplier {
+ private static final Logger logger = LoggerFactory.getLogger(SingleReplicaDataApplier.class);
+
+ public SingleReplicaDataApplier(
+ MetaGroupMember metaGroupMember, DataGroupMember dataGroupMember) {
+ super(metaGroupMember, dataGroupMember);
+ }
+
+ @Override
+ public void apply(Log log) {}
+
+ public void applyPhysicalPlan(PhysicalPlan plan)
+ throws QueryProcessException, StorageGroupNotSetException, StorageEngineException {
+ if (plan instanceof InsertMultiTabletPlan) {
Review comment:
One problem and one enhancement:
- Problem : When the replicaNum is 1, we don't actually need to check for the existence of storage groups for each plan, so I recommend checking if the current version is cluster for `applyInsert`, and if not, just skip it. You can use `IoTDB.isClusterMode()` to check.
- Enhancement : You'd better add this bug-fix logic in [PR3439](https://github.com/apache/iotdb/pull/3439) for SingleReplicaDataApplier.You can refer to [here](https://github.com/apache/iotdb/blob/master/cluster/src/main/java/org/apache/iotdb/cluster/log/applier/DataLogApplier.java#L84) for related code. 5~6 lines code is enough
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -776,14 +774,28 @@ private TSStatus executeNonQueryPlanWithKnownLeader(PhysicalPlan plan) {
boolean hasCreated = false;
try {
if (plan instanceof InsertPlan
- && status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()
&& ClusterDescriptor.getInstance().getConfig().isEnableAutoCreateSchema()) {
- hasCreated = createTimeseriesForFailedInsertion(((InsertPlan) plan));
+ if (plan instanceof InsertRowsPlan || plan instanceof InsertMultiTabletPlan) {
Review comment:
How about put these 10 lines to a new function?
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertRowPlan.java
##########
@@ -414,27 +414,31 @@ private void putValues(DataOutputStream outputStream) throws QueryProcessExcepti
ReadWriteIOUtils.write((String) values[i], outputStream);
} else {
ReadWriteIOUtils.write(dataTypes[i], outputStream);
- switch (dataTypes[i]) {
- case BOOLEAN:
- ReadWriteIOUtils.write((Boolean) values[i], outputStream);
- break;
- case INT32:
- ReadWriteIOUtils.write((Integer) values[i], outputStream);
- break;
- case INT64:
- ReadWriteIOUtils.write((Long) values[i], outputStream);
- break;
- case FLOAT:
- ReadWriteIOUtils.write((Float) values[i], outputStream);
- break;
- case DOUBLE:
- ReadWriteIOUtils.write((Double) values[i], outputStream);
- break;
- case TEXT:
- ReadWriteIOUtils.write((Binary) values[i], outputStream);
- break;
- default:
- throw new QueryProcessException("Unsupported data type:" + dataTypes[i]);
+ if (isNeedInferType) {
+ ReadWriteIOUtils.write(values[i].toString(), outputStream);
Review comment:
Why add this? Can we just infer types once? It look like the type needs to be inferred again after the change, whether it is restarted or sent to other nodes?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -816,6 +828,22 @@ private boolean createTimeseriesForFailedInsertion(InsertPlan plan)
}
}
+ if (plan instanceof InsertRowsPlan) {
+ for (InsertRowPlan insertPlan : ((InsertRowsPlan) plan).getInsertRowPlanList()) {
+ if (insertPlan.getFailedMeasurements() != null) {
+ insertPlan.getPlanFromFailed();
+ }
+ }
+ }
+
+ if (plan instanceof InsertRowsOfOneDevicePlan) {
+ for (InsertRowPlan insertPlan : ((InsertRowsOfOneDevicePlan) plan).getRowPlans()) {
+ if (insertPlan.getFailedMeasurements() != null) {
+ insertPlan.getPlanFromFailed();
Review comment:
What's the difference for `getPlanFromFailed` and `recoverFromFailure`? I remember we used `recoverFromFailure` in `Coordinator` before.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -206,6 +208,7 @@ public DataGroupMember(PartitionGroup nodes) {
if (ClusterDescriptor.getInstance().getConfig().isUseAsyncApplier()) {
applier = new AsyncDataLogApplier(applier, name);
}
+ singleReplicaApplier = new SingleReplicaDataApplier(metaGroupMember, this);
Review comment:
Maybe we can skip `applier,logManager,term,voteFor`s initialization when replicaNum == 1?
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/CreateMultiTimeSeriesPlan.java
##########
@@ -144,6 +144,11 @@ public void setIndexes(List<Integer> indexes) {
return results;
}
+ @Override
+ public List<PartialPath> getPrefixPaths() {
+ return null;
Review comment:
Never return `null`! You can use `Collections.emptyList()`.BTW, why not return true `prefixPaths` for `CreateMultiTimeSeriesPlan` ?
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706948444
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -206,6 +208,7 @@ public DataGroupMember(PartitionGroup nodes) {
if (ClusterDescriptor.getInstance().getConfig().isUseAsyncApplier()) {
applier = new AsyncDataLogApplier(applier, name);
}
+ singleReplicaApplier = new SingleReplicaDataApplier(metaGroupMember, this);
Review comment:
OK, there are also other things we can do under the single replica, such as closing thread pools that are not needed.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r704242634
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
+ List<String> measurement_list = new ArrayList<>();
+ measurement_list.add("s1");
+ measurement_list.add("s2");
+ measurement_list.add("s3");
+
+ List<TSDataType> type_list = new ArrayList<>();
Review comment:
Since all APIs are using camel case nomenclature, you'd better keep consistent
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
Review comment:
You'd better disable the CacheLeader directly in the E2E module, not just in this test. I will recommend that you use the Session.Builder method to create session instead of adding setup methods.
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
+ List<String> measurement_list = new ArrayList<>();
+ measurement_list.add("s1");
+ measurement_list.add("s2");
+ measurement_list.add("s3");
+
+ List<TSDataType> type_list = new ArrayList<>();
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+
+ List<Object> value_list = new ArrayList<>();
+ value_list.add(1L);
+ value_list.add(2L);
+ value_list.add(3L);
+
+ for (int i = 0; i < 5; i++) {
+ String sg = "root.sg" + String.valueOf(i);
+ session.setStorageGroup(sg);
+ for (int j = 0; j < 10; j++) {
+ session.createTimeseries(
+ String.format("%s.d1.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d2.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d3.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d4.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ }
+ }
+
+ // step 1: insert into existing time series.
+ for (int i = 0; i < 5; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ List<List<String>> measurements_list = new ArrayList<>();
+ List<List<Object>> values_list = new ArrayList<>();
+ List<List<TSDataType>> types_List = new ArrayList<>();
+ List<String> device_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ types_List.add(type_list);
+ measurements_list.add(measurement_list);
+ values_list.add(value_list);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
Review comment:
Why doing same work 5 times?
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
+ List<String> measurement_list = new ArrayList<>();
+ measurement_list.add("s1");
+ measurement_list.add("s2");
+ measurement_list.add("s3");
+
+ List<TSDataType> type_list = new ArrayList<>();
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+
+ List<Object> value_list = new ArrayList<>();
+ value_list.add(1L);
+ value_list.add(2L);
+ value_list.add(3L);
+
+ for (int i = 0; i < 5; i++) {
+ String sg = "root.sg" + String.valueOf(i);
+ session.setStorageGroup(sg);
+ for (int j = 0; j < 10; j++) {
+ session.createTimeseries(
+ String.format("%s.d1.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d2.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d3.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d4.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ }
+ }
+
+ // step 1: insert into existing time series.
+ for (int i = 0; i < 5; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ List<List<String>> measurements_list = new ArrayList<>();
+ List<List<Object>> values_list = new ArrayList<>();
+ List<List<TSDataType>> types_List = new ArrayList<>();
+ List<String> device_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ types_List.add(type_list);
+ measurements_list.add(measurement_list);
+ values_list.add(value_list);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ List<IMeasurementSchema> schemaList = new ArrayList<>();
+ schemaList.add(new MeasurementSchema("s1", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s2", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s3", TSDataType.INT64));
+
+ Map<String, Tablet> tabletMap = new HashMap<>();
+ for (int i = 0; i < 5; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
+ // step 2: test auto create sg and time series schema
+ for (int i = 5; i < 10; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ device_list.clear();
+ for (int i = 5; i < 10; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ tabletMap.clear();
+ for (int i = 5; i < 10; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
+ for (Statement readStatement : readStatements) {
+ for (int i = 0; i < 10; i++) {
+ for (int d = 1; d <= 4; d++) {
+ ResultSet resultSet =
+ readStatement.executeQuery(String.format("SELECT s1,s2,s3 from root.sg%s.d%s", i, d));
+ for (long t = 0; t < 3; t++) {
+ Assert.assertTrue(resultSet.next());
+ Assert.assertEquals(resultSet.getLong(1), t);
+ Assert.assertEquals(resultSet.getString(2), "1");
+ Assert.assertEquals(resultSet.getString(3), "2");
+ Assert.assertEquals(resultSet.getString(4), "3");
+ }
+ }
+ }
+ }
+
+ // test create time series without sg
+ for (int i = 0; i < 5; i++) {
+ session.createTimeseries(
+ String.format("root.sg1%s.d1.s1", i),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ }
+
+ List<String> path = new ArrayList<>();
+ List<TSDataType> dataTypes = new ArrayList<>();
+ List<TSEncoding> encodings = new ArrayList<>();
+ List<CompressionType> compressionTypes = new ArrayList<>();
+ for (int i = 5; i < 10; i++) {
+ path.add(String.format("root.sg1%s.d1.s1", i));
+ dataTypes.add(TSDataType.INT64);
+ encodings.add(TSEncoding.RLE);
+ compressionTypes.add(CompressionType.SNAPPY);
+ }
+ session.createMultiTimeseries(
+ path, dataTypes, encodings, compressionTypes, null, null, null, null);
+ for (Statement readStatement : readStatements) {
+ for (int i = 0; i < 10; i++) {
+ ResultSet resultSet =
+ readStatement.executeQuery(String.format("show timeseries root.sg1%s.d1.s1", i));
+ Assert.assertTrue(resultSet.next());
+ }
+ }
+
+ // other insert cases
+ List<Long> time_list = new ArrayList<>();
+ List<List<String>> measurementsList = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add((long) i);
+ List<String> measurements = new ArrayList<>();
+ measurements.add(String.format("s%d", i));
+ measurements.add(String.format("s%d", i + 5));
+ measurements.add(String.format("s%d", i + 10));
+ measurementsList.add(measurements);
+ }
+ session.insertRecordsOfOneDevice(
Review comment:
Why don't you put them in front like all the others?
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
+ List<String> measurement_list = new ArrayList<>();
+ measurement_list.add("s1");
+ measurement_list.add("s2");
+ measurement_list.add("s3");
+
+ List<TSDataType> type_list = new ArrayList<>();
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+
+ List<Object> value_list = new ArrayList<>();
+ value_list.add(1L);
+ value_list.add(2L);
+ value_list.add(3L);
+
+ for (int i = 0; i < 5; i++) {
+ String sg = "root.sg" + String.valueOf(i);
+ session.setStorageGroup(sg);
+ for (int j = 0; j < 10; j++) {
+ session.createTimeseries(
+ String.format("%s.d1.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d2.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d3.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d4.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ }
+ }
+
+ // step 1: insert into existing time series.
+ for (int i = 0; i < 5; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ List<List<String>> measurements_list = new ArrayList<>();
+ List<List<Object>> values_list = new ArrayList<>();
+ List<List<TSDataType>> types_List = new ArrayList<>();
+ List<String> device_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ types_List.add(type_list);
+ measurements_list.add(measurement_list);
+ values_list.add(value_list);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ List<IMeasurementSchema> schemaList = new ArrayList<>();
+ schemaList.add(new MeasurementSchema("s1", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s2", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s3", TSDataType.INT64));
+
+ Map<String, Tablet> tabletMap = new HashMap<>();
+ for (int i = 0; i < 5; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
+ // step 2: test auto create sg and time series schema
+ for (int i = 5; i < 10; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ device_list.clear();
+ for (int i = 5; i < 10; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ tabletMap.clear();
+ for (int i = 5; i < 10; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
+ for (Statement readStatement : readStatements) {
+ for (int i = 0; i < 10; i++) {
+ for (int d = 1; d <= 4; d++) {
+ ResultSet resultSet =
+ readStatement.executeQuery(String.format("SELECT s1,s2,s3 from root.sg%s.d%s", i, d));
+ for (long t = 0; t < 3; t++) {
+ Assert.assertTrue(resultSet.next());
+ Assert.assertEquals(resultSet.getLong(1), t);
+ Assert.assertEquals(resultSet.getString(2), "1");
+ Assert.assertEquals(resultSet.getString(3), "2");
+ Assert.assertEquals(resultSet.getString(4), "3");
+ }
+ }
+ }
+ }
+
+ // test create time series without sg
Review comment:
test create time series with sg
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -610,4 +614,222 @@ public void SetSystemReadOnlyWritableTest() throws SQLException {
writeStatement.execute(setWritable);
}
+
+ @Test
+ public void testAutoCreateSchemaInClusterMode()
+ throws IoTDBConnectionException, StatementExecutionException, SQLException {
+ session.setEnableCacheLeader(false);
+ List<String> measurement_list = new ArrayList<>();
+ measurement_list.add("s1");
+ measurement_list.add("s2");
+ measurement_list.add("s3");
+
+ List<TSDataType> type_list = new ArrayList<>();
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+ type_list.add(TSDataType.INT64);
+
+ List<Object> value_list = new ArrayList<>();
+ value_list.add(1L);
+ value_list.add(2L);
+ value_list.add(3L);
+
+ for (int i = 0; i < 5; i++) {
+ String sg = "root.sg" + String.valueOf(i);
+ session.setStorageGroup(sg);
+ for (int j = 0; j < 10; j++) {
+ session.createTimeseries(
+ String.format("%s.d1.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d2.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d3.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ session.createTimeseries(
+ String.format("%s.d4.s%s", sg, j),
+ TSDataType.INT64,
+ TSEncoding.RLE,
+ CompressionType.SNAPPY);
+ }
+ }
+
+ // step 1: insert into existing time series.
+ for (int i = 0; i < 5; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ List<List<String>> measurements_list = new ArrayList<>();
+ List<List<Object>> values_list = new ArrayList<>();
+ List<List<TSDataType>> types_List = new ArrayList<>();
+ List<String> device_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ types_List.add(type_list);
+ measurements_list.add(measurement_list);
+ values_list.add(value_list);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ List<IMeasurementSchema> schemaList = new ArrayList<>();
+ schemaList.add(new MeasurementSchema("s1", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s2", TSDataType.INT64));
+ schemaList.add(new MeasurementSchema("s3", TSDataType.INT64));
+
+ Map<String, Tablet> tabletMap = new HashMap<>();
+ for (int i = 0; i < 5; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
+ // step 2: test auto create sg and time series schema
+ for (int i = 5; i < 10; i++) {
+ for (long t = 0; t < 3; t++) {
+ session.insertRecord(
+ String.format("root.sg%s.d1", i), t, measurement_list, type_list, 1L, 2L, 3L);
+ }
+ }
+
+ device_list.clear();
+ for (int i = 5; i < 10; i++) {
+ String device_path = String.format("root.sg%s.d2", i);
+ device_list.add(device_path);
+ }
+
+ for (long t = 0; t < 3; t++) {
+ List<Long> time_list = new ArrayList<>();
+ for (int i = 0; i < 5; i++) {
+ time_list.add(t);
+ }
+ session.insertRecords(device_list, time_list, measurements_list, types_List, values_list);
+ }
+
+ tabletMap.clear();
+ for (int i = 5; i < 10; i++) {
+ Tablet tablet = new Tablet(String.format("root.sg%s.d3", i), schemaList, 10);
+ for (long row = 0; row < 3; row++) {
+ int rowIndex = tablet.rowSize++;
+ tablet.addTimestamp(rowIndex, row);
+ tablet.addValue("s1", rowIndex, 1L);
+ tablet.addValue("s2", rowIndex, 2L);
+ tablet.addValue("s3", rowIndex, 3L);
+ }
+ session.insertTablet(tablet);
+ tablet.setPrefixPath(String.format("root.sg%s.d4", i));
+ tabletMap.put(String.format("root.sg%s.d4", i), tablet);
+ }
+
+ session.insertTablets(tabletMap);
+
Review comment:
where are the cases that sg exists but device does not exist?
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl merged pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879
--
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] MrQuansy commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
MrQuansy commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706946028
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -206,6 +208,7 @@ public DataGroupMember(PartitionGroup nodes) {
if (ClusterDescriptor.getInstance().getConfig().isUseAsyncApplier()) {
applier = new AsyncDataLogApplier(applier, name);
}
+ singleReplicaApplier = new SingleReplicaDataApplier(metaGroupMember, this);
Review comment:
Maybe I can achieve this using another pr.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42865322)
Coverage decreased (-0.01%) to 67.376% when pulling **bc4cf8eb396a44ef01d8a6744844a94b5155f71d on MrQuansy:auto_create_schema** into **511cb00e948a3dfa9d435daf8ffa4ba9a5d9317f 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] MrQuansy commented on a change in pull request #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
MrQuansy commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706859591
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/InsertRowPlan.java
##########
@@ -414,27 +414,31 @@ private void putValues(DataOutputStream outputStream) throws QueryProcessExcepti
ReadWriteIOUtils.write((String) values[i], outputStream);
} else {
ReadWriteIOUtils.write(dataTypes[i], outputStream);
- switch (dataTypes[i]) {
- case BOOLEAN:
- ReadWriteIOUtils.write((Boolean) values[i], outputStream);
- break;
- case INT32:
- ReadWriteIOUtils.write((Integer) values[i], outputStream);
- break;
- case INT64:
- ReadWriteIOUtils.write((Long) values[i], outputStream);
- break;
- case FLOAT:
- ReadWriteIOUtils.write((Float) values[i], outputStream);
- break;
- case DOUBLE:
- ReadWriteIOUtils.write((Double) values[i], outputStream);
- break;
- case TEXT:
- ReadWriteIOUtils.write((Binary) values[i], outputStream);
- break;
- default:
- throw new QueryProcessException("Unsupported data type:" + dataTypes[i]);
+ if (isNeedInferType) {
+ ReadWriteIOUtils.write(values[i].toString(), outputStream);
Review comment:
You can image this situation: at first the data type has been inferred (data type is double but value is string type), then when the plan needs to be serialized, it will throw the exception like "String can not cast to Double".
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#discussion_r706949702
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/DataGroupMember.java
##########
@@ -776,14 +774,28 @@ private TSStatus executeNonQueryPlanWithKnownLeader(PhysicalPlan plan) {
boolean hasCreated = false;
try {
if (plan instanceof InsertPlan
- && status.getCode() == TSStatusCode.TIMESERIES_NOT_EXIST.getStatusCode()
&& ClusterDescriptor.getInstance().getConfig().isEnableAutoCreateSchema()) {
- hasCreated = createTimeseriesForFailedInsertion(((InsertPlan) plan));
+ if (plan instanceof InsertRowsPlan || plan instanceof InsertMultiTabletPlan) {
Review comment:
It doesn't matter how many lines of code you have. In my opinion, these ten or so lines of code are in a lower lever than `processPlanLocally`, `createTimeseries`, etc. If you don't think so, then I'm okay with that. It's about everyone's code style.
--
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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42567713)
Coverage decreased (-0.01%) to 67.36% when pulling **bc312d889e5841cb6ce0c699f009c73d822dffa2 on MrQuansy:auto_create_schema** into **79f29f4af3db8ee6815eb72974092b31147e8358 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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42634166)
Coverage increased (+0.03%) to 67.25% when pulling **3504fe7f792eadb58d62c35b36802dd99d045c05 on MrQuansy:auto_create_schema** into **bffb7999d5197473c77207b27553826b2720b88d 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 #3879: [IOTDB-1484]Auto create schema functionality with e2e testing in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3879:
URL: https://github.com/apache/iotdb/pull/3879#issuecomment-909948586
[](https://coveralls.io/builds/42815687)
Coverage decreased (-0.009%) to 67.482% when pulling **17317a9efbf9d764911abffceda70902ee8fcd93 on MrQuansy:auto_create_schema** into **922cc09f207a52ba7e7a2f8adb5944195db19439 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