When the config node can not find the configuration file, the set configuration command does not update other nodes' configuration (#14390)

shuwenwei · web-flow · commit e2cd299d17a9 · 2024-12-13T10:48:20.000+08:00
* ignore undefined config items

* modify ConfigManager

* modify it

* check exception msg
diff --git a/integration-test/src/test/java/org/apache/iotdb/relational/it/db/it/IoTDBSetConfigurationTableIT.java b/integration-test/src/test/java/org/apache/iotdb/relational/it/db/it/IoTDBSetConfigurationTableIT.java
@@ -54,6 +54,41 @@ public static void tearDown() throws Exception {
     EnvFactory.getEnv().cleanClusterEnvironment();
   }
 
+  @Test
+  public void testSetConfigurationWithUndefinedConfigKey() {
+    String expectedExceptionMsg =
+        "301: ignored config items: [a] because they are immutable or undefined.";
+    try (Connection connection = EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT);
+        Statement statement = connection.createStatement()) {
+      executeAndExpectException(statement, "set configuration \"a\"='false'", expectedExceptionMsg);
+      int configNodeNum = EnvFactory.getEnv().getConfigNodeWrapperList().size();
+      int dataNodeNum = EnvFactory.getEnv().getDataNodeWrapperList().size();
+
+      for (int i = 0; i < configNodeNum; i++) {
+        executeAndExpectException(
+            statement, "set configuration a=\'false\' on " + i, expectedExceptionMsg);
+      }
+      for (int i = 0; i < dataNodeNum; i++) {
+        int dnId = configNodeNum + i;
+        executeAndExpectException(
+            statement, "set configuration \"a\"='false' on " + dnId, expectedExceptionMsg);
+      }
+    } catch (Exception e) {
+      Assert.fail(e.getMessage());
+    }
+  }
+
+  private void executeAndExpectException(
+      Statement statement, String sql, String expectedContentInExceptionMsg) {
+    try {
+      statement.execute(sql);
+    } catch (Exception e) {
+      Assert.assertTrue(e.getMessage().contains(expectedContentInExceptionMsg));
+      return;
+    }
+    Assert.fail();
+  }
+
   @Test
   public void testSetConfiguration() {
     try (Connection connection = EnvFactory.getEnv().getConnection(BaseEnv.TABLE_SQL_DIALECT);
@@ -69,7 +104,7 @@ public void testSetConfiguration() {
         int dnId = configNodeNum + i;
         statement.execute("set configuration \"enable_cross_space_compaction\"='false' on " + dnId);
         statement.execute(
-            "set configuration max_inner_compaction_candidate_file_num='1',max_cross_compaction_candidate_file_num='1' on "
+            "set configuration inner_compaction_candidate_file_num='1',max_cross_compaction_candidate_file_num='1' on "
                 + dnId);
       }
     } catch (Exception e) {
@@ -91,7 +126,7 @@ public void testSetConfiguration() {
                         nodeWrapper,
                         "enable_seq_space_compaction=false",
                         "enable_cross_space_compaction=false",
-                        "max_inner_compaction_candidate_file_num=1",
+                        "inner_compaction_candidate_file_num=1",
                         "max_cross_compaction_candidate_file_num=1")));
   }
 
