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/05/26 09:13:45 UTC
[GitHub] [iotdb] neuyilan opened a new pull request #3269: [IOTDB-1292] Versioned MTree
neuyilan opened a new pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41210019)
Coverage increased (+0.02%) to 68.178% when pulling **b330bfd82de4126a7f5e962b8c009dec8a82f761 on neuyilan:apache_master_0426_version_mtree** into **18ae729e2792c709b7054cb0de25f4d331d7558e 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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41349408)
Coverage decreased (-0.007%) to 68.169% when pulling **497b25e1a2e7090618c523924777e095d1c122e5 on neuyilan:apache_master_0426_version_mtree** into **5e020b5880a97c8530b39986b504c5f28fcbcd33 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] neuyilan commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r670972560
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -732,6 +746,16 @@ public boolean createTimeseries(InsertPlan insertPlan)
logger.error("Failed to infer storage group from deviceId {}", deviceId);
return false;
}
+
+ Pair<Long, Long> sgNodeVersion;
+ try {
+ sgNodeVersion = getSgNodeMajorVersion(storageGroupName);
+ } catch (MetadataException e) {
+ logger.error("failed get the major version", e);
+ return false;
+ }
+
+ boolean hasVector = false;
Review comment:
It may be caused by merge with master branch and have some conflicts have moved.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +768,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
if (unregisteredSeriesList.isEmpty()) {
return true;
}
- logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+ logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
- return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+ return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+ }
+
+ /**
+ * @param partialPath the storage group's partialPath
+ * @return the major version and the minor version of the storage group node
+ */
+ private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
+ StorageGroupMNode node;
+ try {
+ node = getStorageGroupNodeByPath(partialPath);
+ } catch (MetadataException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(partialPath);
+ }
+ if (node == null) {
Review comment:
yes, actually when we can not find the sg, the getStorageGroupNodeByPath will throw `MetadataException`.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/UpdateStorageGroupPlan.java
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.db.qp.physical.sys;
+
+import org.apache.iotdb.db.exception.metadata.IllegalPathException;
+import org.apache.iotdb.db.metadata.PartialPath;
+import org.apache.iotdb.db.qp.logical.Operator;
+import org.apache.iotdb.db.qp.logical.Operator.OperatorType;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class UpdateStorageGroupPlan extends PhysicalPlan {
+
+ private PartialPath path;
+
+ public UpdateStorageGroupPlan() {
+ super(false, OperatorType.UPDATE_STORAGE_GROUP);
+ }
+
+ public UpdateStorageGroupPlan(PartialPath path) {
+ super(false, Operator.OperatorType.UPDATE_STORAGE_GROUP);
+ this.path = path;
+ }
+
+ public PartialPath getPath() {
+ return path;
+ }
+
+ public void setPath(PartialPath path) {
+ this.path = path;
+ }
+
+ @Override
+ public List<PartialPath> getPaths() {
+ return path != null ? Collections.singletonList(path) : Collections.emptyList();
+ }
+
+ @Override
+ public void serialize(DataOutputStream stream) throws IOException {
+ stream.write((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(stream, path.getFullPath());
+ stream.writeLong(index);
Review comment:
done, thanks.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -368,6 +376,90 @@ public static PartitionGroup partitionByPathTimeWithSync(
return partitionGroup;
}
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ long majorVersion = 0;
+ long minorVersion = 0;
+ if (plan.getOperatorType() == null) {
+ logger.warn("the plan={} have no operator, ignore it", plan);
+ return;
+ }
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ majorVersion = currLogIndex;
+ minorVersion = 0;
+ plan.setMajorVersion(majorVersion);
+ plan.setMinorVersion(minorVersion);
+ break;
+ case DELETE_STORAGE_GROUP:
+ List<PartialPath> deletePathList = new ArrayList<>();
+ for (PartialPath storageGroupPath : plan.getPaths()) {
+ List<PartialPath> allRelatedStorageGroupPath =
+ IoTDB.metaManager.getStorageGroupPaths(storageGroupPath);
+ if (allRelatedStorageGroupPath.isEmpty()) {
+ throw new PathNotExistException(storageGroupPath.getFullPath(), true);
+ }
+ for (PartialPath path : allRelatedStorageGroupPath) {
+ StorageGroupMNode storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(path);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = storageGroupMNode.getMinorVersion();
+ path.setMajorVersion(majorVersion);
+ path.setMinorVersion(minorVersion);
+ deletePathList.add(path);
+ }
+ }
+ // replace the to be deleted paths
+ ((DeleteStorageGroupPlan) plan).setPaths(deletePathList);
+ break;
+ case CREATE_TIMESERIES:
+ PartialPath partialPath = ((CreateTimeSeriesPlan) plan).getPath();
+ StorageGroupMNode storageGroupMNode =
+ IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((CreateTimeSeriesPlan) plan).getPath().setMajorVersion(majorVersion);
+ ((CreateTimeSeriesPlan) plan).getPath().setMinorVersion(minorVersion);
+ break;
+ case ALTER_TIMESERIES:
+ partialPath = ((AlterTimeSeriesPlan) plan).getPath();
+ storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((AlterTimeSeriesPlan) plan).getPath().setMajorVersion(majorVersion);
+ ((AlterTimeSeriesPlan) plan).getPath().setMinorVersion(minorVersion);
+ break;
+ case CREATE_ALIGNED_TIMESERIES:
+ partialPath = ((CreateAlignedTimeSeriesPlan) plan).getPrefixPath();
+ storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((CreateAlignedTimeSeriesPlan) plan).getPrefixPath().setMajorVersion(majorVersion);
+ ((CreateAlignedTimeSeriesPlan) plan).getPrefixPath().setMinorVersion(minorVersion);
+ break;
+ case CREATE_MULTI_TIMESERIES:
+ case DELETE_TIMESERIES:
+ List<PartialPath> partialPaths = plan.getPaths();
+ for (PartialPath path : partialPaths) {
Review comment:
The path is get from the plan, so here the plan's path version have been chaned.
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -36,23 +36,32 @@
import org.junit.Assert;
import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
public abstract class Cases {
+ private static Logger logger = LoggerFactory.getLogger(Cases.class);
protected Statement writeStatement;
protected Connection writeConnection;
protected Statement[] readStatements;
protected Connection[] readConnections;
protected Session session;
+ protected Session antherSession;
Review comment:
done
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1874,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MajorVersionNotEqualException sync meta leader failed and the
+ * @throws StorageGroupNotSetException some error occurred.
+ */
+ private boolean checkSgNodeAndPlanMajorVersion(PartialPath path)
+ throws MetadataException, StorageGroupNotSetException {
+ StorageGroupMNode node = null;
+ try {
+ node = getStorageGroupNodeByPath(path);
+ } catch (StorageGroupNotSetException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
+ long nodeMajorVersion = node.getMajorVersion();
+ if (nodeMajorVersion < path.getMajorVersion()) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
+ nodeMajorVersion = node.getMajorVersion();
+ if (nodeMajorVersion < path.getMajorVersion()) {
+ logger.error(
Review comment:
Thanks for bringing this problem, this problem is caused when some node apply log failed, in the previous design, it still occurs, I think we can set the apply thread is blocked when apply one log and only when it apply success we can move the log to the committed's logs from the uncommitted logs, the success means it's should be applied(apply success or failed is both ok). I think we can do this work in another thread. How do you think?
##########
File path: server/src/main/java/org/apache/iotdb/db/tools/mlog/MLogParser.java
##########
@@ -175,6 +176,10 @@ public static void parseFromFile(String inputFile, String outputFile) throws IOE
case SET_STORAGE_GROUP:
mLogTxtWriter.setStorageGroup(((SetStorageGroupPlan) plan).getPath().getFullPath());
break;
+ case UPDATE_STORAGE_GROUP:
+ mLogTxtWriter.updateStorageGroup(
Review comment:
done
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/UpdateStorageGroupPlan.java
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.db.qp.physical.sys;
+
+import org.apache.iotdb.db.exception.metadata.IllegalPathException;
+import org.apache.iotdb.db.metadata.PartialPath;
+import org.apache.iotdb.db.qp.logical.Operator;
+import org.apache.iotdb.db.qp.logical.Operator.OperatorType;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class UpdateStorageGroupPlan extends PhysicalPlan {
+
+ private PartialPath path;
+
+ public UpdateStorageGroupPlan() {
+ super(false, OperatorType.UPDATE_STORAGE_GROUP);
+ }
+
+ public UpdateStorageGroupPlan(PartialPath path) {
+ super(false, Operator.OperatorType.UPDATE_STORAGE_GROUP);
+ this.path = path;
+ }
+
+ public PartialPath getPath() {
+ return path;
+ }
+
+ public void setPath(PartialPath path) {
+ this.path = path;
+ }
+
+ @Override
+ public List<PartialPath> getPaths() {
+ return path != null ? Collections.singletonList(path) : Collections.emptyList();
+ }
+
+ @Override
+ public void serialize(DataOutputStream stream) throws IOException {
+ stream.write((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(stream, path.getFullPath());
+ stream.writeLong(index);
+ }
+
+ @Override
+ public void serialize(ByteBuffer buffer) {
+ buffer.put((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(buffer, path.getFullPath());
+ buffer.putLong(index);
+ buffer.putLong(path.getMajorVersion());
+ buffer.putLong(path.getMinorVersion());
+ }
+
+ @Override
+ public void deserialize(ByteBuffer buffer) throws IllegalPathException {
+ path = new PartialPath(readString(buffer));
+ this.index = buffer.getLong();
+ path.setMajorVersion(buffer.getLong());
+ path.setMinorVersion(buffer.getLong());
+ }
+
+ @Override
+ public String toString() {
Review comment:
The path has contains the major version and minor version, so only print the path is ok.
And the ", majorVersion=" here is redundant, thanks for your carefull review.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -368,6 +376,90 @@ public static PartitionGroup partitionByPathTimeWithSync(
return partitionGroup;
}
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ long majorVersion = 0;
+ long minorVersion = 0;
+ if (plan.getOperatorType() == null) {
+ logger.warn("the plan={} have no operator, ignore it", plan);
+ return;
+ }
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ majorVersion = currLogIndex;
+ minorVersion = 0;
+ plan.setMajorVersion(majorVersion);
+ plan.setMinorVersion(minorVersion);
+ break;
+ case DELETE_STORAGE_GROUP:
+ List<PartialPath> deletePathList = new ArrayList<>();
+ for (PartialPath storageGroupPath : plan.getPaths()) {
+ List<PartialPath> allRelatedStorageGroupPath =
+ IoTDB.metaManager.getStorageGroupPaths(storageGroupPath);
+ if (allRelatedStorageGroupPath.isEmpty()) {
+ throw new PathNotExistException(storageGroupPath.getFullPath(), true);
+ }
+ for (PartialPath path : allRelatedStorageGroupPath) {
+ StorageGroupMNode storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(path);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = storageGroupMNode.getMinorVersion();
+ path.setMajorVersion(majorVersion);
+ path.setMinorVersion(minorVersion);
+ deletePathList.add(path);
+ }
+ }
+ // replace the to be deleted paths
+ ((DeleteStorageGroupPlan) plan).setPaths(deletePathList);
+ break;
+ case CREATE_TIMESERIES:
+ PartialPath partialPath = ((CreateTimeSeriesPlan) plan).getPath();
+ StorageGroupMNode storageGroupMNode =
+ IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((CreateTimeSeriesPlan) plan).getPath().setMajorVersion(majorVersion);
+ ((CreateTimeSeriesPlan) plan).getPath().setMinorVersion(minorVersion);
+ break;
+ case ALTER_TIMESERIES:
Review comment:
Thanks for your carefully review. I have add it.
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/UpdateStorageGroupPlan.java
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.db.qp.physical.sys;
+
+import org.apache.iotdb.db.exception.metadata.IllegalPathException;
+import org.apache.iotdb.db.metadata.PartialPath;
+import org.apache.iotdb.db.qp.logical.Operator;
+import org.apache.iotdb.db.qp.logical.Operator.OperatorType;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class UpdateStorageGroupPlan extends PhysicalPlan {
+
+ private PartialPath path;
+
+ public UpdateStorageGroupPlan() {
+ super(false, OperatorType.UPDATE_STORAGE_GROUP);
+ }
+
+ public UpdateStorageGroupPlan(PartialPath path) {
+ super(false, Operator.OperatorType.UPDATE_STORAGE_GROUP);
+ this.path = path;
+ }
+
+ public PartialPath getPath() {
+ return path;
+ }
+
+ public void setPath(PartialPath path) {
+ this.path = path;
+ }
+
+ @Override
+ public List<PartialPath> getPaths() {
+ return path != null ? Collections.singletonList(path) : Collections.emptyList();
+ }
+
+ @Override
+ public void serialize(DataOutputStream stream) throws IOException {
+ stream.write((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(stream, path.getFullPath());
+ stream.writeLong(index);
+ }
+
+ @Override
+ public void serialize(ByteBuffer buffer) {
+ buffer.put((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(buffer, path.getFullPath());
+ buffer.putLong(index);
+ buffer.putLong(path.getMajorVersion());
+ buffer.putLong(path.getMinorVersion());
+ }
+
+ @Override
+ public void deserialize(ByteBuffer buffer) throws IllegalPathException {
+ path = new PartialPath(readString(buffer));
+ this.index = buffer.getLong();
+ path.setMajorVersion(buffer.getLong());
+ path.setMinorVersion(buffer.getLong());
+ }
+
+ @Override
+ public String toString() {
+ return "UpdateStorageGroupPlan{" + " path=" + path + ", majorVersion=" + "}";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+
+ UpdateStorageGroupPlan that = (UpdateStorageGroupPlan) o;
+
+ return Objects.equals(path, that.path);
Review comment:
Have added the version
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41423415)
Coverage decreased (-0.04%) to 68.114% when pulling **441e8003f53d15780bafa80e3678f43c60dfa571 on neuyilan:apache_master_0426_version_mtree** into **5102f7f672e7e15f60b5095230a75db1e44712c2 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] sonarcloud[bot] removed a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-851990576
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=coverage&view=list) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41350088)
Coverage increased (+0.005%) to 68.18% when pulling **497b25e1a2e7090618c523924777e095d1c122e5 on neuyilan:apache_master_0426_version_mtree** into **5e020b5880a97c8530b39986b504c5f28fcbcd33 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] mychaow commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r670344302
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -368,6 +376,90 @@ public static PartitionGroup partitionByPathTimeWithSync(
return partitionGroup;
}
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ long majorVersion = 0;
+ long minorVersion = 0;
+ if (plan.getOperatorType() == null) {
+ logger.warn("the plan={} have no operator, ignore it", plan);
+ return;
+ }
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ majorVersion = currLogIndex;
+ minorVersion = 0;
+ plan.setMajorVersion(majorVersion);
+ plan.setMinorVersion(minorVersion);
+ break;
+ case DELETE_STORAGE_GROUP:
+ List<PartialPath> deletePathList = new ArrayList<>();
+ for (PartialPath storageGroupPath : plan.getPaths()) {
+ List<PartialPath> allRelatedStorageGroupPath =
+ IoTDB.metaManager.getStorageGroupPaths(storageGroupPath);
+ if (allRelatedStorageGroupPath.isEmpty()) {
+ throw new PathNotExistException(storageGroupPath.getFullPath(), true);
+ }
+ for (PartialPath path : allRelatedStorageGroupPath) {
+ StorageGroupMNode storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(path);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = storageGroupMNode.getMinorVersion();
+ path.setMajorVersion(majorVersion);
+ path.setMinorVersion(minorVersion);
+ deletePathList.add(path);
+ }
+ }
+ // replace the to be deleted paths
+ ((DeleteStorageGroupPlan) plan).setPaths(deletePathList);
+ break;
+ case CREATE_TIMESERIES:
+ PartialPath partialPath = ((CreateTimeSeriesPlan) plan).getPath();
+ StorageGroupMNode storageGroupMNode =
+ IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((CreateTimeSeriesPlan) plan).getPath().setMajorVersion(majorVersion);
+ ((CreateTimeSeriesPlan) plan).getPath().setMinorVersion(minorVersion);
+ break;
+ case ALTER_TIMESERIES:
Review comment:
So, why do not check version when we alter timeseries?
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r675286745
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -732,6 +746,15 @@ public boolean createTimeseries(InsertPlan insertPlan)
logger.error("Failed to infer storage group from deviceId {}", deviceId);
return false;
}
+
+ Pair<Long, Long> sgNodeVersion;
+ try {
+ sgNodeVersion = getSgNodeMajorVersion(storageGroupName);
+ } catch (MetadataException e) {
Review comment:
StorageGroupNotSetException
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +768,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
if (unregisteredSeriesList.isEmpty()) {
return true;
}
- logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+ logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
- return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+ return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+ }
+
+ /**
+ * @param partialPath the storage group's partialPath
+ * @return the major version and the minor version of the storage group node
+ */
+ private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
+ StorageGroupMNode node;
+ try {
+ node = getStorageGroupNodeByPath(partialPath);
+ } catch (MetadataException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(partialPath);
+ }
+ if (node == null) {
Review comment:
same, seems redundant
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +767,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
if (unregisteredSeriesList.isEmpty()) {
return true;
}
- logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+ logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
- return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+ return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+ }
+
+ /**
+ * @param partialPath the storage group's partialPath
+ * @return the major version and the minor version of the storage group node
+ */
+ private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
Review comment:
change function name as we not only return `MajorVersion`?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1873,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MajorVersionNotEqualException sync meta leader failed and the
Review comment:
please fix comments
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +767,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
if (unregisteredSeriesList.isEmpty()) {
return true;
}
- logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+ logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
- return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+ return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+ }
+
+ /**
+ * @param partialPath the storage group's partialPath
+ * @return the major version and the minor version of the storage group node
+ */
+ private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
+ StorageGroupMNode node;
+ try {
+ node = getStorageGroupNodeByPath(partialPath);
+ } catch (MetadataException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(partialPath);
+ }
+ if (node == null) {
+ throw new MetadataException("get major version failed");
+ }
+ return new Pair(node.getMajorVersion(), node.getMinorVersion());
}
private boolean createAlignedTimeseries(List<String> seriesList, InsertPlan insertPlan)
Review comment:
add versions for `createAlignedTimeseries`, they ought to be same.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1873,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
Review comment:
throw MetadataException if the sg node major version < the path's major version **after syncMetaLeader**?
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r671597326
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanExecutor.java
##########
@@ -634,4 +639,39 @@ public void delete(PartialPath path, long startTime, long endTime, long planInde
throw new QueryProcessException(e);
}
}
+
+ /**
+ * the deleteStorageGroupPlan.getPaths() are all storage group paths, it have been processed in
+ * the {@link org.apache.iotdb.cluster.utils.ClusterUtils#setVersionForSpecialPlan(PhysicalPlan,
+ * long)}
+ */
+ @Override
+ protected boolean deleteStorageGroups(DeleteStorageGroupPlan deleteStorageGroupPlan)
+ throws QueryProcessException {
+ AUDIT_LOGGER.info("delete storage group {}", deleteStorageGroupPlan.getPaths());
+ try {
+ IoTDB.metaManager.deleteStorageGroups(deleteStorageGroupPlan.getPaths());
+ } catch (MetadataException e) {
+ throw new QueryProcessException(e);
+ }
+ return true;
Review comment:
why do not use super.deleteStorageGroup, I find other operate in super.deleteStorageGroup(), Not only metaManager.deleteStorageGroup...
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/SingleNodeIT.java
##########
@@ -72,6 +73,9 @@ public void init() throws Exception {
writeStatement = readStatements[0] = writeConnection.createStatement();
session = new Session("127.0.0.1", rpcPort);
session.open();
+
+ antherSession = new Session("127.0.0.1", rpcPort);
Review comment:
the same
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/RaftMember.java
##########
@@ -1032,6 +1037,27 @@ TSStatus processPlanLocally(PhysicalPlan plan) {
log.setCurrLogTerm(getTerm().get());
log.setCurrLogIndex(logManager.getLastLogIndex() + 1);
+ // Set version for some special plans
+ try {
+ ClusterUtils.setVersionForSpecialPlan(plan, log.getCurrLogIndex());
+ } catch (StorageGroupNotSetException sge) {
+ // if sg not set, sync the meta leader and try again
+ try {
+ ((CMManager) IoTDB.metaManager).syncMetaLeader();
+ ClusterUtils.setVersionForSpecialPlan(plan, log.getCurrLogIndex());
+ } catch (MetadataException e1) {
+ logger.error("process plan failed, plan={}", plan, e1);
+ TSStatus tsStatus = StatusUtils.INTERNAL_ERROR.deepCopy();
Review comment:
why does deepCopy?
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -508,4 +518,110 @@ public void testCreateSnapshot() throws InterruptedException {
Assert.assertNull(e);
}
}
+
+ // test https://issues.apache.org/jira/browse/IOTDB-1292
+ @Test
+ public void testDataRaftAndMetaRaftConcurrentOperation() {
+ ExecutorService dataRaftGroupThread = Executors.newFixedThreadPool(1);
+ ExecutorService metaRaftGroupThread = Executors.newFixedThreadPool(1);
+ createSgTask();
+ Future<?> dataFeature = dataRaftGroupThread.submit(this::createTimeSeriesTask);
+ Future<?> metaFeature = metaRaftGroupThread.submit(this::deleteSgTask);
+ try {
+ dataFeature.get();
+ metaFeature.get();
+ } catch (InterruptedException e) {
+ logger.error(e.getMessage());
+ } catch (ExecutionException e) {
+ Assert.fail(e.getMessage());
+ }
+
+ // check the final result
+ try {
+ queryFinalResult();
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ public void createTimeSeriesTask() {
+ int sgNum = 10;
+ int deviceNum = 10;
+ int measurementsNum = 100;
+ String sqlFormat =
+ "create timeseries root.sg_%d.device_%d.s_%d with datatype=INT64, encoding=PLAIN, compression=SNAPPY";
+ for (int sg = 0; sg < sgNum; sg++) {
+ for (int device = 0; device < deviceNum; device++) {
+ for (int measurement = 0; measurement < measurementsNum; measurement++) {
+ String sql = String.format(sqlFormat, sg, device, measurement);
+ try {
+ session.executeNonQueryStatement(sql);
+ } catch (StatementExecutionException e) {
+ if (e.getMessage().contains("Storage group is not set for current seriesPath")) {
+ // it's ok due to concurrent operation, do nothing.
+ } else {
+ Assert.fail(e.getMessage());
+ }
+ } catch (IoTDBConnectionException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+ }
+ }
+ }
+
+ public void createSgTask() {
Review comment:
private
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -508,4 +518,110 @@ public void testCreateSnapshot() throws InterruptedException {
Assert.assertNull(e);
}
}
+
+ // test https://issues.apache.org/jira/browse/IOTDB-1292
+ @Test
+ public void testDataRaftAndMetaRaftConcurrentOperation() {
+ ExecutorService dataRaftGroupThread = Executors.newFixedThreadPool(1);
+ ExecutorService metaRaftGroupThread = Executors.newFixedThreadPool(1);
+ createSgTask();
+ Future<?> dataFeature = dataRaftGroupThread.submit(this::createTimeSeriesTask);
+ Future<?> metaFeature = metaRaftGroupThread.submit(this::deleteSgTask);
+ try {
+ dataFeature.get();
+ metaFeature.get();
+ } catch (InterruptedException e) {
+ logger.error(e.getMessage());
+ } catch (ExecutionException e) {
+ Assert.fail(e.getMessage());
+ }
+
+ // check the final result
+ try {
+ queryFinalResult();
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ public void createTimeSeriesTask() {
Review comment:
private
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -508,4 +518,110 @@ public void testCreateSnapshot() throws InterruptedException {
Assert.assertNull(e);
}
}
+
+ // test https://issues.apache.org/jira/browse/IOTDB-1292
+ @Test
+ public void testDataRaftAndMetaRaftConcurrentOperation() {
+ ExecutorService dataRaftGroupThread = Executors.newFixedThreadPool(1);
+ ExecutorService metaRaftGroupThread = Executors.newFixedThreadPool(1);
+ createSgTask();
+ Future<?> dataFeature = dataRaftGroupThread.submit(this::createTimeSeriesTask);
+ Future<?> metaFeature = metaRaftGroupThread.submit(this::deleteSgTask);
+ try {
+ dataFeature.get();
+ metaFeature.get();
+ } catch (InterruptedException e) {
+ logger.error(e.getMessage());
+ } catch (ExecutionException e) {
+ Assert.fail(e.getMessage());
+ }
+
+ // check the final result
+ try {
+ queryFinalResult();
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ public void createTimeSeriesTask() {
+ int sgNum = 10;
+ int deviceNum = 10;
+ int measurementsNum = 100;
+ String sqlFormat =
+ "create timeseries root.sg_%d.device_%d.s_%d with datatype=INT64, encoding=PLAIN, compression=SNAPPY";
+ for (int sg = 0; sg < sgNum; sg++) {
+ for (int device = 0; device < deviceNum; device++) {
+ for (int measurement = 0; measurement < measurementsNum; measurement++) {
+ String sql = String.format(sqlFormat, sg, device, measurement);
+ try {
+ session.executeNonQueryStatement(sql);
+ } catch (StatementExecutionException e) {
+ if (e.getMessage().contains("Storage group is not set for current seriesPath")) {
+ // it's ok due to concurrent operation, do nothing.
+ } else {
+ Assert.fail(e.getMessage());
+ }
+ } catch (IoTDBConnectionException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+ }
+ }
+ }
+
+ public void createSgTask() {
+ int sgNum = 10;
+ String sqlFormat = "set storage group to root.sg_%d";
+ for (int sg = 0; sg < sgNum; sg++) {
+ String sql = String.format(sqlFormat, sg);
+ try {
+ writeStatement.execute(sql);
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+ }
+
+ public void deleteSgTask() {
Review comment:
private
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -732,6 +746,16 @@ public boolean createTimeseries(InsertPlan insertPlan)
logger.error("Failed to infer storage group from deviceId {}", deviceId);
return false;
}
+
+ Pair<Long, Long> sgNodeVersion;
+ try {
+ sgNodeVersion = getSgNodeMajorVersion(storageGroupName);
+ } catch (MetadataException e) {
+ logger.error("failed get the major version", e);
Review comment:
Good Job.
This is a grammar error, "Failed to get the major version"
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/StorageGroupMNodePlan.java
##########
@@ -30,19 +30,27 @@
import java.util.Objects;
public class StorageGroupMNodePlan extends MNodePlan {
+
private long dataTTL;
private int alignedTimeseriesIndex;
+ private long majorVersion;
+
+ private long minorVersion;
+
public StorageGroupMNodePlan() {
super(false, Operator.OperatorType.STORAGE_GROUP_MNODE);
}
- public StorageGroupMNodePlan(String name, long dataTTL, int childSize) {
Review comment:
The original interface is retained. If the majorversion and minorversion values are not setted, the default values can be used.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -732,6 +746,16 @@ public boolean createTimeseries(InsertPlan insertPlan)
logger.error("Failed to infer storage group from deviceId {}", deviceId);
return false;
}
+
+ Pair<Long, Long> sgNodeVersion;
+ try {
+ sgNodeVersion = getSgNodeMajorVersion(storageGroupName);
+ } catch (MetadataException e) {
+ logger.error("failed get the major version", e);
+ return false;
+ }
+
+ boolean hasVector = false;
Review comment:
I have the same question.
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/ClusterIT.java
##########
@@ -85,6 +85,8 @@ public void init() throws Exception {
}
session = new Session(getWriteRpcIp(), getWriteRpcPort());
session.open();
+ antherSession = new Session(getWriteRpcIp(), getWriteRpcPort());
Review comment:
anther->another
--
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] sonarcloud[bot] removed a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-852138808
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=coverage&view=list) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] mychaow commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r675252744
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +768,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
if (unregisteredSeriesList.isEmpty()) {
return true;
}
- logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+ logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
- return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+ return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+ }
+
+ /**
+ * @param partialPath the storage group's partialPath
+ * @return the major version and the minor version of the storage group node
+ */
+ private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
+ StorageGroupMNode node;
+ try {
+ node = getStorageGroupNodeByPath(partialPath);
+ } catch (MetadataException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(partialPath);
+ }
+ if (node == null) {
Review comment:
so, this judge is not necessary.
--
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] neuyilan commented on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
neuyilan commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-885333744
@mychaow @wangchao316 @jt2594838 @LebronAl Could you please take a review?
--
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] neuyilan commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r665847966
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1816,4 +1858,63 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MetadataException sync meta leader failed
Review comment:
sure, I have amend the comment
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/PartialPath.java
##########
@@ -44,6 +44,10 @@
// alias of measurement, null pointer cannot be serialized in thrift so empty string is instead
protected String measurementAlias = "";
+ private long majorVersion = 0;
+
+ private long minorVersion = 0;
+
Review comment:
done
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -353,4 +361,65 @@ public static PartitionGroup partitionByPathTimeWithSync(
}
return partitionGroup;
}
+
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ plan.setMajorVersion(currLogIndex);
+ plan.setMinorVersion(0);
+ break;
+ case DELETE_STORAGE_GROUP:
+ List<PartialPath> deletePathList = new ArrayList<>();
+ for (PartialPath storageGroupPath : plan.getPaths()) {
+ List<PartialPath> allRelatedStorageGroupPath =
+ IoTDB.metaManager.getStorageGroupPaths(storageGroupPath);
+ if (allRelatedStorageGroupPath.isEmpty()) {
+ throw new PathNotExistException(storageGroupPath.getFullPath(), true);
+ }
+ for (PartialPath path : allRelatedStorageGroupPath) {
+ StorageGroupMNode storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(path);
+ path.setMajorVersion(storageGroupMNode.getMajorVersion());
+ path.setMinorVersion(storageGroupMNode.getMinorVersion());
+ }
+ }
+ // replace the to be deleted paths
+ plan.setPaths(deletePathList);
+ break;
Review comment:
Sure, we can just set the plan's major version and can delete the sg as long as its version is smaller than that of the plan.
But, I want to unify the processing logic in the code：that is when applying one plan, only the plan's major version equals the node's major version can do it. I think this will make the code easier to understand.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/RaftMember.java
##########
@@ -974,9 +977,17 @@ TSStatus processPlanLocally(PhysicalPlan plan) {
synchronized (logManager) {
log.setCurrLogTerm(getTerm().get());
log.setCurrLogIndex(logManager.getLastLogIndex() + 1);
-
log.setPlan(plan);
plan.setIndex(log.getCurrLogIndex());
+
+ // Set version for some special plans
+ try {
+ ClusterUtils.setVersionForSpecialPlan(plan, log.getCurrLogIndex());
+ } catch (MetadataException e) {
+ logger.error("process plan failed, plan={}", plan, e);
+ return StatusUtils.INTERNAL_ERROR;
Review comment:
done
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/logfile/MLogWriter.java
##########
@@ -437,7 +448,7 @@ public static PhysicalPlan convertFromString(String str) {
CompressionType.values()[Integer.parseInt(words[5])]));
case "1":
return new StorageGroupMNodePlan(
- words[1], Long.parseLong(words[2]), Integer.parseInt(words[3]));
+ words[1], Long.parseLong(words[2]), Integer.parseInt(words[3]), 0, 0);
case "0":
Review comment:
This function is only used for upgrade, in the previous version(<0.13), the node has no versions. so there I use zero.
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/MLogTxtWriter.java
##########
@@ -166,6 +166,14 @@ public void setStorageGroup(String storageGroup) throws IOException {
lineNumber.incrementAndGet();
}
+ public void updateStorageGroup(String storageGroup) throws IOException {
+ String outputStr =
+ MetadataOperationType.UPDATE_STORAGE_GROUP + "," + storageGroup + LINE_SEPARATOR;
+ ByteBuffer buff = ByteBuffer.wrap(outputStr.getBytes());
+ channel.write(buff);
+ lineNumber.incrementAndGet();
+ }
+
Review comment:
Sure, we have serialized the versions and after a restart, the plan can be deserialized correctly, the above codes is just one parse util that parses mlog.bin to mlog.txt
--
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] mychaow commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r670317373
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -368,6 +376,90 @@ public static PartitionGroup partitionByPathTimeWithSync(
return partitionGroup;
}
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ long majorVersion = 0;
+ long minorVersion = 0;
+ if (plan.getOperatorType() == null) {
+ logger.warn("the plan={} have no operator, ignore it", plan);
+ return;
+ }
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ majorVersion = currLogIndex;
+ minorVersion = 0;
+ plan.setMajorVersion(majorVersion);
+ plan.setMinorVersion(minorVersion);
+ break;
+ case DELETE_STORAGE_GROUP:
+ List<PartialPath> deletePathList = new ArrayList<>();
+ for (PartialPath storageGroupPath : plan.getPaths()) {
+ List<PartialPath> allRelatedStorageGroupPath =
+ IoTDB.metaManager.getStorageGroupPaths(storageGroupPath);
+ if (allRelatedStorageGroupPath.isEmpty()) {
+ throw new PathNotExistException(storageGroupPath.getFullPath(), true);
+ }
+ for (PartialPath path : allRelatedStorageGroupPath) {
+ StorageGroupMNode storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(path);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = storageGroupMNode.getMinorVersion();
+ path.setMajorVersion(majorVersion);
+ path.setMinorVersion(minorVersion);
+ deletePathList.add(path);
+ }
+ }
+ // replace the to be deleted paths
+ ((DeleteStorageGroupPlan) plan).setPaths(deletePathList);
+ break;
+ case CREATE_TIMESERIES:
+ PartialPath partialPath = ((CreateTimeSeriesPlan) plan).getPath();
+ StorageGroupMNode storageGroupMNode =
+ IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((CreateTimeSeriesPlan) plan).getPath().setMajorVersion(majorVersion);
+ ((CreateTimeSeriesPlan) plan).getPath().setMinorVersion(minorVersion);
+ break;
+ case ALTER_TIMESERIES:
+ partialPath = ((AlterTimeSeriesPlan) plan).getPath();
+ storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((AlterTimeSeriesPlan) plan).getPath().setMajorVersion(majorVersion);
+ ((AlterTimeSeriesPlan) plan).getPath().setMinorVersion(minorVersion);
+ break;
+ case CREATE_ALIGNED_TIMESERIES:
+ partialPath = ((CreateAlignedTimeSeriesPlan) plan).getPrefixPath();
+ storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(partialPath);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = currLogIndex;
+ ((CreateAlignedTimeSeriesPlan) plan).getPrefixPath().setMajorVersion(majorVersion);
+ ((CreateAlignedTimeSeriesPlan) plan).getPrefixPath().setMinorVersion(minorVersion);
+ break;
+ case CREATE_MULTI_TIMESERIES:
+ case DELETE_TIMESERIES:
+ List<PartialPath> partialPaths = plan.getPaths();
+ for (PartialPath path : partialPaths) {
Review comment:
why do not replace the path of plan?
--
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] neuyilan commented on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
neuyilan commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-881844781
> hi , houliang , I I've raised a few small questions that you can take a look at.
Thanks for your careful review, @wangchao316 I have fixed the comments you posted. PTAL
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r676337661
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -368,6 +376,90 @@ public static PartitionGroup partitionByPathTimeWithSync(
return partitionGroup;
}
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ long majorVersion = 0;
+ long minorVersion = 0;
+ if (plan.getOperatorType() == null) {
+ logger.warn("the plan={} have no operator, ignore it", plan);
+ return;
+ }
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ majorVersion = currLogIndex;
+ minorVersion = 0;
+ plan.setMajorVersion(majorVersion);
Review comment:
Why not using `getPath().setMajorVersion()` like all other plans? make them consistent or add some comments
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -184,7 +190,12 @@ public void deleteStorageGroups(List<PartialPath> storageGroups) throws Metadata
mRemoteMetaCache.removeItem(storageGroup);
}
cacheLock.writeLock().unlock();
- super.deleteStorageGroups(storageGroups);
+
+ for (PartialPath path : storageGroups) {
Review comment:
same as above
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -368,6 +376,90 @@ public static PartitionGroup partitionByPathTimeWithSync(
return partitionGroup;
}
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ long majorVersion = 0;
+ long minorVersion = 0;
+ if (plan.getOperatorType() == null) {
Review comment:
We should handle `LogPlan` as it's operationType is `null` currently
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/RaftMember.java
##########
@@ -1032,6 +1037,27 @@ TSStatus processPlanLocally(PhysicalPlan plan) {
log.setCurrLogTerm(getTerm().get());
log.setCurrLogIndex(logManager.getLastLogIndex() + 1);
+ // Set version for some special plans
+ try {
+ ClusterUtils.setVersionForSpecialPlan(plan, log.getCurrLogIndex());
+ } catch (StorageGroupNotSetException sge) {
+ // if sg not set, sync the meta leader and try again
+ try {
+ ((CMManager) IoTDB.metaManager).syncMetaLeader();
Review comment:
Holding the lock to send RPCs is a very dangerous design. Can't we put this code after releasing the lock?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanExecutor.java
##########
@@ -634,4 +639,39 @@ public void delete(PartialPath path, long startTime, long endTime, long planInde
throw new QueryProcessException(e);
}
}
+
+ /**
+ * the deleteStorageGroupPlan.getPaths() are all storage group paths, it have been processed in
+ * the {@link org.apache.iotdb.cluster.utils.ClusterUtils#setVersionForSpecialPlan(PhysicalPlan,
+ * long)}
+ */
+ @Override
+ protected boolean deleteStorageGroups(DeleteStorageGroupPlan deleteStorageGroupPlan)
+ throws QueryProcessException {
+ AUDIT_LOGGER.info("delete storage group {}", deleteStorageGroupPlan.getPaths());
+ try {
+ IoTDB.metaManager.deleteStorageGroups(deleteStorageGroupPlan.getPaths());
+ } catch (MetadataException e) {
+ throw new QueryProcessException(e);
+ }
+ return true;
Review comment:
Can we guarantee compatibility? It looks like we're not going to throw a `PathNotExistException` like we did before
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -586,6 +597,9 @@ public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsi
// need to verify the storage group is created
verifyCreatedSgSuccess(storageGroups, plan);
+ // reset the majorVersion as long as we have create new storage groups above;
+ ClusterUtils.setVersionForSpecialPlan(plan, plan.getIndex());
Review comment:
It seems that the index of PhysicalPlan hasn't been set until here? we set this only in `processPlanLocallyV2` and `processPlanLocally` currently?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -174,7 +177,10 @@ public String deleteTimeseries(PartialPath prefixPath) throws MetadataException
cacheLock.writeLock().lock();
mRemoteMetaCache.removeItem(prefixPath);
cacheLock.writeLock().unlock();
- return super.deleteTimeseries(prefixPath);
+ if (checkSgNodeAndPlanMajorVersion(prefixPath)) {
+ return super.deleteTimeseries(prefixPath);
Review comment:
Shall we delete `mRemoteMetaCache` but return null ?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -368,6 +376,90 @@ public static PartitionGroup partitionByPathTimeWithSync(
return partitionGroup;
}
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ long majorVersion = 0;
+ long minorVersion = 0;
+ if (plan.getOperatorType() == null) {
+ logger.warn("the plan={} have no operator, ignore it", plan);
+ return;
+ }
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ majorVersion = currLogIndex;
+ minorVersion = 0;
+ plan.setMajorVersion(majorVersion);
+ plan.setMinorVersion(minorVersion);
+ break;
+ case DELETE_STORAGE_GROUP:
+ List<PartialPath> deletePathList = new ArrayList<>();
+ for (PartialPath storageGroupPath : plan.getPaths()) {
+ List<PartialPath> allRelatedStorageGroupPath =
+ IoTDB.metaManager.getStorageGroupPaths(storageGroupPath);
+ if (allRelatedStorageGroupPath.isEmpty()) {
+ throw new PathNotExistException(storageGroupPath.getFullPath(), true);
+ }
+ for (PartialPath path : allRelatedStorageGroupPath) {
+ StorageGroupMNode storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(path);
+ majorVersion = storageGroupMNode.getMajorVersion();
+ minorVersion = storageGroupMNode.getMinorVersion();
+ path.setMajorVersion(majorVersion);
+ path.setMinorVersion(minorVersion);
+ deletePathList.add(path);
+ }
+ }
+ // replace the to be deleted paths
Review comment:
fix comments?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/RaftMember.java
##########
@@ -1032,6 +1037,27 @@ TSStatus processPlanLocally(PhysicalPlan plan) {
log.setCurrLogTerm(getTerm().get());
log.setCurrLogIndex(logManager.getLastLogIndex() + 1);
+ // Set version for some special plans
Review comment:
It seems `processPlanLocallyV2` should be considered too.
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41210511)
Coverage increased (+0.001%) to 68.157% when pulling **b330bfd82de4126a7f5e962b8c009dec8a82f761 on neuyilan:apache_master_0426_version_mtree** into **18ae729e2792c709b7054cb0de25f4d331d7558e 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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41386668)
Coverage increased (+0.008%) to 68.165% when pulling **84e87d7dad97ee480aabc1baee994fb86af2ade4 on neuyilan:apache_master_0426_version_mtree** into **5102f7f672e7e15f60b5095230a75db1e44712c2 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] sonarcloud[bot] commented on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-852138808
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=coverage&view=list) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] sonarcloud[bot] removed a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-852084934
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=coverage&view=list) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] jt2594838 commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r654090322
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/logfile/MLogWriter.java
##########
@@ -437,7 +448,7 @@ public static PhysicalPlan convertFromString(String str) {
CompressionType.values()[Integer.parseInt(words[5])]));
case "1":
return new StorageGroupMNodePlan(
- words[1], Long.parseLong(words[2]), Integer.parseInt(words[3]));
+ words[1], Long.parseLong(words[2]), Integer.parseInt(words[3]), 0, 0);
case "0":
Review comment:
The versions seem not to be persisted.
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/MLogTxtWriter.java
##########
@@ -166,6 +166,14 @@ public void setStorageGroup(String storageGroup) throws IOException {
lineNumber.incrementAndGet();
}
+ public void updateStorageGroup(String storageGroup) throws IOException {
+ String outputStr =
+ MetadataOperationType.UPDATE_STORAGE_GROUP + "," + storageGroup + LINE_SEPARATOR;
+ ByteBuffer buff = ByteBuffer.wrap(outputStr.getBytes());
+ channel.write(buff);
+ lineNumber.incrementAndGet();
+ }
+
Review comment:
Is it possible to recover the versions of an SG after a restart?
##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/PartialPath.java
##########
@@ -44,6 +44,10 @@
// alias of measurement, null pointer cannot be serialized in thrift so empty string is instead
protected String measurementAlias = "";
+ private long majorVersion = 0;
+
+ private long minorVersion = 0;
+
Review comment:
Add some comments to explain the two fields.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/RaftMember.java
##########
@@ -974,9 +977,17 @@ TSStatus processPlanLocally(PhysicalPlan plan) {
synchronized (logManager) {
log.setCurrLogTerm(getTerm().get());
log.setCurrLogIndex(logManager.getLastLogIndex() + 1);
-
log.setPlan(plan);
plan.setIndex(log.getCurrLogIndex());
+
+ // Set version for some special plans
+ try {
+ ClusterUtils.setVersionForSpecialPlan(plan, log.getCurrLogIndex());
+ } catch (MetadataException e) {
+ logger.error("process plan failed, plan={}", plan, e);
+ return StatusUtils.INTERNAL_ERROR;
Review comment:
Include the message of `e` in the status and use a copy of the status.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1816,4 +1858,63 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MetadataException sync meta leader failed
Review comment:
This `MetadataException` seems to include a `StorageGroupNotSetException`.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/utils/ClusterUtils.java
##########
@@ -353,4 +361,65 @@ public static PartitionGroup partitionByPathTimeWithSync(
}
return partitionGroup;
}
+
+ public static void setVersionForSpecialPlan(PhysicalPlan plan, long currLogIndex)
+ throws MetadataException {
+ switch (plan.getOperatorType()) {
+ case SET_STORAGE_GROUP:
+ plan.setMajorVersion(currLogIndex);
+ plan.setMinorVersion(0);
+ break;
+ case DELETE_STORAGE_GROUP:
+ List<PartialPath> deletePathList = new ArrayList<>();
+ for (PartialPath storageGroupPath : plan.getPaths()) {
+ List<PartialPath> allRelatedStorageGroupPath =
+ IoTDB.metaManager.getStorageGroupPaths(storageGroupPath);
+ if (allRelatedStorageGroupPath.isEmpty()) {
+ throw new PathNotExistException(storageGroupPath.getFullPath(), true);
+ }
+ for (PartialPath path : allRelatedStorageGroupPath) {
+ StorageGroupMNode storageGroupMNode = IoTDB.metaManager.getStorageGroupNodeByPath(path);
+ path.setMajorVersion(storageGroupMNode.getMajorVersion());
+ path.setMinorVersion(storageGroupMNode.getMinorVersion());
+ }
+ }
+ // replace the to be deleted paths
+ plan.setPaths(deletePathList);
+ break;
Review comment:
Maybe it is not necessary to record the major version of every deleted SGs as we can delete the SG as long as its version is smaller than that of the plan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] neuyilan commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r671630105
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanExecutor.java
##########
@@ -634,4 +639,39 @@ public void delete(PartialPath path, long startTime, long endTime, long planInde
throw new QueryProcessException(e);
}
}
+
+ /**
+ * the deleteStorageGroupPlan.getPaths() are all storage group paths, it have been processed in
+ * the {@link org.apache.iotdb.cluster.utils.ClusterUtils#setVersionForSpecialPlan(PhysicalPlan,
+ * long)}
+ */
+ @Override
+ protected boolean deleteStorageGroups(DeleteStorageGroupPlan deleteStorageGroupPlan)
+ throws QueryProcessException {
+ AUDIT_LOGGER.info("delete storage group {}", deleteStorageGroupPlan.getPaths());
+ try {
+ IoTDB.metaManager.deleteStorageGroups(deleteStorageGroupPlan.getPaths());
+ } catch (MetadataException e) {
+ throw new QueryProcessException(e);
+ }
+ return true;
Review comment:
Because the super.deleteStorageGroup() will use the `deleteStorageGroupPlan.getPaths() `to get allRelatedStorageGroupPath, in this phase, the version of the plan's path will be moved. so when executing the plan, it will be failed due to no version.
So here we override the `deleteStorageGroups()` method in the `ClusterPlanExecutor.java`
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -508,4 +518,110 @@ public void testCreateSnapshot() throws InterruptedException {
Assert.assertNull(e);
}
}
+
+ // test https://issues.apache.org/jira/browse/IOTDB-1292
+ @Test
+ public void testDataRaftAndMetaRaftConcurrentOperation() {
+ ExecutorService dataRaftGroupThread = Executors.newFixedThreadPool(1);
+ ExecutorService metaRaftGroupThread = Executors.newFixedThreadPool(1);
+ createSgTask();
+ Future<?> dataFeature = dataRaftGroupThread.submit(this::createTimeSeriesTask);
+ Future<?> metaFeature = metaRaftGroupThread.submit(this::deleteSgTask);
+ try {
+ dataFeature.get();
+ metaFeature.get();
+ } catch (InterruptedException e) {
+ logger.error(e.getMessage());
+ } catch (ExecutionException e) {
+ Assert.fail(e.getMessage());
+ }
+
+ // check the final result
+ try {
+ queryFinalResult();
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ public void createTimeSeriesTask() {
Review comment:
done
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -732,6 +746,16 @@ public boolean createTimeseries(InsertPlan insertPlan)
logger.error("Failed to infer storage group from deviceId {}", deviceId);
return false;
}
+
+ Pair<Long, Long> sgNodeVersion;
+ try {
+ sgNodeVersion = getSgNodeMajorVersion(storageGroupName);
+ } catch (MetadataException e) {
+ logger.error("failed get the major version", e);
Review comment:
Thanks, have fixed.
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -508,4 +518,110 @@ public void testCreateSnapshot() throws InterruptedException {
Assert.assertNull(e);
}
}
+
+ // test https://issues.apache.org/jira/browse/IOTDB-1292
+ @Test
+ public void testDataRaftAndMetaRaftConcurrentOperation() {
+ ExecutorService dataRaftGroupThread = Executors.newFixedThreadPool(1);
+ ExecutorService metaRaftGroupThread = Executors.newFixedThreadPool(1);
+ createSgTask();
+ Future<?> dataFeature = dataRaftGroupThread.submit(this::createTimeSeriesTask);
+ Future<?> metaFeature = metaRaftGroupThread.submit(this::deleteSgTask);
+ try {
+ dataFeature.get();
+ metaFeature.get();
+ } catch (InterruptedException e) {
+ logger.error(e.getMessage());
+ } catch (ExecutionException e) {
+ Assert.fail(e.getMessage());
+ }
+
+ // check the final result
+ try {
+ queryFinalResult();
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ public void createTimeSeriesTask() {
+ int sgNum = 10;
+ int deviceNum = 10;
+ int measurementsNum = 100;
+ String sqlFormat =
+ "create timeseries root.sg_%d.device_%d.s_%d with datatype=INT64, encoding=PLAIN, compression=SNAPPY";
+ for (int sg = 0; sg < sgNum; sg++) {
+ for (int device = 0; device < deviceNum; device++) {
+ for (int measurement = 0; measurement < measurementsNum; measurement++) {
+ String sql = String.format(sqlFormat, sg, device, measurement);
+ try {
+ session.executeNonQueryStatement(sql);
+ } catch (StatementExecutionException e) {
+ if (e.getMessage().contains("Storage group is not set for current seriesPath")) {
+ // it's ok due to concurrent operation, do nothing.
+ } else {
+ Assert.fail(e.getMessage());
+ }
+ } catch (IoTDBConnectionException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+ }
+ }
+ }
+
+ public void createSgTask() {
Review comment:
done
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/server/member/RaftMember.java
##########
@@ -1032,6 +1037,27 @@ TSStatus processPlanLocally(PhysicalPlan plan) {
log.setCurrLogTerm(getTerm().get());
log.setCurrLogIndex(logManager.getLastLogIndex() + 1);
+ // Set version for some special plans
+ try {
+ ClusterUtils.setVersionForSpecialPlan(plan, log.getCurrLogIndex());
+ } catch (StorageGroupNotSetException sge) {
+ // if sg not set, sync the meta leader and try again
+ try {
+ ((CMManager) IoTDB.metaManager).syncMetaLeader();
+ ClusterUtils.setVersionForSpecialPlan(plan, log.getCurrLogIndex());
+ } catch (MetadataException e1) {
+ logger.error("process plan failed, plan={}", plan, e1);
+ TSStatus tsStatus = StatusUtils.INTERNAL_ERROR.deepCopy();
Review comment:
We want to return new `TSStatus` that contain the exception's message, so there use deepCopy.
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -508,4 +518,110 @@ public void testCreateSnapshot() throws InterruptedException {
Assert.assertNull(e);
}
}
+
+ // test https://issues.apache.org/jira/browse/IOTDB-1292
+ @Test
+ public void testDataRaftAndMetaRaftConcurrentOperation() {
+ ExecutorService dataRaftGroupThread = Executors.newFixedThreadPool(1);
+ ExecutorService metaRaftGroupThread = Executors.newFixedThreadPool(1);
+ createSgTask();
+ Future<?> dataFeature = dataRaftGroupThread.submit(this::createTimeSeriesTask);
+ Future<?> metaFeature = metaRaftGroupThread.submit(this::deleteSgTask);
+ try {
+ dataFeature.get();
+ metaFeature.get();
+ } catch (InterruptedException e) {
+ logger.error(e.getMessage());
+ } catch (ExecutionException e) {
+ Assert.fail(e.getMessage());
+ }
+
+ // check the final result
+ try {
+ queryFinalResult();
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+
+ public void createTimeSeriesTask() {
+ int sgNum = 10;
+ int deviceNum = 10;
+ int measurementsNum = 100;
+ String sqlFormat =
+ "create timeseries root.sg_%d.device_%d.s_%d with datatype=INT64, encoding=PLAIN, compression=SNAPPY";
+ for (int sg = 0; sg < sgNum; sg++) {
+ for (int device = 0; device < deviceNum; device++) {
+ for (int measurement = 0; measurement < measurementsNum; measurement++) {
+ String sql = String.format(sqlFormat, sg, device, measurement);
+ try {
+ session.executeNonQueryStatement(sql);
+ } catch (StatementExecutionException e) {
+ if (e.getMessage().contains("Storage group is not set for current seriesPath")) {
+ // it's ok due to concurrent operation, do nothing.
+ } else {
+ Assert.fail(e.getMessage());
+ }
+ } catch (IoTDBConnectionException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+ }
+ }
+ }
+
+ public void createSgTask() {
+ int sgNum = 10;
+ String sqlFormat = "set storage group to root.sg_%d";
+ for (int sg = 0; sg < sgNum; sg++) {
+ String sql = String.format(sqlFormat, sg);
+ try {
+ writeStatement.execute(sql);
+ } catch (SQLException e) {
+ Assert.fail(e.getMessage());
+ }
+ }
+ }
+
+ public void deleteSgTask() {
Review comment:
done
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/StorageGroupMNodePlan.java
##########
@@ -30,19 +30,27 @@
import java.util.Objects;
public class StorageGroupMNodePlan extends MNodePlan {
+
private long dataTTL;
private int alignedTimeseriesIndex;
+ private long majorVersion;
+
+ private long minorVersion;
+
public StorageGroupMNodePlan() {
super(false, Operator.OperatorType.STORAGE_GROUP_MNODE);
}
- public StorageGroupMNodePlan(String name, long dataTTL, int childSize) {
Review comment:
Sure, thanks
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41315634)
Coverage decreased (-0.01%) to 68.144% when pulling **648343191ebc1fbe676f9f8801c68b19296f3043 on neuyilan:apache_master_0426_version_mtree** into **18ae729e2792c709b7054cb0de25f4d331d7558e 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] sonarcloud[bot] commented on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-852821895
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=coverage&view=list) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41348230)
Coverage decreased (-0.003%) to 68.172% when pulling **55de30b40ff9a58121ff29a201bdfd0ac6ce8c6b on neuyilan:apache_master_0426_version_mtree** into **5e020b5880a97c8530b39986b504c5f28fcbcd33 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] mychaow commented on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
mychaow commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-885338114
> @mychaow @wangchao316 @jt2594838 @LebronAl Could you please take a review?
fix conflict
--
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] sonarcloud[bot] commented on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-852084934
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=coverage&view=list) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] mychaow commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r670239158
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +768,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
if (unregisteredSeriesList.isEmpty()) {
return true;
}
- logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+ logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
- return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+ return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+ }
+
+ /**
+ * @param partialPath the storage group's partialPath
+ * @return the major version and the minor version of the storage group node
+ */
+ private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
+ StorageGroupMNode node;
+ try {
+ node = getStorageGroupNodeByPath(partialPath);
+ } catch (MetadataException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(partialPath);
+ }
+ if (node == null) {
Review comment:
So, node can not be null? If we could not find the node, we will throw MetadataException?
##########
File path: testcontainer/src/test/java/org/apache/iotdb/db/sql/Cases.java
##########
@@ -36,23 +36,32 @@
import org.junit.Assert;
import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
public abstract class Cases {
+ private static Logger logger = LoggerFactory.getLogger(Cases.class);
protected Statement writeStatement;
protected Connection writeConnection;
protected Statement[] readStatements;
protected Connection[] readConnections;
protected Session session;
+ protected Session antherSession;
Review comment:
anther->another
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/UpdateStorageGroupPlan.java
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.db.qp.physical.sys;
+
+import org.apache.iotdb.db.exception.metadata.IllegalPathException;
+import org.apache.iotdb.db.metadata.PartialPath;
+import org.apache.iotdb.db.qp.logical.Operator;
+import org.apache.iotdb.db.qp.logical.Operator.OperatorType;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class UpdateStorageGroupPlan extends PhysicalPlan {
+
+ private PartialPath path;
+
+ public UpdateStorageGroupPlan() {
+ super(false, OperatorType.UPDATE_STORAGE_GROUP);
+ }
+
+ public UpdateStorageGroupPlan(PartialPath path) {
+ super(false, Operator.OperatorType.UPDATE_STORAGE_GROUP);
+ this.path = path;
+ }
+
+ public PartialPath getPath() {
+ return path;
+ }
+
+ public void setPath(PartialPath path) {
+ this.path = path;
+ }
+
+ @Override
+ public List<PartialPath> getPaths() {
+ return path != null ? Collections.singletonList(path) : Collections.emptyList();
+ }
+
+ @Override
+ public void serialize(DataOutputStream stream) throws IOException {
+ stream.write((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(stream, path.getFullPath());
+ stream.writeLong(index);
+ }
+
+ @Override
+ public void serialize(ByteBuffer buffer) {
+ buffer.put((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(buffer, path.getFullPath());
+ buffer.putLong(index);
+ buffer.putLong(path.getMajorVersion());
+ buffer.putLong(path.getMinorVersion());
+ }
+
+ @Override
+ public void deserialize(ByteBuffer buffer) throws IllegalPathException {
+ path = new PartialPath(readString(buffer));
+ this.index = buffer.getLong();
+ path.setMajorVersion(buffer.getLong());
+ path.setMinorVersion(buffer.getLong());
+ }
+
+ @Override
+ public String toString() {
+ return "UpdateStorageGroupPlan{" + " path=" + path + ", majorVersion=" + "}";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+
+ UpdateStorageGroupPlan that = (UpdateStorageGroupPlan) o;
+
+ return Objects.equals(path, that.path);
Review comment:
equals only compare whether is same object?
##########
File path: server/src/main/java/org/apache/iotdb/db/tools/mlog/MLogParser.java
##########
@@ -175,6 +176,10 @@ public static void parseFromFile(String inputFile, String outputFile) throws IOE
case SET_STORAGE_GROUP:
mLogTxtWriter.setStorageGroup(((SetStorageGroupPlan) plan).getPath().getFullPath());
break;
+ case UPDATE_STORAGE_GROUP:
+ mLogTxtWriter.updateStorageGroup(
Review comment:
you should print major & minor version
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/UpdateStorageGroupPlan.java
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.db.qp.physical.sys;
+
+import org.apache.iotdb.db.exception.metadata.IllegalPathException;
+import org.apache.iotdb.db.metadata.PartialPath;
+import org.apache.iotdb.db.qp.logical.Operator;
+import org.apache.iotdb.db.qp.logical.Operator.OperatorType;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class UpdateStorageGroupPlan extends PhysicalPlan {
+
+ private PartialPath path;
+
+ public UpdateStorageGroupPlan() {
+ super(false, OperatorType.UPDATE_STORAGE_GROUP);
+ }
+
+ public UpdateStorageGroupPlan(PartialPath path) {
+ super(false, Operator.OperatorType.UPDATE_STORAGE_GROUP);
+ this.path = path;
+ }
+
+ public PartialPath getPath() {
+ return path;
+ }
+
+ public void setPath(PartialPath path) {
+ this.path = path;
+ }
+
+ @Override
+ public List<PartialPath> getPaths() {
+ return path != null ? Collections.singletonList(path) : Collections.emptyList();
+ }
+
+ @Override
+ public void serialize(DataOutputStream stream) throws IOException {
+ stream.write((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(stream, path.getFullPath());
+ stream.writeLong(index);
Review comment:
so, why do not serialize major & minor version?
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -732,6 +746,16 @@ public boolean createTimeseries(InsertPlan insertPlan)
logger.error("Failed to infer storage group from deviceId {}", deviceId);
return false;
}
+
+ Pair<Long, Long> sgNodeVersion;
+ try {
+ sgNodeVersion = getSgNodeMajorVersion(storageGroupName);
+ } catch (MetadataException e) {
+ logger.error("failed get the major version", e);
+ return false;
+ }
+
+ boolean hasVector = false;
Review comment:
what is the meaning of this variable？
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1874,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MajorVersionNotEqualException sync meta leader failed and the
+ * @throws StorageGroupNotSetException some error occurred.
+ */
+ private boolean checkSgNodeAndPlanMajorVersion(PartialPath path)
+ throws MetadataException, StorageGroupNotSetException {
+ StorageGroupMNode node = null;
+ try {
+ node = getStorageGroupNodeByPath(path);
+ } catch (StorageGroupNotSetException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
+ long nodeMajorVersion = node.getMajorVersion();
+ if (nodeMajorVersion < path.getMajorVersion()) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
+ nodeMajorVersion = node.getMajorVersion();
+ if (nodeMajorVersion < path.getMajorVersion()) {
+ logger.error(
Review comment:
If syncmetaleader failed, the follower will be inconsistent with leader. And how to find the inconsistent?
##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/sys/UpdateStorageGroupPlan.java
##########
@@ -0,0 +1,107 @@
+/*
+ * 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.db.qp.physical.sys;
+
+import org.apache.iotdb.db.exception.metadata.IllegalPathException;
+import org.apache.iotdb.db.metadata.PartialPath;
+import org.apache.iotdb.db.qp.logical.Operator;
+import org.apache.iotdb.db.qp.logical.Operator.OperatorType;
+import org.apache.iotdb.db.qp.physical.PhysicalPlan;
+
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+public class UpdateStorageGroupPlan extends PhysicalPlan {
+
+ private PartialPath path;
+
+ public UpdateStorageGroupPlan() {
+ super(false, OperatorType.UPDATE_STORAGE_GROUP);
+ }
+
+ public UpdateStorageGroupPlan(PartialPath path) {
+ super(false, Operator.OperatorType.UPDATE_STORAGE_GROUP);
+ this.path = path;
+ }
+
+ public PartialPath getPath() {
+ return path;
+ }
+
+ public void setPath(PartialPath path) {
+ this.path = path;
+ }
+
+ @Override
+ public List<PartialPath> getPaths() {
+ return path != null ? Collections.singletonList(path) : Collections.emptyList();
+ }
+
+ @Override
+ public void serialize(DataOutputStream stream) throws IOException {
+ stream.write((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(stream, path.getFullPath());
+ stream.writeLong(index);
+ }
+
+ @Override
+ public void serialize(ByteBuffer buffer) {
+ buffer.put((byte) PhysicalPlanType.UPDATE_STORAGE_GROUP.ordinal());
+ putString(buffer, path.getFullPath());
+ buffer.putLong(index);
+ buffer.putLong(path.getMajorVersion());
+ buffer.putLong(path.getMinorVersion());
+ }
+
+ @Override
+ public void deserialize(ByteBuffer buffer) throws IllegalPathException {
+ path = new PartialPath(readString(buffer));
+ this.index = buffer.getLong();
+ path.setMajorVersion(buffer.getLong());
+ path.setMinorVersion(buffer.getLong());
+ }
+
+ @Override
+ public String toString() {
Review comment:
why only print major version?
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41446761)
Coverage decreased (-0.02%) to 68.136% when pulling **c9da727ce9d85e27cc58b635797d27a673b6ad60 on neuyilan:apache_master_0426_version_mtree** into **5102f7f672e7e15f60b5095230a75db1e44712c2 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] mychaow commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r669524011
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1874,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MajorVersionNotEqualException sync meta leader failed and the
+ * @throws StorageGroupNotSetException some error occurred.
+ */
+ private boolean checkSgNodeAndPlanMajorVersion(PartialPath path)
+ throws MetadataException, StorageGroupNotSetException {
+ StorageGroupMNode node = null;
+ try {
+ node = getStorageGroupNodeByPath(path);
+ } catch (StorageGroupNotSetException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
Review comment:
so, why do not check whether node is null?
--
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] neuyilan commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r670059042
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1874,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MajorVersionNotEqualException sync meta leader failed and the
+ * @throws StorageGroupNotSetException some error occurred.
+ */
+ private boolean checkSgNodeAndPlanMajorVersion(PartialPath path)
+ throws MetadataException, StorageGroupNotSetException {
+ StorageGroupMNode node = null;
+ try {
+ node = getStorageGroupNodeByPath(path);
+ } catch (StorageGroupNotSetException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
Review comment:
The node can not be null, if the sg not found, it will throw `StorageGroupNotSetException` rather than null, so we do not need to check the node is null or not.
--
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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/40500396)
Coverage decreased (-0.02%) to 67.844% when pulling **dbe46fca025a1def7d7eb93da37ae820602b9271 on neuyilan:apache_master_0426_version_mtree** into **ff645f69c771b7672cd2a07197b241b8916bcafa 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41423307)
Coverage decreased (-0.001%) to 68.155% when pulling **441e8003f53d15780bafa80e3678f43c60dfa571 on neuyilan:apache_master_0426_version_mtree** into **5102f7f672e7e15f60b5095230a75db1e44712c2 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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41208954)
Coverage increased (+0.01%) to 68.166% when pulling **0d0e0a1b4164714ba4911da11df821f53df9eb35 on neuyilan:apache_master_0426_version_mtree** into **18ae729e2792c709b7054cb0de25f4d331d7558e 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] neuyilan commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r675975857
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +767,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
if (unregisteredSeriesList.isEmpty()) {
return true;
}
- logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+ logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
- return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+ return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+ }
+
+ /**
+ * @param partialPath the storage group's partialPath
+ * @return the major version and the minor version of the storage group node
+ */
+ private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
Review comment:
sure, thanks
--
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] mychaow commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r675253351
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1874,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
return super.getStorageGroupPath(path);
}
}
+
+ @Override
+ public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPrefixPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createAlignedTimeSeries(plan);
+ }
+ }
+
+ @Override
+ public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+ PartialPath path = plan.getPath();
+ if (checkSgNodeAndPlanMajorVersion(path)) {
+ super.createTimeseries(plan);
+ }
+ }
+
+ /**
+ * @param path the partial path
+ * @return true if the sg node major version == the path's major version; false if the sg node
+ * major version > the path's major version; throw MetadataException if the sg node major
+ * version < the path's major version.
+ * @throws MajorVersionNotEqualException sync meta leader failed and the
+ * @throws StorageGroupNotSetException some error occurred.
+ */
+ private boolean checkSgNodeAndPlanMajorVersion(PartialPath path)
+ throws MetadataException, StorageGroupNotSetException {
+ StorageGroupMNode node = null;
+ try {
+ node = getStorageGroupNodeByPath(path);
+ } catch (StorageGroupNotSetException e) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
+ long nodeMajorVersion = node.getMajorVersion();
+ if (nodeMajorVersion < path.getMajorVersion()) {
+ syncMetaLeader();
+ node = getStorageGroupNodeByPath(path);
+ }
+
+ nodeMajorVersion = node.getMajorVersion();
+ if (nodeMajorVersion < path.getMajorVersion()) {
+ logger.error(
Review comment:
ok, but we should retry apply until success, not move the log to the committed's log from uncommitted logs.
--
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] sonarcloud[bot] commented on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-851990576
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=3269&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=coverage&view=list) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=3269&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41360623)
Coverage decreased (-0.002%) to 68.173% when pulling **ed8ccb88a6aa4d8087f65e378ceee83881f90875 on neuyilan:apache_master_0426_version_mtree** into **5e020b5880a97c8530b39986b504c5f28fcbcd33 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 #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls edited a comment on pull request #3269: [IOTDB-1292] Versioned MTree
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#issuecomment-859256455
[](https://coveralls.io/builds/41386648)
Coverage decreased (-0.03%) to 68.13% when pulling **84e87d7dad97ee480aabc1baee994fb86af2ade4 on neuyilan:apache_master_0426_version_mtree** into **5102f7f672e7e15f60b5095230a75db1e44712c2 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