You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "JackieTien97 (via GitHub)" <gi...@apache.org> on 2026/04/27 02:55:48 UTC

[PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

JackieTien97 opened a new pull request, #17551:
URL: https://github.com/apache/iotdb/pull/17551

   ## Summary
   
   This PR cleans up dead/misconfigured RPC thread parameters and ensures ConfigNode and DataNode each use their own `selectorNumOfClientManager` value when constructing async client pools.
   
   ### 1. Fix config naming mismatch
   
   - Rename `dn_selector_thread_count_of_client_manager` → `dn_selector_thread_nums_of_client_manager` in both the properties template and `IoTDBDescriptor` to match the name already used in `CommonDescriptor`.
   
   ### 2. Remove dead config parameters
   
   - **`dn_rpc_selector_thread_count`**: Parsed and stored in `IoTDBConfig.rpcSelectorThreadCount` but never actually consumed by any server component. Removed the field, getter, setter, template entry, and loading code in `IoTDBDescriptor`.
   - **`coordinator_write_executor_size`**: Created a `writeOperationExecutor` thread pool in `Coordinator` that was passed to `TreeModelPlanner` but never referenced in any of its methods — completely dead code. Removed the config field, the thread pool, the `TreeModelPlanner` constructor parameter, and the `MPP_COORDINATOR_WRITE_EXECUTOR` thread name.
   
   ### 3. Fix `ClientPoolProperty.Builder` hardcoded default
   
   - The `Builder.maxClientNumForEachNode` was initialized to the compile-time constant `DefaultProperty.MAX_CLIENT_NUM_FOR_EACH_NODE` (1000), ignoring any runtime config override from `dn_max_client_count_for_each_node_in_client_manager` or `cn_max_client_count_for_each_node_in_client_manager`. Changed to read from `CommonDescriptor.getInstance().getConfig().getMaxClientNumForEachNode()` so callers that don't explicitly call `setMaxClientNumForEachNode()` still respect the configured value.
   
   ### 4. Use node-specific `selectorNum` for async client pools
   
   Previously, `AsyncDataNodeInternalServiceClientPoolFactory` always read `selectorNumOfClientManager` from `CommonConfig`, which is shared. ConfigNode and DataNode may configure different values via `cn_selector_thread_nums_of_client_manager` and `dn_selector_thread_nums_of_client_manager` respectively.
   
   Changes:
   - **`AsyncDataNodeInternalServiceClientPoolFactory`**: Removed the no-arg constructor; the only constructor now requires an explicit `selectorNumOfAsyncClientManager` int.
   - **`Coordinator`**: Passes `IoTDBConfig.getSelectorNumOfClientManager()` when constructing the factory.
   - **`DataNodeInternalServiceRequestManager`**: Added a `final int selectorNumOfAsyncClientManager` field and a `protected` constructor so each subclass supplies its own value.
   - **`DnToDnInternalServiceAsyncRequestManager`**: Reads from `IoTDBConfig.getSelectorNumOfClientManager()`.
   - **`CnToDnInternalServiceAsyncRequestManager`**: Reads from `ConfigNodeConfig.getSelectorNumOfClientManager()`.
   - **`ConfigNodeConfig`**: Added `selectorNumOfClientManager` field (default 1) with getter/setter.
   - **`ConfigNodeDescriptor`**: Loads `cn_selector_thread_nums_of_client_manager` into `ConfigNodeConfig`.
   
   ## Test plan
   
   - [ ] Verify the cluster starts successfully with default config (1C1D)
   - [ ] Verify custom `dn_selector_thread_nums_of_client_manager` / `cn_selector_thread_nums_of_client_manager` values take effect
   - [ ] Verify `dn_max_client_count_for_each_node_in_client_manager` is respected by client pools that don't explicitly set it


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #17551:
URL: https://github.com/apache/iotdb/pull/17551


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "CRZbulabula (via GitHub)" <gi...@apache.org>.
CRZbulabula commented on code in PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#discussion_r3200314070


##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/client/DataNodeClientPoolFactory.java:
##########
@@ -63,34 +63,6 @@ public GenericKeyedObjectPool<ConfigRegionId, ConfigNodeClient> createClientPool
     }
   }
 
