Uncomment commented set-configuration-able items in the config template (fix enable_topology_probing & topology_probing_* hot/restart reload) (#17933)

Author: CRZbulabula

integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java

@@ -88,6 +88,35 @@ public void testSetConfiguration() {
                         "enable_cross_space_compaction=false")));
   }
 
+  /**
+   * Regression test for V2-995: the topology-probing config items were left commented out in the
+   * template, so {@code getConfigurationItemsFromTemplate} never recorded them as known defaults
+   * and {@code set configuration} rejected them as "immutable or undefined". After uncommenting
+   * them, {@code set configuration} must accept and persist them: {@code enable_topology_probing}
+   * (hot_reload) and {@code topology_probing_base_interval_in_ms} / {@code
+   * topology_probing_timeout_ratio} (restart).
+   */
+  @Test
+  public void testSetTopologyProbingConfiguration() {
+    try (Connection connection = EnvFactory.getEnv().getConnection();
+        Statement statement = connection.createStatement()) {
+      statement.execute("set configuration \"enable_topology_probing\"=\"true\"");
+      statement.execute("set configuration \"topology_probing_base_interval_in_ms\"=\"3000\"");
+      statement.execute("set configuration \"topology_probing_timeout_ratio\"=\"0.4\"");
+    } catch (Exception e) {
+      Assert.fail(e.getMessage());
+    }
+    Assert.assertTrue(
+        EnvFactory.getEnv().getConfigNodeWrapperList().stream()
+            .allMatch(
+                nodeWrapper ->
+                    checkConfigFileContains(
+                        nodeWrapper,
+                        "enable_topology_probing=true",
+                        "topology_probing_base_interval_in_ms=3000",
+                        "topology_probing_timeout_ratio=0.4")));
+  }
+
   @Test
   public void testSetClusterName() throws Exception {
     // set cluster name on cn and dn
@@ -207,6 +236,14 @@ public void testSetDefaultSGLevel() throws SQLException {
             "509: An error occurred when executing getDeviceToDatabase():root.db1 is not a legal path, because it is no longer than default sg level: 3",
             e.getMessage());
       }
+    } finally {
+      // This test leaves default_database_level=-1 baked into the cluster's config file. Because
+      // tests in this class share one cluster and run in random order, a later 'set configuration'
+      // test would otherwise fail its hot-reload while re-validating the leftover illegal value
+      // ("Illegal defaultDatabaseLevel: -1, should >= 1"). Restore a clean cluster before leaving.
+      EnvFactory.getEnv().cleanClusterEnvironment();
+      EnvFactory.getEnv().getConfig().getCommonConfig().setDefaultDatabaseLevel(1);
+      EnvFactory.getEnv().initClusterEnvironment();
     }
   }
 }

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/utils/ConfigurationFileUtilsTest.java

@@ -19,20 +19,26 @@
 
 package org.apache.iotdb.db.utils;
 
+import org.apache.iotdb.commons.conf.CommonConfig;
 import org.apache.iotdb.commons.conf.ConfigurationFileUtils;
 import org.apache.iotdb.db.utils.constant.TestConstant;
 
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.nio.file.Files;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
+import java.util.Set;
 
 public class ConfigurationFileUtilsTest {
 
@@ -89,6 +95,82 @@ public void checkIoTDBSystemTemplateFileFormat() throws IOException {
     }
   }
 
