[GitHub] [iotdb] LebronAl commented on a change in pull request #2950: [IOTDB-1263] optimize matched paths

[GitBox <gi...@apache.org>]

LebronAl commented on a change in pull request #2950:
URL: https://github.com/apache/iotdb/pull/2950#discussion_r634160228



##########
File path: cluster/src/assembly/resources/conf/iotdb-cluster.properties
##########
@@ -175,4 +175,16 @@ default_replica_num=1
 # If the number of connections created for a node exceeds  `max_client_pernode_permember_number`,
 # we need to wait so much time for other connections to be released until timeout,
 # or a new connection will be created.
-# wait_client_timeout_ms=5000
\ No newline at end of file
+#wait_client_timeout_ms=5000
+
+# If the path is not updated frequently, you can enable cache to update the path every 1s
+# improving the query performance by 14%
+enable_query_paths_cache=false
+
+# When enable_query_paths_cache is true, this parameter effective.

Review comment:
       grammer error? `this parameter effective`

##########
File path: cluster/src/assembly/resources/conf/iotdb-cluster.properties
##########
@@ -175,4 +175,16 @@ default_replica_num=1
 # If the number of connections created for a node exceeds  `max_client_pernode_permember_number`,
 # we need to wait so much time for other connections to be released until timeout,
 # or a new connection will be created.
-# wait_client_timeout_ms=5000
\ No newline at end of file
+#wait_client_timeout_ms=5000
+
+# If the path is not updated frequently, you can enable cache to update the path every 1s
+# improving the query performance by 14%
+enable_query_paths_cache=false
+
+# When enable_query_paths_cache is true, this parameter effective.
+# query paths cache size, default value is 1000
+query_paths_cache_size=1000
+
+# When enable_query_paths_cache is true, this parameter effective.

Review comment:
       grammer error? `this parameter effective`

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1283,8 +1449,29 @@ private boolean tryPullTimeSeriesSchemas(Node node, PullSchemaRequest request) {
     List<Node> coordinatedNodes = QueryCoordinator.getINSTANCE().reorderNodes(partitionGroup);
     for (Node node : coordinatedNodes) {
       try {
-        List<PartialPath> paths =
+        GetAllPathsResult result =
             getMatchedPaths(node, partitionGroup.getHeader(), pathsToQuery, withAlias);
+
+        List<PartialPath> partialPaths = new ArrayList<>();
+        if (result != null) {
+          result.pathToPaths.forEach(
+              (k, v) -> {
+                v.forEach(
+                    p -> {

Review comment:
       In fact, when `vector` functionality is added, using partialPath directly here may not work for all paths.
   maybe we need a detailed discussion to adapt Vector and CMManager

##########
File path: cluster/src/assembly/resources/conf/iotdb-cluster.properties
##########
@@ -175,4 +175,16 @@ default_replica_num=1
 # If the number of connections created for a node exceeds  `max_client_pernode_permember_number`,
 # we need to wait so much time for other connections to be released until timeout,
 # or a new connection will be created.
-# wait_client_timeout_ms=5000
\ No newline at end of file
+#wait_client_timeout_ms=5000
+
+# If the path is not updated frequently, you can enable cache to update the path every 1s

Review comment:
       1s -> query_paths_cache_update_interval?
   path -> paths?

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -551,6 +564,135 @@ public synchronized boolean containsKey(PartialPath key) {
     }
   }
 
+  private class RemotePathAliasCache extends LRUCache<String, List<PartialPath>> {
+    private long valueSizeThreshold;
+    private AtomicLong valueSize = new AtomicLong(0);
+
+    // period executor thread

Review comment:
       `period` -> `periodic`

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/config/ClusterConfig.java
##########
@@ -184,6 +184,24 @@ public ClusterConfig() {
     seedNodeUrls = Arrays.asList(String.format("%s:%d", internalIp, internalMetaPort));
   }
 
+  /**
+   * If the path is not updated frequently, you can enable cache to update the path every 1s
+   * improving the query performance by 14%
+   */
+  private boolean enableQueryPathsCache = false;
+
+  /**
+   * When enable_query_paths_cache is true, this parameter effective. query paths cache size,

Review comment:
       grammer error? `this parameter effective`
   what's this ? `query paths cache size`

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -551,6 +564,135 @@ public synchronized boolean containsKey(PartialPath key) {
     }
   }
 
+  private class RemotePathAliasCache extends LRUCache<String, List<PartialPath>> {
+    private long valueSizeThreshold;
+    private AtomicLong valueSize = new AtomicLong(0);
+
+    // period executor thread
+    private ScheduledExecutorService periodExecutorService;
+
+    // executor every partition group update cache

Review comment:
       what's this?

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/config/ClusterConfig.java
##########
@@ -184,6 +184,24 @@ public ClusterConfig() {
     seedNodeUrls = Arrays.asList(String.format("%s:%d", internalIp, internalMetaPort));
   }
 
+  /**
+   * If the path is not updated frequently, you can enable cache to update the path every 1s

Review comment:
       same as above




-- 
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