-  public static class ClusterDeletionConfigNodeClientPoolFactory
-      implements IClientPoolFactory<ConfigRegionId, ConfigNodeClient> {
-
-    @Override
-    public GenericKeyedObjectPool<ConfigRegionId, ConfigNodeClient> createClientPool(
-        ClientManager<ConfigRegionId, ConfigNodeClient> manager) {
-      GenericKeyedObjectPool<ConfigRegionId, ConfigNodeClient> clientPool =
-          new GenericKeyedObjectPool<>(
-              new ConfigNodeClient.Factory(
-                  manager,
-                  new ThriftClientProperty.Builder()
-                      .setConnectionTimeoutMs(CONF.getConnectionTimeoutInMS() * 10)

Review Comment:
   This factory set `ConnectionTimeoutMs` to **10x** the normal value (`CONF.getConnectionTimeoutInMS() * 10`, i.e. ~600s vs 60s) for schema-modification RPCs (`deactivateSchemaTemplate`, `deleteTimeSeries`, `dropTable`, `deleteDevice`, `alterLogicalView`, etc.).
   
   All ~15 call sites in `ClusterConfigTaskExecutor` now use the standard `CONFIG_NODE_CLIENT_MANAGER` with a 60s timeout. This could cause timeout failures for long-running schema operations on large clusters.
   
   If the intent is to remove the separate pool, the base connection timeout should be validated as sufficient for these operations, or the timeout should be extended on these specific RPC calls.



##########
iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template:
##########
@@ -505,6 +506,14 @@ cn_selector_thread_nums_of_client_manager=1
 # Datatype: int
 cn_max_client_count_for_each_node_in_client_manager=1000
 
+# The maximum number of idle clients that can be retained for a node in a clientManager.
+# When the number of idle clients to a single node exceeds this number, excess idle clients will be evicted.
+# Idle clients are determined by a time threshold (default 1 minute of inactivity).
+# 0 means no idle clients will be retained, connections are destroyed immediately upon return.

Review Comment:
   The PR description says:
   > default `0`, meaning no idle eviction limit
   
   But the template defaults to `1000` (see the commented-out value below), and `GenericKeyedObjectPool.setMaxIdlePerKey(0)` actually means **destroy all idle connections immediately** — which is the opposite of "no limit."
   
   The template comment here is correct; the PR description should be fixed to avoid confusion.



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/client/property/ClientPoolProperty.java:
##########
@@ -49,7 +51,11 @@ public static class Builder<V> {
      * the maximum number of clients that can be allocated for a node. When some clients are idle
      * for more than {@code maxIdleTimeForClient}, they will be cleaned up.
      */
-    private int maxClientNumForEachNode = DefaultProperty.MAX_CLIENT_NUM_FOR_EACH_NODE;
+    private int maxClientNumForEachNode =
+        CommonDescriptor.getInstance().getConfig().getMaxClientNumForEachNode();

Review Comment:
   Reading from `CommonDescriptor.getInstance()` at field-initialization time couples this Builder to the singleton's lifecycle. If a `Builder` is ever constructed before `CommonDescriptor` has loaded its properties (e.g., during static init of an early-loaded class), it silently falls back to the field's Java default (1000) — which happens to match the old hardcoded constant, so it works today.
   
   Consider keeping the compile-time constant as the field default and relying on callers or a factory method to inject the configured value, which avoids the hidden ordering dependency.



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/client/property/ClientPoolProperty.java:
##########
@@ -87,7 +98,7 @@ public Builder<V> setTimeBetweenEvictionRuns(long timeBetweenEvictionRuns) {
     public ClientPoolProperty<V> build() {
       GenericKeyedObjectPoolConfig<V> poolConfig = new GenericKeyedObjectPoolConfig<>();
       poolConfig.setMaxTotalPerKey(maxClientNumForEachNode);

Review Comment:
   Previously `maxIdlePerKey` was set equal to `maxTotalPerKey` (= `maxClientNumForEachNode`). Now they can diverge, which is the intended behavior.
   
   Worth noting: if `maxIdleClientNumForEachNode > maxClientNumForEachNode`, the pool silently clamps idle to total — `GenericKeyedObjectPool` never keeps more idle objects than the total per key. Consider adding a validation or comment to prevent users from configuring a misleading idle > max combination.



##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java:
##########
@@ -291,11 +291,23 @@ public void loadProperties(TrimProperties properties) throws BadNodeUrlException
                 "dn_max_client_count_for_each_node_in_client_manager",
                 String.valueOf(conf.getMaxClientNumForEachNode()))));
 
-    conf.setSelectorNumOfClientManager(
+    int dnMaxIdleClientNumForEachNode =
         Integer.parseInt(
             properties.getProperty(
-                "dn_selector_thread_count_of_client_manager",
-                String.valueOf(conf.getSelectorNumOfClientManager()))));
+                "dn_max_idle_client_count_for_each_node_in_client_manager",
+                String.valueOf(conf.getMaxIdleClientNumForEachNode())));
+    if (dnMaxIdleClientNumForEachNode >= 0) {
+      conf.setMaxIdleClientNumForEachNode(dnMaxIdleClientNumForEachNode);
+    }
+
+    int dnSelectorNumOfClientManager =

Review Comment:
   Minor: `CommonConfig` and `ConfigNodeConfig` both change their `selectorNumOfClientManager` field default to `max(1, CPU/4)`, but `IoTDBConfig.selectorNumOfClientManager` doesn't appear to be updated in this PR (the field itself is not in the diff).
   
   The behavior is correct because the `> 0` guard here preserves the existing default when the template value is 0. But the field initialization in `IoTDBConfig` is inconsistent with the other two config classes — worth aligning to `max(1, CPU/4)` for clarity.



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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4388008286

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [5.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list) (required ≤ 5%)  
     
   <!-- sqra:qg_revision=a5c8ea78d3171c1e942b7f955590987278c58224de710e8375e88de096e49a25 -->
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4384419097

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=17551&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=17551&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=17551&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4387100170

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [5.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list) (required ≤ 5%)  
     
   <!-- sqra:qg_revision=a5c8ea78d3171c1e942b7f955590987278c58224de710e8375e88de096e49a25 -->
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4385095157

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [5.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list) (required ≤ 5%)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#discussion_r3198431986


##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/CommonDescriptor.java:
##########
@@ -170,13 +184,16 @@ public void loadCommonProps(TrimProperties properties) throws IOException {
                     String.valueOf(config.isRpcThriftCompressionEnabled()))
                 .trim()));
 
