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/02/24 13:10:37 UTC
[GitHub] [iotdb] LebronAl opened a new pull request #2731: [IoTDB-1173] auto-create schema improvement for createMultiTimeSeriesPaln
LebronAl opened a new pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731
see [1173](https://issues.apache.org/jira/browse/IOTDB-1173).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#discussion_r582621629
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -426,58 +428,77 @@ public synchronized boolean containsKey(PartialPath key) {
}
/**
- * create storage groups for CreateTimeseriesPlan and InsertPlan, also create timeseries for
- * InsertPlan. Only the two kind of plans can use this method.
+ * create storage groups for CreateTimeseriesPlan, CreateMultiTimeseriesPlan and InsertPlan, also
+ * create timeseries for InsertPlan. Only the three kind of plans can use this method.
*/
public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsistencyException {
- // try to set storage group
- List<PartialPath> deviceIds;
- // only handle InsertPlan, CreateTimeSeriesPlan and CreateMultiTimeSeriesPlan currently,
- if (plan instanceof InsertPlan
- && !(plan instanceof InsertMultiTabletPlan)
- && !(plan instanceof InsertRowsPlan)) {
- // InsertMultiTabletPlan and InsertRowsPlan have multiple devices, and other types of
- // InsertPlan have only one device.
- deviceIds = Collections.singletonList(((InsertPlan) plan).getDeviceId());
+ List<PartialPath> storageGroups = new ArrayList<>();
+ // for InsertPlan, try to just use deviceIds to get related storage groups because there's no
+ // need to call getPaths to concat deviceId and sensor as they will gain same result.
+ // for CreateTimeSeriesPlan, use getPath() to get timeseries to get related storage group
+ // for CreateTimeMultiSeriesPlan, use getPaths() to get all timeseries to get related storage
+ // groups
Review comment:
OK
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -426,58 +428,77 @@ public synchronized boolean containsKey(PartialPath key) {
}
/**
- * create storage groups for CreateTimeseriesPlan and InsertPlan, also create timeseries for
- * InsertPlan. Only the two kind of plans can use this method.
+ * create storage groups for CreateTimeseriesPlan, CreateMultiTimeseriesPlan and InsertPlan, also
+ * create timeseries for InsertPlan. Only the three kind of plans can use this method.
*/
public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsistencyException {
- // try to set storage group
- List<PartialPath> deviceIds;
- // only handle InsertPlan, CreateTimeSeriesPlan and CreateMultiTimeSeriesPlan currently,
- if (plan instanceof InsertPlan
- && !(plan instanceof InsertMultiTabletPlan)
- && !(plan instanceof InsertRowsPlan)) {
- // InsertMultiTabletPlan and InsertRowsPlan have multiple devices, and other types of
- // InsertPlan have only one device.
- deviceIds = Collections.singletonList(((InsertPlan) plan).getDeviceId());
+ List<PartialPath> storageGroups = new ArrayList<>();
+ // for InsertPlan, try to just use deviceIds to get related storage groups because there's no
+ // need to call getPaths to concat deviceId and sensor as they will gain same result.
+ // for CreateTimeSeriesPlan, use getPath() to get timeseries to get related storage group
Review comment:
OK
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] sonarcloud[bot] commented on pull request #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#issuecomment-785820728
SonarCloud Quality Gate failed.
[<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=2731&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=2731&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='74.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list) [74.3% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] LebronAl commented on a change in pull request #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan
Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#discussion_r582626345
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -500,24 +521,23 @@ private void verifyCreatedSgSuccess(List<PartialPath> deviceIds, PhysicalPlan ph
}
/**
- * Create storage group automatically for deviceId.
+ * Create storage groups automatically for paths.
*
- * @param deviceId
+ * @param storageGroups the uncreated storage groups
*/
- private void createStorageGroup(PartialPath deviceId) throws MetadataException {
- PartialPath storageGroupName =
- MetaUtils.getStorageGroupPathByLevel(
- deviceId, IoTDBDescriptor.getInstance().getConfig().getDefaultStorageGroupLevel());
- SetStorageGroupPlan setStorageGroupPlan = new SetStorageGroupPlan(storageGroupName);
- TSStatus setStorageGroupResult =
- metaGroupMember.processNonPartitionedMetaPlan(setStorageGroupPlan);
- if (setStorageGroupResult.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()
- && setStorageGroupResult.getCode()
- != TSStatusCode.PATH_ALREADY_EXIST_ERROR.getStatusCode()) {
- throw new MetadataException(
- String.format(
- "Status Code: %d, failed to set storage group %s",
- setStorageGroupResult.getCode(), storageGroupName));
+ private void createStorageGroups(List<PartialPath> storageGroups) throws MetadataException {
+ for (PartialPath storageGroup : storageGroups) {
+ SetStorageGroupPlan setStorageGroupPlan = new SetStorageGroupPlan(storageGroup);
+ TSStatus setStorageGroupResult =
+ metaGroupMember.processNonPartitionedMetaPlan(setStorageGroupPlan);
+ if (setStorageGroupResult.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()
+ && setStorageGroupResult.getCode()
+ != TSStatusCode.PATH_ALREADY_EXIST_ERROR.getStatusCode()) {
+ throw new MetadataException(
+ String.format(
+ "Status Code: %d, failed to set storage group %s",
+ setStorageGroupResult.getCode(), storageGroup));
+ }
Review comment:
No matter how many storage groups are created, client will get an `NO_STORAGE_GROUP` TSStatus as long as there exists a failed setStorageGroup operation, which means the client need to perform a retry, and the failed setStorageGroup operation will be executed in this client's retry request. As this situation is relatively rare, so current implemention is ok for me.
----------------------------------------------------------------
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 #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPaln
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#issuecomment-785094154
SonarCloud Quality Gate failed.
[<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=2731&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=2731&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='63.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list) [63.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
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 #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#issuecomment-785138268
SonarCloud Quality Gate failed.
[<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=2731&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=2731&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='74.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list) [74.3% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
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 merged pull request #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan
Posted by GitBox <gi...@apache.org>.
mychaow merged pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731
----------------------------------------------------------------
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] commented on pull request #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPaln
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#issuecomment-785138268
SonarCloud Quality Gate failed.
[<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=2731&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=2731&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='74.3%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list) [74.3% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
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 #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan
Posted by GitBox <gi...@apache.org>.
mychaow commented on a change in pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#discussion_r582603780
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -500,24 +521,23 @@ private void verifyCreatedSgSuccess(List<PartialPath> deviceIds, PhysicalPlan ph
}
/**
- * Create storage group automatically for deviceId.
+ * Create storage groups automatically for paths.
*
- * @param deviceId
+ * @param storageGroups the uncreated storage groups
*/
- private void createStorageGroup(PartialPath deviceId) throws MetadataException {
- PartialPath storageGroupName =
- MetaUtils.getStorageGroupPathByLevel(
- deviceId, IoTDBDescriptor.getInstance().getConfig().getDefaultStorageGroupLevel());
- SetStorageGroupPlan setStorageGroupPlan = new SetStorageGroupPlan(storageGroupName);
- TSStatus setStorageGroupResult =
- metaGroupMember.processNonPartitionedMetaPlan(setStorageGroupPlan);
- if (setStorageGroupResult.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()
- && setStorageGroupResult.getCode()
- != TSStatusCode.PATH_ALREADY_EXIST_ERROR.getStatusCode()) {
- throw new MetadataException(
- String.format(
- "Status Code: %d, failed to set storage group %s",
- setStorageGroupResult.getCode(), storageGroupName));
+ private void createStorageGroups(List<PartialPath> storageGroups) throws MetadataException {
+ for (PartialPath storageGroup : storageGroups) {
+ SetStorageGroupPlan setStorageGroupPlan = new SetStorageGroupPlan(storageGroup);
+ TSStatus setStorageGroupResult =
+ metaGroupMember.processNonPartitionedMetaPlan(setStorageGroupPlan);
+ if (setStorageGroupResult.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()
+ && setStorageGroupResult.getCode()
+ != TSStatusCode.PATH_ALREADY_EXIST_ERROR.getStatusCode()) {
+ throw new MetadataException(
+ String.format(
+ "Status Code: %d, failed to set storage group %s",
+ setStorageGroupResult.getCode(), storageGroup));
+ }
Review comment:
allow partial set storage group success is better.
----------------------------------------------------------------
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 #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPlan
Posted by GitBox <gi...@apache.org>.
neuyilan commented on a change in pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#discussion_r582436686
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -426,58 +428,77 @@ public synchronized boolean containsKey(PartialPath key) {
}
/**
- * create storage groups for CreateTimeseriesPlan and InsertPlan, also create timeseries for
- * InsertPlan. Only the two kind of plans can use this method.
+ * create storage groups for CreateTimeseriesPlan, CreateMultiTimeseriesPlan and InsertPlan, also
+ * create timeseries for InsertPlan. Only the three kind of plans can use this method.
*/
public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsistencyException {
- // try to set storage group
- List<PartialPath> deviceIds;
- // only handle InsertPlan, CreateTimeSeriesPlan and CreateMultiTimeSeriesPlan currently,
- if (plan instanceof InsertPlan
- && !(plan instanceof InsertMultiTabletPlan)
- && !(plan instanceof InsertRowsPlan)) {
- // InsertMultiTabletPlan and InsertRowsPlan have multiple devices, and other types of
- // InsertPlan have only one device.
- deviceIds = Collections.singletonList(((InsertPlan) plan).getDeviceId());
+ List<PartialPath> storageGroups = new ArrayList<>();
+ // for InsertPlan, try to just use deviceIds to get related storage groups because there's no
+ // need to call getPaths to concat deviceId and sensor as they will gain same result.
+ // for CreateTimeSeriesPlan, use getPath() to get timeseries to get related storage group
+ // for CreateTimeMultiSeriesPlan, use getPaths() to get all timeseries to get related storage
+ // groups
Review comment:
CreateTimeMultiSeriesPlan -> CreateMultiTimeSeriesPlan
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -426,58 +428,77 @@ public synchronized boolean containsKey(PartialPath key) {
}
/**
- * create storage groups for CreateTimeseriesPlan and InsertPlan, also create timeseries for
- * InsertPlan. Only the two kind of plans can use this method.
+ * create storage groups for CreateTimeseriesPlan, CreateMultiTimeseriesPlan and InsertPlan, also
+ * create timeseries for InsertPlan. Only the three kind of plans can use this method.
*/
public void createSchema(PhysicalPlan plan) throws MetadataException, CheckConsistencyException {
- // try to set storage group
- List<PartialPath> deviceIds;
- // only handle InsertPlan, CreateTimeSeriesPlan and CreateMultiTimeSeriesPlan currently,
- if (plan instanceof InsertPlan
- && !(plan instanceof InsertMultiTabletPlan)
- && !(plan instanceof InsertRowsPlan)) {
- // InsertMultiTabletPlan and InsertRowsPlan have multiple devices, and other types of
- // InsertPlan have only one device.
- deviceIds = Collections.singletonList(((InsertPlan) plan).getDeviceId());
+ List<PartialPath> storageGroups = new ArrayList<>();
+ // for InsertPlan, try to just use deviceIds to get related storage groups because there's no
+ // need to call getPaths to concat deviceId and sensor as they will gain same result.
+ // for CreateTimeSeriesPlan, use getPath() to get timeseries to get related storage group
Review comment:
```suggestion
// for CreateTimeSeriesPlan, use getPath() to get timeseries to get related storage group,
```
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -500,24 +521,23 @@ private void verifyCreatedSgSuccess(List<PartialPath> deviceIds, PhysicalPlan ph
}
/**
- * Create storage group automatically for deviceId.
+ * Create storage groups automatically for paths.
*
- * @param deviceId
+ * @param storageGroups the uncreated storage groups
*/
- private void createStorageGroup(PartialPath deviceId) throws MetadataException {
- PartialPath storageGroupName =
- MetaUtils.getStorageGroupPathByLevel(
- deviceId, IoTDBDescriptor.getInstance().getConfig().getDefaultStorageGroupLevel());
- SetStorageGroupPlan setStorageGroupPlan = new SetStorageGroupPlan(storageGroupName);
- TSStatus setStorageGroupResult =
- metaGroupMember.processNonPartitionedMetaPlan(setStorageGroupPlan);
- if (setStorageGroupResult.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()
- && setStorageGroupResult.getCode()
- != TSStatusCode.PATH_ALREADY_EXIST_ERROR.getStatusCode()) {
- throw new MetadataException(
- String.format(
- "Status Code: %d, failed to set storage group %s",
- setStorageGroupResult.getCode(), storageGroupName));
+ private void createStorageGroups(List<PartialPath> storageGroups) throws MetadataException {
+ for (PartialPath storageGroup : storageGroups) {
+ SetStorageGroupPlan setStorageGroupPlan = new SetStorageGroupPlan(storageGroup);
+ TSStatus setStorageGroupResult =
+ metaGroupMember.processNonPartitionedMetaPlan(setStorageGroupPlan);
+ if (setStorageGroupResult.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()
+ && setStorageGroupResult.getCode()
+ != TSStatusCode.PATH_ALREADY_EXIST_ERROR.getStatusCode()) {
+ throw new MetadataException(
+ String.format(
+ "Status Code: %d, failed to set storage group %s",
+ setStorageGroupResult.getCode(), storageGroup));
+ }
Review comment:
Hi, do we need to allow partial set storage group success? @LebronAl @mychaow @jt2594838
In the current implementation, if one of the storage groups is created failure, the remaining storage group will not be created.
----------------------------------------------------------------
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] commented on pull request #2731: [IOTDB-1173] auto-create schema improvement for createMultiTimeSeriesPaln
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #2731:
URL: https://github.com/apache/iotdb/pull/2731#issuecomment-785094154
SonarCloud Quality Gate failed.
[<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=2731&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=2731&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=2731&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=2731&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=2731&resolved=false&types=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=2731&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60.png' alt='63.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list) [63.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=2731&metric=new_duplicated_lines_density&view=list)
----------------------------------------------------------------
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