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/03/01 02:39:46 UTC
[GitHub] [iotdb] jt2594838 commented on a change in pull request #5125: [IOTDB-2611] Fix incorrect result of COUNT NODES in cluster
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