-    config.setSelectorNumOfClientManager(
+    int dnSelectorNumOfClientManager =
         Integer.parseInt(
             properties
                 .getProperty(
                     "dn_selector_thread_nums_of_client_manager",
                     String.valueOf(config.getSelectorNumOfClientManager()))
-                .trim()));
+                .trim());
+    if (dnSelectorNumOfClientManager > 0) {
+      config.setSelectorNumOfClientManager(dnSelectorNumOfClientManager);
+    }

Review Comment:
   `CommonConfig` only has a single `selectorNumOfClientManager` field, but `loadCommonProps` applies both `cn_selector_thread_nums_of_client_manager` and `dn_selector_thread_nums_of_client_manager` to it. When both keys exist, the dn value will overwrite the cn value, which defeats node-specific configuration and can produce surprising results for any code reading `CommonDescriptor.getConfig()` (e.g., ConfigNode code ending up with DataNode selector settings). Consider splitting into per-node fields in `CommonConfig`, or only loading the relevant prefix depending on the running node type, instead of overwriting the same field twice.



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/client/property/ClientPoolProperty.java:
##########
@@ -49,7 +51,11 @@ public static class Builder<V> {
      * the maximum number of clients that can be allocated for a node. When some clients are idle
      * for more than {@code maxIdleTimeForClient}, they will be cleaned up.
      */
-    private int maxClientNumForEachNode = DefaultProperty.MAX_CLIENT_NUM_FOR_EACH_NODE;
+    private int maxClientNumForEachNode =
+        CommonDescriptor.getInstance().getConfig().getMaxClientNumForEachNode();
+
+    private int maxIdleClientNumForEachNode =
+        CommonDescriptor.getInstance().getConfig().getMaxIdleClientNumForEachNode();
 

Review Comment:
   `ClientPoolProperty.Builder` now initializes defaults by reading `CommonDescriptor.getInstance().getConfig()`. This couples pool defaults to the global `CommonConfig`, which currently can’t represent separate cn/dn values (it gets overwritten during `loadCommonProps`) and can also make pool behavior depend on initialization order (builder created before config load will capture defaults). Prefer passing the effective per-node config explicitly (e.g., from `IoTDBConfig`/`ConfigNodeConfig`) or deferring lookup to `build()` so it reflects the final loaded config.



##########
iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/client/ClientManagerTest.java:
##########
@@ -632,7 +639,11 @@ public GenericKeyedObjectPool<TEndPoint, AsyncDataNodeInternalServiceClient> cre
               manager,
               new ThriftClientProperty.Builder().setConnectionTimeoutMs(CONNECTION_TIMEOUT).build(),
               ThreadName.ASYNC_DATANODE_CLIENT_POOL.getName()),
