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 2022/02/26 11:45:22 UTC
[GitHub] [iotdb] ericpai opened a new pull request #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
ericpai opened a new pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125
See JIRA: https://issues.apache.org/jira/browse/IOTDB-2611
--
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 #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#issuecomment-1052115629
[](https://coveralls.io/builds/46905001)
Coverage increased (+0.02%) to 67.809% when pulling **2c6732f164ed113dda05e01fc96719928fc9d970 on ericpai:bugfix/iotdb-** into **b954d0f74f1f84c56cfb51a049c5a30bf77e8c4e 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 #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#issuecomment-1052115629
[](https://coveralls.io/builds/46905238)
Coverage remained the same at 67.791% when pulling **648411d080319e7e08bb7cd4df72de10d0fad821 on ericpai:bugfix/iotdb-** into **b954d0f74f1f84c56cfb51a049c5a30bf77e8c4e on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] coveralls commented on pull request #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#issuecomment-1052115629
[](https://coveralls.io/builds/46904602)
Coverage increased (+0.007%) to 67.798% when pulling **f027afdac941e3ca030ced9aeabf400425e8bc5e on ericpai:bugfix/iotdb-** into **b954d0f74f1f84c56cfb51a049c5a30bf77e8c4e 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] jt2594838 commented on a change in pull request #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#discussion_r816404033
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanExecutor.java
##########
@@ -322,59 +324,82 @@ protected int getNodesNumInGivenLevel(PartialPath path, int level) throws Metada
throw new MetadataException(e);
}
- // Here we append a ** to the path to query the storage groups which have the prefix as 'path',
- // if path doesn't end with **.
+ // Here we append a ** to the path to query the storage groups which have the prefix as 'path'.
+ // If path doesn't have a **, we append one ** to the query path to avoid missing storage
+ // groups.
// e.g. we have SG root.sg.a and root.sg.b, the query path is root.sg, we should return the map
// with key root.sg.a and root.sg.b instead of an empty one.
- PartialPath wildcardPath = path;
- if (!wildcardPath.getMeasurement().equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)) {
- wildcardPath = wildcardPath.concatNode(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD);
+ boolean hasMultiLevelWildcard = false;
+ for (String node : path.getNodes()) {
+ if (node.equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)) {
+ hasMultiLevelWildcard = true;
+ break;
+ }
+ }
+ Map<String, List<PartialPath>> sgPathMap;
+ if (!hasMultiLevelWildcard) {
+ Map<String, List<PartialPath>> extendedSgPathMap =
+ IoTDB.metaManager.groupPathByStorageGroup(
+ path.concatNode(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD));
+ sgPathMap = new HashMap<>(extendedSgPathMap.size());
+ // We should remove the last ** to restore the pattern to make it equivalent with the query.
+ for (Entry<String, List<PartialPath>> extendSgPaths : extendedSgPathMap.entrySet()) {
+ List<PartialPath> validPaths =
+ extendSgPaths.getValue().stream()
+ .map(PartialPath::getDevicePath)
+ .collect(Collectors.toList());
+ sgPathMap.put(extendSgPaths.getKey(), validPaths);
+ }
+
+ } else {
+ sgPathMap = IoTDB.metaManager.groupPathByStorageGroup(path);
}
- Map<String, List<PartialPath>> sgPathMap =
- IoTDB.metaManager.groupPathByStorageGroup(wildcardPath);
if (sgPathMap.isEmpty()) {
return 0;
}
logger.debug("The storage groups of path {} are {}", path, sgPathMap.keySet());
int ret = 0;
try {
- // level >= 0 is the COUNT NODE query
+ Map<PartialPath, Integer> detailedPaths = getPathCount(sgPathMap, level);
if (level >= 0) {
- int prefixPartIdx = 0;
- for (; prefixPartIdx < path.getNodeLength(); prefixPartIdx++) {
- String currentPart = path.getNodes()[prefixPartIdx];
- if (currentPart.equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)) {
- break;
- } else if (currentPart.equals(IoTDBConstant.ONE_LEVEL_PATH_WILDCARD)) {
- // Only level equals the first * occurred level, e.g. root.sg.d1.* and level = 4, the
- // query makes sense.
- if (level != prefixPartIdx) {
- return 0;
+ // level >= 0 is the COUNT NODE query.
+ // If one pattern has a prefix with level being not greater than the query level and without
+ // any wildcards, it will only contribute the result with 1, and the same prefix should
+ // contribute only once.
+ // The visitedTrivialPaths is used to record the ones which have already contributed 1.
+ Set<PartialPath> visitedTrivialPaths = new HashSet<>();
+ for (Entry<PartialPath, Integer> entry : detailedPaths.entrySet()) {
+ PartialPath pattern = entry.getKey();
+ int count = entry.getValue();
+ if (count == 0) {
+ continue;
+ }
+ boolean hasWildcard = false;
+ int targetLevel = Math.min(pattern.getNodeLength(), level);
+ for (int i = 0; i < targetLevel; i++) {
+ if (pattern.getNodes()[i].equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)
+ || pattern.getNodes()[i].equals(IoTDBConstant.ONE_LEVEL_PATH_WILDCARD)) {
+ hasWildcard = true;
+ break;
}
- break;
}
Review comment:
I think this may be unnecessarily complicated. Since we know the storage groups and what nodes are managing them, we can simply send the original query to them, only with specific storage groups.
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanExecutor.java
##########
@@ -322,59 +324,82 @@ protected int getNodesNumInGivenLevel(PartialPath path, int level) throws Metada
throw new MetadataException(e);
}
- // Here we append a ** to the path to query the storage groups which have the prefix as 'path',
- // if path doesn't end with **.
+ // Here we append a ** to the path to query the storage groups which have the prefix as 'path'.
+ // If path doesn't have a **, we append one ** to the query path to avoid missing storage
+ // groups.
// e.g. we have SG root.sg.a and root.sg.b, the query path is root.sg, we should return the map
// with key root.sg.a and root.sg.b instead of an empty one.
- PartialPath wildcardPath = path;
- if (!wildcardPath.getMeasurement().equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)) {
- wildcardPath = wildcardPath.concatNode(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD);
+ boolean hasMultiLevelWildcard = false;
+ for (String node : path.getNodes()) {
+ if (node.equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)) {
+ hasMultiLevelWildcard = true;
+ break;
+ }
+ }
+ Map<String, List<PartialPath>> sgPathMap;
+ if (!hasMultiLevelWildcard) {
+ Map<String, List<PartialPath>> extendedSgPathMap =
+ IoTDB.metaManager.groupPathByStorageGroup(
+ path.concatNode(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD));
+ sgPathMap = new HashMap<>(extendedSgPathMap.size());
+ // We should remove the last ** to restore the pattern to make it equivalent with the query.
+ for (Entry<String, List<PartialPath>> extendSgPaths : extendedSgPathMap.entrySet()) {
+ List<PartialPath> validPaths =
+ extendSgPaths.getValue().stream()
+ .map(PartialPath::getDevicePath)
+ .collect(Collectors.toList());
+ sgPathMap.put(extendSgPaths.getKey(), validPaths);
+ }
+
Review comment:
Why is this necessary? If the query does not have a wildcard, we should match it strictly according to it like other queries.
--
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 #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#issuecomment-1052115629
[](https://coveralls.io/builds/46969892)
Coverage decreased (-0.001%) to 67.747% when pulling **bcfa1ce1a4105580328230921304aecc9b40d26d on ericpai:bugfix/iotdb-** into **6fe4b9196515244431388bab9ee8a48bf29ce787 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 #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#issuecomment-1052115629
[](https://coveralls.io/builds/46905479)
Coverage increased (+0.01%) to 67.803% when pulling **648411d080319e7e08bb7cd4df72de10d0fad821 on ericpai:bugfix/iotdb-** into **b954d0f74f1f84c56cfb51a049c5a30bf77e8c4e on apache:master**.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [iotdb] ericpai commented on a change in pull request #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
ericpai commented on a change in pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#discussion_r816792849
##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/query/ClusterPlanExecutor.java
##########
@@ -322,59 +324,82 @@ protected int getNodesNumInGivenLevel(PartialPath path, int level) throws Metada
throw new MetadataException(e);
}
- // Here we append a ** to the path to query the storage groups which have the prefix as 'path',
- // if path doesn't end with **.
+ // Here we append a ** to the path to query the storage groups which have the prefix as 'path'.
+ // If path doesn't have a **, we append one ** to the query path to avoid missing storage
+ // groups.
// e.g. we have SG root.sg.a and root.sg.b, the query path is root.sg, we should return the map
// with key root.sg.a and root.sg.b instead of an empty one.
- PartialPath wildcardPath = path;
- if (!wildcardPath.getMeasurement().equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)) {
- wildcardPath = wildcardPath.concatNode(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD);
+ boolean hasMultiLevelWildcard = false;
+ for (String node : path.getNodes()) {
+ if (node.equals(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD)) {
+ hasMultiLevelWildcard = true;
+ break;
+ }
+ }
+ Map<String, List<PartialPath>> sgPathMap;
+ if (!hasMultiLevelWildcard) {
+ Map<String, List<PartialPath>> extendedSgPathMap =
+ IoTDB.metaManager.groupPathByStorageGroup(
+ path.concatNode(IoTDBConstant.MULTI_LEVEL_PATH_WILDCARD));
+ sgPathMap = new HashMap<>(extendedSgPathMap.size());
+ // We should remove the last ** to restore the pattern to make it equivalent with the query.
+ for (Entry<String, List<PartialPath>> extendSgPaths : extendedSgPathMap.entrySet()) {
+ List<PartialPath> validPaths =
+ extendSgPaths.getValue().stream()
+ .map(PartialPath::getDevicePath)
+ .collect(Collectors.toList());
+ sgPathMap.put(extendSgPaths.getKey(), validPaths);
+ }
+
Review comment:
Thanks for your comments. As we discussed online, I will refine those codes in a reasonable way soon.
--
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 #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5125:
URL: https://github.com/apache/iotdb/pull/5125#issuecomment-1052115629
[](https://coveralls.io/builds/46904619)
Coverage decreased (-0.01%) to 67.779% when pulling **f027afdac941e3ca030ced9aeabf400425e8bc5e on ericpai:bugfix/iotdb-** into **b954d0f74f1f84c56cfb51a049c5a30bf77e8c4e 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