diff --git a/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java b/iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java
@@ -1666,28 +1666,41 @@ public TSStatus clearCache(final Set<Integer> clearCacheOptions) {
   public TSStatus setConfiguration(TSetConfigurationReq req) {
     TSStatus tsStatus = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
     int currentNodeId = CONF.getConfigNodeId();
-    if (req.getNodeId() < 0 || currentNodeId == req.getNodeId()) {
-      URL url = ConfigNodeDescriptor.getPropsUrl(CommonConfig.SYSTEM_CONFIG_NAME);
-      if (url == null || !new File(url.getFile()).exists()) {
+    if (currentNodeId != req.getNodeId()) {
+      tsStatus = confirmLeader();
+      if (tsStatus.getCode() != TSStatusCode.SUCCESS_STATUS.getStatusCode()) {
         return tsStatus;
       }
-      File file = new File(url.getFile());
+    }
+    if (currentNodeId == req.getNodeId() || req.getNodeId() < 0) {
+      URL url = ConfigNodeDescriptor.getPropsUrl(CommonConfig.SYSTEM_CONFIG_NAME);
+      boolean configurationFileFound = (url != null && new File(url.getFile()).exists());
       TrimProperties properties = new TrimProperties();
       properties.putAll(req.getConfigs());
-      try {
-        ConfigurationFileUtils.updateConfigurationFile(file, properties);
-      } catch (Exception e) {
-        return RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, e.getMessage());
+
+      if (configurationFileFound) {
+        File file = new File(url.getFile());
+        try {
+          ConfigurationFileUtils.updateConfigurationFile(file, properties);
+        } catch (Exception e) {
+          tsStatus = RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, e.getMessage());
+        }
+      } else {
+        String msg =
+            "Unable to find the configuration file. Some modifications are made only in memory.";
+        tsStatus = RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, msg);
+        LOGGER.warn(msg);
       }
       ConfigNodeDescriptor.getInstance().loadHotModifiedProps(properties);
-      if (CONF.getConfigNodeId() == req.getNodeId()) {
+      if (currentNodeId == req.getNodeId()) {
         return tsStatus;
       }
     }
-    tsStatus = confirmLeader();
-    return tsStatus.getCode() == TSStatusCode.SUCCESS_STATUS.getStatusCode()
-        ? RpcUtils.squashResponseStatusList(nodeManager.setConfiguration(req))
-        : tsStatus;
+    List<TSStatus> statusListOfOtherNodes = nodeManager.setConfiguration(req);
+    List<TSStatus> statusList = new ArrayList<>(statusListOfOtherNodes.size() + 1);
+    statusList.add(tsStatus);
+    statusList.addAll(statusListOfOtherNodes);
+    return RpcUtils.squashResponseStatusList(statusList);
   }
 
   @Override
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
@@ -2944,7 +2944,7 @@ public synchronized void loadHotModifiedProps() throws QueryProcessException {
     try (InputStream inputStream = url.openStream()) {
       LOGGER.info("Start to reload config file {}", url);
       commonProperties.load(new InputStreamReader(inputStream, StandardCharsets.UTF_8));
-      ConfigurationFileUtils.getConfigurationDefaultValue();
+      ConfigurationFileUtils.loadConfigurationDefaultValueFromTemplate();
       loadHotModifiedProps(commonProperties);
     } catch (Exception e) {
       LOGGER.warn("Fail to reload config file {}", url, e);
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java
@@ -1142,11 +1142,14 @@ public SettableFuture<ConfigTaskResult> setConfiguration(TSetConfigurationReq re
 
     TSStatus tsStatus = new TSStatus(TSStatusCode.SUCCESS_STATUS.getStatusCode());
     List<String> ignoredConfigItems =
-        ConfigurationFileUtils.filterImmutableConfigItems(req.getConfigs());
+        ConfigurationFileUtils.filterInvalidConfigItems(req.getConfigs());
     TSStatus warningTsStatus = null;
     if (!ignoredConfigItems.isEmpty()) {
       warningTsStatus = new TSStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR.getStatusCode());
-      warningTsStatus.setMessage("ignored config items: " + ignoredConfigItems);
+      warningTsStatus.setMessage(
+          "ignored config items: "
+              + ignoredConfigItems
+              + " because they are immutable or undefined.");
       if (req.getConfigs().isEmpty()) {
         future.setException(new IoTDBException(warningTsStatus.message, warningTsStatus.code));
         return future;
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/StorageEngine.java
@@ -662,6 +662,10 @@ public TSStatus setConfiguration(TSetConfigurationReq req) {
     URL configFileUrl = IoTDBDescriptor.getPropsUrl(CommonConfig.SYSTEM_CONFIG_NAME);
     if (configFileUrl == null || !(new File(configFileUrl.getFile()).exists())) {
       // configuration file not exist, update in mem
+      String msg =
+          "Unable to find the configuration file. Some modifications are made only in memory.";
+      tsStatus = RpcUtils.getStatus(TSStatusCode.EXECUTE_STATEMENT_ERROR, msg);
+      LOGGER.warn(msg);
       try {
         IoTDBDescriptor.getInstance().loadHotModifiedProps(properties);
         IoTDBDescriptor.getInstance().reloadMetricProperties(properties);
diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/ConfigurationFileUtils.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/conf/ConfigurationFileUtils.java
@@ -158,7 +158,7 @@ public static String readConfigFileContent(URL url) throws IOException {
     return readConfigLines(f);
   }
 
-  public static void getConfigurationDefaultValue() throws IOException {
+  public static void loadConfigurationDefaultValueFromTemplate() throws IOException {
     if (configuration2DefaultValue != null) {
       return;
     }
@@ -185,6 +185,7 @@ public static void getConfigurationDefaultValue() throws IOException {
         }
       }
     } catch (IOException e) {
+      configuration2DefaultValue = null;
       logger.warn("Failed to read configuration template", e);
       throw e;
     }
@@ -197,7 +198,7 @@ public static String getConfigurationDefaultValue(String parameterName) throws I
     if (configuration2DefaultValue != null) {
       return configuration2DefaultValue.get(parameterName);
     } else {
-      getConfigurationDefaultValue();
+      loadConfigurationDefaultValueFromTemplate();
       return configuration2DefaultValue.getOrDefault(parameterName, null);
     }
   }
@@ -225,14 +226,24 @@ public static String readConfigurationTemplateFile() throws IOException {
     return content.toString();
   }
 
-  public static List<String> filterImmutableConfigItems(Map<String, String> configItems) {
+  public static List<String> filterInvalidConfigItems(Map<String, String> configItems) {
+    boolean successLoadDefaultValueMap = true;
+    try {
+      loadConfigurationDefaultValueFromTemplate();
+    } catch (IOException e) {
+      successLoadDefaultValueMap = false;
+    }
+
     List<String> ignoredConfigItems = new ArrayList<>();
-    for (String ignoredKey : ignoreConfigKeys) {
-      if (configItems.containsKey(ignoredKey)) {
-        configItems.remove(ignoredKey);
-        ignoredConfigItems.add(ignoredKey);
+    for (String key : configItems.keySet()) {
+      if (ignoreConfigKeys.contains(key)) {
+        ignoredConfigItems.add(key);
+      }
+      if (successLoadDefaultValueMap && !configuration2DefaultValue.containsKey(key)) {
+        ignoredConfigItems.add(key);
       }
     }
+    ignoredConfigItems.forEach(configItems::remove);
     return ignoredConfigItems;
   }
 

Original file line number	Diff line number	Diff line change
`@@ -158,7 +158,7 @@ public static String readConfigFileContent(URL url) throws IOException {`
`158`	`158`	`return readConfigLines(f);`
`159`	`159`	`}`
`160`	`160`
`161`		`- public static void getConfigurationDefaultValue() throws IOException {`
	`161`	`+ public static void loadConfigurationDefaultValueFromTemplate() throws IOException {`
`162`	`162`	`if (configuration2DefaultValue != null) {`
`163`	`163`	`return;`
`164`	`164`	`}`
`@@ -185,6 +185,7 @@ public static void getConfigurationDefaultValue() throws IOException {`
`185`	`185`	`}`
`186`	`186`	`}`
`187`	`187`	`} catch (IOException e) {`
	`188`	`+ configuration2DefaultValue = null;`
`188`	`189`	`logger.warn("Failed to read configuration template", e);`
`189`	`190`	`throw e;`
`190`	`191`	`}`
`@@ -197,7 +198,7 @@ public static String getConfigurationDefaultValue(String parameterName) throws I`
`197`	`198`	`if (configuration2DefaultValue != null) {`
`198`	`199`	`return configuration2DefaultValue.get(parameterName);`
`199`	`200`	`} else {`
`200`		`- getConfigurationDefaultValue();`
	`201`	`+ loadConfigurationDefaultValueFromTemplate();`
`201`	`202`	`return configuration2DefaultValue.getOrDefault(parameterName, null);`
`202`	`203`	`}`
`203`	`204`	`}`
`@@ -225,14 +226,24 @@ public static String readConfigurationTemplateFile() throws IOException {`
`225`	`226`	`return content.toString();`
`226`	`227`	`}`
`227`	`228`
`228`		`- public static List<String> filterImmutableConfigItems(Map<String, String> configItems) {`
	`229`	`+ public static List<String> filterInvalidConfigItems(Map<String, String> configItems) {`
	`230`	`+ boolean successLoadDefaultValueMap = true;`
	`231`	`+ try {`
	`232`	`+ loadConfigurationDefaultValueFromTemplate();`
	`233`	`+ } catch (IOException e) {`
	`234`	`+ successLoadDefaultValueMap = false;`
	`235`	`+ }`
	`236`	`+`
`229`	`237`	`List<String> ignoredConfigItems = new ArrayList<>();`
`230`		`- for (String ignoredKey : ignoreConfigKeys) {`
`231`		`- if (configItems.containsKey(ignoredKey)) {`
`232`		`- configItems.remove(ignoredKey);`
`233`		`- ignoredConfigItems.add(ignoredKey);`
	`238`	`+ for (String key : configItems.keySet()) {`
	`239`	`+ if (ignoreConfigKeys.contains(key)) {`
	`240`	`+ ignoredConfigItems.add(key);`
	`241`	`+ }`
	`242`	`+ if (successLoadDefaultValueMap && !configuration2DefaultValue.containsKey(key)) {`
	`243`	`+ ignoredConfigItems.add(key);`
`234`	`244`	`}`
`235`	`245`	`}`
	`246`	`+ ignoredConfigItems.forEach(configItems::remove);`
`236`	`247`	`return ignoredConfigItems;`
`237`	`248`	`}`
`238`	`249`