-          new ClientPoolProperty.Builder<AsyncDataNodeInternalServiceClient>().build().getConfig());
+          new ClientPoolProperty.Builder<AsyncDataNodeInternalServiceClient>()
+              .setMaxIdleClientNumForEachNode(
+                  ClientPoolProperty.DefaultProperty.MAX_CLIENT_NUM_FOR_EACH_NODE)
+              .build()
+              .getConfig());

Review Comment:
   These test pool factories now set `maxIdleClientNumForEachNode` but rely on `ClientPoolProperty.Builder`’s default for `maxClientNumForEachNode`, which is read from global `CommonDescriptor` config. That makes the test dependent on global configuration state and can become flaky if other tests mutate or load different common props. Set both `maxClientNumForEachNode` and `maxIdleClientNumForEachNode` explicitly in the test factories to keep the test self-contained.



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/CommonDescriptor.java:
##########
@@ -139,13 +139,16 @@ public void loadCommonProps(TrimProperties properties) throws IOException {
                     "cn_connection_timeout_ms", String.valueOf(config.getCnConnectionTimeoutInMS()))
                 .trim()));
 
-    config.setSelectorNumOfClientManager(
+    int cnSelectorNumOfClientManager =
         Integer.parseInt(
             properties
                 .getProperty(
                     "cn_selector_thread_nums_of_client_manager",
                     String.valueOf(config.getSelectorNumOfClientManager()))
-                .trim()));
+                .trim());
+    if (cnSelectorNumOfClientManager > 0) {
+      config.setSelectorNumOfClientManager(cnSelectorNumOfClientManager);

Review Comment:
   The new `<= 0` “auto-compute selector threads” behavior is implemented by *not* updating the config when the parsed value is <= 0, but `CommonConfig.selectorNumOfClientManager` still defaults to `1`. This means components that rely on `CommonDescriptor.getConfig().getSelectorNumOfClientManager()` won’t get the intended `max(1, CPU/4)` default when the properties file sets `0` (as in the template). Consider aligning `CommonConfig`’s default with the auto-compute logic, or computing a value here when the parsed value is <= 0.
   



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/client/property/ClientPoolProperty.java:
##########
@@ -87,7 +98,7 @@ public Builder<V> setTimeBetweenEvictionRuns(long timeBetweenEvictionRuns) {
     public ClientPoolProperty<V> build() {
       GenericKeyedObjectPoolConfig<V> poolConfig = new GenericKeyedObjectPoolConfig<>();
       poolConfig.setMaxTotalPerKey(maxClientNumForEachNode);
-      poolConfig.setMaxIdlePerKey(maxClientNumForEachNode);
+      poolConfig.setMaxIdlePerKey(maxIdleClientNumForEachNode);
       poolConfig.setTimeBetweenEvictionRuns(Duration.ofMillis(timeBetweenEvictionRuns));

Review Comment:
   With the new `maxIdleClientNumForEachNode` field defaulting to a constant (1000), overriding `maxClientNumForEachNode` without also overriding `maxIdleClientNumForEachNode` can unintentionally change behavior (e.g., maxTotalPerKey=2000 but maxIdlePerKey=1000, causing extra churn; or maxTotalPerKey<maxIdlePerKey, which is inconsistent). Consider defaulting `maxIdleClientNumForEachNode` to `maxClientNumForEachNode` unless the idle limit is explicitly configured, and/or clamping maxIdlePerKey to maxTotalPerKey during build.



##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/CommonDescriptor.java:
##########
@@ -186,6 +203,17 @@ public void loadCommonProps(TrimProperties properties) throws IOException {
                     String.valueOf(config.getMaxClientNumForEachNode()))
                 .trim()));
 
+    int dnMaxIdleClientNumForEachNode =
+        Integer.parseInt(
+            properties
+                .getProperty(
+                    "dn_max_idle_client_count_for_each_node_in_client_manager",
+                    String.valueOf(config.getMaxIdleClientNumForEachNode()))
+                .trim());
+    if (dnMaxIdleClientNumForEachNode >= 0) {
+      config.setMaxIdleClientNumForEachNode(dnMaxIdleClientNumForEachNode);
+    }

Review Comment:
   `loadCommonProps` writes both `cn_max_client_count_for_each_node_in_client_manager`/`cn_max_idle_client_count_for_each_node_in_client_manager` and the `dn_*` equivalents into the same `CommonConfig.maxClientNumForEachNode` / `maxIdleClientNumForEachNode` fields. If both cn_* and dn_* are set (the template exposes both), the later dn_* values will override the cn_* ones, so ConfigNode-side pools that rely on `CommonDescriptor` can’t reliably use cn-specific limits. Consider separating cn/dn limits in `CommonConfig` or loading only the relevant node’s keys.



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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4395301806

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [5.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list) (required ≤ 5%)  
     
   <!-- sqra:qg_revision=a5c8ea78d3171c1e942b7f955590987278c58224de710e8375e88de096e49a25 -->
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4323870649

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [2 New issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=17551&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_iotdb&pullRequest=17551&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_iotdb&pullRequest=17551&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4324021553

   ## [Codecov](https://app.codecov.io/gh/apache/iotdb/pull/17551?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `63.63636%` with `8 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 39.85%. Comparing base ([`859c719`](https://app.codecov.io/gh/apache/iotdb/commit/859c719b9a4a728b8785eb70ab5e176b445dbbf7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d171cd1`](https://app.codecov.io/gh/apache/iotdb/commit/d171cd1cdf1a44aee77842195603d23b1a18b85c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 3 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/iotdb/pull/17551?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...he/iotdb/confignode/conf/ConfigNodeDescriptor.java](https://app.codecov.io/gh/apache/iotdb/pull/17551?src=pr&el=tree&filepath=iotdb-core%2Fconfignode%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fiotdb%2Fconfignode%2Fconf%2FConfigNodeDescriptor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW90ZGItY29yZS9jb25maWdub2RlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9pb3RkYi9jb25maWdub2RlL2NvbmYvQ29uZmlnTm9kZURlc2NyaXB0b3IuamF2YQ==) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/iotdb/pull/17551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...apache/iotdb/confignode/conf/ConfigNodeConfig.java](https://app.codecov.io/gh/apache/iotdb/pull/17551?src=pr&el=tree&filepath=iotdb-core%2Fconfignode%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fiotdb%2Fconfignode%2Fconf%2FConfigNodeConfig.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW90ZGItY29yZS9jb25maWdub2RlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9pb3RkYi9jb25maWdub2RlL2NvbmYvQ29uZmlnTm9kZUNvbmZpZy5qYXZh) | 50.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/iotdb/pull/17551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...t/dn/DnToDnInternalServiceAsyncRequestManager.java](https://app.codecov.io/gh/apache/iotdb/pull/17551?src=pr&el=tree&filepath=iotdb-core%2Fdatanode%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fiotdb%2Fdb%2Fprotocol%2Fclient%2Fdn%2FDnToDnInternalServiceAsyncRequestManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW90ZGItY29yZS9kYXRhbm9kZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaW90ZGIvZGIvcHJvdG9jb2wvY2xpZW50L2RuL0RuVG9EbkludGVybmFsU2VydmljZUFzeW5jUmVxdWVzdE1hbmFnZXIuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/iotdb/pull/17551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #17551      +/-   ##
   ============================================
   + Coverage     39.83%   39.85%   +0.01%     
   - Complexity     2539     2547       +8     
   ============================================
     Files          5171     5171              
     Lines        348095   348165      +70     
     Branches      44474    44488      +14     
   ============================================
   + Hits         138677   138764      +87     
   + Misses       209418   209401      -17     
   ```
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/iotdb/pull/17551?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   </details>


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4385063714

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [5.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list) (required ≤ 5%)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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


Re: [PR] Clean up dead RPC thread config and use node-specific selectorNum [iotdb]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #17551:
URL: https://github.com/apache/iotdb/pull/17551#issuecomment-4394049728

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [5.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_iotdb&pullRequest=17551&metric=new_duplicated_lines_density&view=list) (required ≤ 5%)  
     
   <!-- sqra:qg_revision=a5c8ea78d3171c1e942b7f955590987278c58224de710e8375e88de096e49a25 -->
   <!-- sqra-placement-anchor -->
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_iotdb&pullRequest=17551)
   
   


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