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