This is an automated email from the ASF dual-hosted git repository.

jiangtian pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/master by this push:
     new e2cd299d17a When the config node can not find the configuration file, 
the set configuration command does not update other nodes' configuration 
(#14390)
e2cd299d17a is described below

commit e2cd299d17a93390fef1ce1a77666e9536596f6d
Author: shuwenwei <[email protected]>
AuthorDate: Fri Dec 13 10:48:20 2024 +0800

    When the config node can not find the configuration file, the set 
configuration command does not update other nodes' configuration (#14390)
    
    * ignore undefined config items
    
    * modify ConfigManager
    
    * modify it
    
    * check exception msg
---
 .../it/db/it/IoTDBSetConfigurationTableIT.java     | 39 ++++++++++++++++++++--
 .../iotdb/confignode/manager/ConfigManager.java    | 39 ++++++++++++++--------
 .../org/apache/iotdb/db/conf/IoTDBDescriptor.java  |  2 +-
 .../config/executor/ClusterConfigTaskExecutor.java |  7 ++--
 .../iotdb/db/storageengine/StorageEngine.java      |  4 +++
 .../iotdb/commons/conf/ConfigurationFileUtils.java | 25 ++++++++++----
 6 files changed, 91 insertions(+), 25 deletions(-)

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
index e4467363f5d..8a8e0e5e08d 100644
--- 
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 class IoTDBSetConfigurationTableIT {
     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 class IoTDBSetConfigurationTableIT {
         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 class IoTDBSetConfigurationTableIT {
                         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
index 43383319f85..06d256f236e 100644
--- 
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 class ConfigManager implements IManager {
   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
index 6dfd5c85ab1..2264cb5ba3b 100755
--- 
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 class IoTDBDescriptor {
     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
index 3dc8af00917..0ee63cd981a 100644
--- 
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 class ClusterConfigTaskExecutor implements 
IConfigTaskExecutor {
 
     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
index 87679e32a8d..6284c70a5c3 100644
--- 
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 class StorageEngine implements IService {
     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
index 0b86d5e48c6..0857b493b9a 100644
--- 
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 class ConfigurationFileUtils {
     return readConfigLines(f);
   }
 
-  public static void getConfigurationDefaultValue() throws IOException {
+  public static void loadConfigurationDefaultValueFromTemplate() throws 
IOException {
     if (configuration2DefaultValue != null) {
       return;
     }
@@ -185,6 +185,7 @@ public class ConfigurationFileUtils {
         }
       }
     } catch (IOException e) {
+      configuration2DefaultValue = null;
       logger.warn("Failed to read configuration template", e);
       throw e;
     }
@@ -197,7 +198,7 @@ public class ConfigurationFileUtils {
     if (configuration2DefaultValue != null) {
       return configuration2DefaultValue.get(parameterName);
     } else {
-      getConfigurationDefaultValue();
+      loadConfigurationDefaultValueFromTemplate();
       return configuration2DefaultValue.getOrDefault(parameterName, null);
     }
   }
@@ -225,14 +226,24 @@ public class ConfigurationFileUtils {
     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;
   }
 

Reply via email to