[to dev/1.3] When the config node can not find the configuration file, the set configuration command does not update other nodes' configuration

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

@@ -20,6 +20,7 @@
 package org.apache.iotdb.db.it;
 
 import org.apache.iotdb.commons.conf.CommonConfig;
+import org.apache.iotdb.commons.conf.ConfigurationFileUtils;
 import org.apache.iotdb.it.env.EnvFactory;
 import org.apache.iotdb.it.env.cluster.node.AbstractNodeWrapper;
 import org.apache.iotdb.it.framework.IoTDBTestRunner;
@@ -40,6 +41,7 @@
 import java.sql.ResultSet;
 import java.sql.Statement;
 import java.util.Arrays;
+import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
 @RunWith(IoTDBTestRunner.class)
@@ -55,6 +57,42 @@ 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();
+        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();
@@ -119,24 +157,49 @@ public void testSetClusterName() throws Exception {
     Awaitility.await()
         .atMost(10, TimeUnit.SECONDS)
         .until(() -> !EnvFactory.getEnv().getDataNodeWrapper(0).isAlive());
+    AbstractNodeWrapper datanode = EnvFactory.getEnv().getDataNodeWrapper(0);
     Assert.assertTrue(
         checkConfigFileContains(EnvFactory.getEnv().getDataNodeWrapper(0), "cluster_name=yy"));
+
+    // Modify the config file manually because the datanode can not restart
+    Properties properties = new Properties();
+    properties.put("cluster_name", "xx");
+    ConfigurationFileUtils.updateConfigurationFile(getConfigFile(datanode), properties);
+    EnvFactory.getEnv().getDataNodeWrapper(0).stop();
+    EnvFactory.getEnv().getDataNodeWrapper(0).start();
+    // wait the datanode restart successfully (won't do any meaningful modification)
+    Awaitility.await()
+        .atMost(30, TimeUnit.SECONDS)
+        .pollDelay(1, TimeUnit.SECONDS)
+        .until(
+            () -> {
+              try (Connection connection = EnvFactory.getEnv().getConnection();
+                  Statement statement = connection.createStatement()) {
+                statement.execute("set configuration \"cluster_name\"=\"xx\" on 1");
+              } catch (Exception e) {
+                return false;
+              }
+              return true;
+            });
   }
 
   private static boolean checkConfigFileContains(
       AbstractNodeWrapper nodeWrapper, String... contents) {
     try {
-      String systemPropertiesPath =
-          nodeWrapper.getNodePath()
-              + File.separator
-              + "conf"
-              + File.separator
-              + CommonConfig.SYSTEM_CONFIG_NAME;
-      File f = new File(systemPropertiesPath);
-      String fileContent = new String(Files.readAllBytes(f.toPath()));
+      String fileContent = new String(Files.readAllBytes(getConfigFile(nodeWrapper).toPath()));
       return Arrays.stream(contents).allMatch(fileContent::contains);
     } catch (IOException ignore) {
       return false;
     }
   }
+
+  private static File getConfigFile(AbstractNodeWrapper nodeWrapper) {
+    String systemPropertiesPath =
+        nodeWrapper.getNodePath()
+            + File.separator
+            + "conf"
+            + File.separator
+            + CommonConfig.SYSTEM_CONFIG_NAME;
+    return new File(systemPropertiesPath);
+  }
 }

iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/manager/ConfigManager.java

@@ -1573,28 +1573,41 @@ public TSStatus clearCache() {
   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());
       Properties properties = new Properties();
       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

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java

@@ -2022,7 +2022,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);

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/execution/config/executor/ClusterConfigTaskExecutor.java

@@ -1061,11 +1061,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;

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

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