+  /**
+   * Regression guard for V2-995: a configuration item whose {@code effectiveMode} allows {@code set
+   * configuration} (i.e. anything except {@code FIRST_START}) must have its {@code key=value} line
+   * left uncommented in the template. {@code getConfigurationItemsFromTemplate} only parses
+   * uncommented lines, so a commented item never enters {@code configuration2DefaultValue}; {@code
+   * filterInvalidConfigItems} then treats it as undefined and {@code set configuration} rejects it
+   * with "immutable or undefined" — silently disabling the advertised dynamic-config entry point.
+   * This was the root cause for {@code enable_topology_probing} (hot_reload) and the two {@code
+   * topology_probing_*} items (restart).
+   */
+  @Test
+  public void checkSettableItemsAreUncommentedInTemplate() throws IOException {
+    // Keys whose value line appears uncommented at least once. Some keys (e.g. dn_data_dirs) list a
+    // commented Windows-path variant next to an uncommented Unix-path variant; only the uncommented
+    // one is parsed, so such keys are fine and must not be flagged.
+    Set<String> uncommentedKeys = new HashSet<>();
+    // Keys with a commented value line whose enclosing block declares a settable effectiveMode.
+    Set<String> commentedSettableKeys = new HashSet<>();
+    try (InputStream inputStream =
+            ConfigurationFileUtilsTest.class
+                .getClassLoader()
+                .getResourceAsStream(CommonConfig.SYSTEM_CONFIG_TEMPLATE_NAME);
+        BufferedReader reader =
+            new BufferedReader(new InputStreamReader(Objects.requireNonNull(inputStream)))) {
+      String line;
+      ConfigurationFileUtils.EffectiveModeType currentMode = null;
+      while ((line = reader.readLine()) != null) {
+        line = line.trim();
+        if (line.isEmpty()) {
+          // Blank line separates configuration blocks; reset the accumulated effectiveMode.
+          currentMode = null;
+          continue;
+        }
+        if (line.startsWith("# effectiveMode:")) {
+          currentMode =
+              ConfigurationFileUtils.EffectiveModeType.getEffectiveMode(
+                  line.substring("# effectiveMode:".length()).trim());
+          continue;
+        }
+        // Detect the (possibly commented) "key=value" line that closes a block.
+        String stripped = line.startsWith("#") ? line.substring(1).trim() : line;
+        int equalsIndex = stripped.indexOf('=');
+        boolean isValueLine =
+            equalsIndex > 0 && stripped.substring(0, equalsIndex).trim().matches("[a-zA-Z0-9_.]+");
+        if (!isValueLine) {
+          continue;
+        }
+        String key = stripped.substring(0, equalsIndex).trim();
+        if (line.startsWith("#")) {
+          // FIRST_START items are intentionally rejected by 'set configuration'; UNKNOWN items have
+          // no declared mode. Only flag items that 'set configuration' is supposed to accept.
+          if (isSettableByConfiguration(currentMode)) {
+            commentedSettableKeys.add(key);
+          }
+        } else {
+          uncommentedKeys.add(key);
+        }
+        // A value line ends the current block's effectiveMode scope.
+        currentMode = null;
+      }
+    }
+    commentedSettableKeys.removeAll(uncommentedKeys);
+    Assert.assertTrue(
+        "configuration items settable via 'set configuration' must be uncommented in the template "
+            + "so the command can reach them instead of rejecting them as undefined; "
+            + "commented settable items found: "
+            + commentedSettableKeys,
+        commentedSettableKeys.isEmpty());
+  }
+
+  private static boolean isSettableByConfiguration(ConfigurationFileUtils.EffectiveModeType mode) {
+    return mode == ConfigurationFileUtils.EffectiveModeType.HOT_RELOAD
+        || mode == ConfigurationFileUtils.EffectiveModeType.RESTART
+        || mode == ConfigurationFileUtils.EffectiveModeType.FIRST_START_OR_SET_CONFIGURATION;
+  }
+
   private void generateFile(File file, String content) throws IOException {
     Files.write(file.toPath(), content.getBytes());
   }

iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties.template

@@ -512,7 +512,7 @@ cn_max_client_count_for_each_node_in_client_manager=1000
 # 0 means no idle clients will be retained, connections are destroyed immediately upon return.
 # effectiveMode: restart
 # Datatype: int
-# cn_max_idle_client_count_for_each_node_in_client_manager=1000
+cn_max_idle_client_count_for_each_node_in_client_manager=1000
 
 # The maximum session idle time. unit: ms
 # Idle sessions are the ones that performs neither query or non-query operations for a period of time
@@ -571,7 +571,7 @@ dn_max_client_count_for_each_node_in_client_manager=1000
 # 0 means no idle clients will be retained, connections are destroyed immediately upon return.
 # effectiveMode: restart
 # Datatype: int
-# dn_max_idle_client_count_for_each_node_in_client_manager=1000
+dn_max_idle_client_count_for_each_node_in_client_manager=1000
 
 ####################
 ### REST Service Configuration
@@ -686,7 +686,7 @@ data_region_per_data_node=0
 # 3. PGP (Partite-Graph Placement; based on the PGP paper, with database-aware balance)
 # effectiveMode: restart
 # Datatype: String
-# region_group_allocate_policy=GCR
+region_group_allocate_policy=GCR
 
 # Whether to enable auto leader balance for Ratis consensus protocol.
 # The ConfigNode-leader will balance the leader of Ratis-RegionGroups by leader_distribution_policy if set true.
@@ -747,17 +747,17 @@ failure_detector_phi_acceptable_pause_in_ms=10000
 # Whether to enable topology probing between DataNodes
 # effectiveMode: hot_reload
 # Datatype: Boolean
-# enable_topology_probing=false
+enable_topology_probing=false
 
 # Base interval in ms for topology probing between DataNodes
 # effectiveMode: restart
 # Datatype: long
-# topology_probing_base_interval_in_ms=5000
+topology_probing_base_interval_in_ms=5000
 
 # Ratio of probing timeout to probing interval (must be less than 1.0)
 # effectiveMode: restart
 # Datatype: double
-# topology_probing_timeout_ratio=0.5
+topology_probing_timeout_ratio=0.5
 
 # Disk remaining threshold at which DataNode is set to ReadOnly status
 # effectiveMode: restart