Copilot commented on code in PR #17975:
URL: https://github.com/apache/iotdb/pull/17975#discussion_r3427628800
##########
integration-test/src/test/java/org/apache/iotdb/db/it/IoTDBSetConfigurationIT.java:
##########
@@ -177,6 +329,57 @@ private static boolean checkConfigFileContains(
}
}
+ private static void assertAppliedConfiguration(int nodeId, String key,
String value)
+ throws Exception {
+ try (ITableSession tableSessionConnection =
EnvFactory.getEnv().getTableSessionConnection()) {
+ SessionDataSet sessionDataSet =
+ tableSessionConnection.executeQueryStatement("show configuration on
" + nodeId);
+ SessionDataSet.DataIterator iterator = sessionDataSet.iterator();
+ while (iterator.next()) {
+ if (key.equals(iterator.getString(1))) {
+ Assert.assertEquals(value, iterator.isNull(2) ? null :
iterator.getString(2));
+ return;
+ }
+ }
+ }
Review Comment:
`SessionDataSet` implements `AutoCloseable` (via `ISessionDataSet`), but it
is not closed here, which can leak RPC/resources during IT runs. Wrap the
`SessionDataSet` in try-with-resources as well.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeDescriptor.java:
##########
@@ -782,20 +735,156 @@ private void loadCQConfig(TrimProperties properties) {
}
conf.setCqSubmitThread(cqSubmitThread);
+ conf.setCqMinEveryIntervalInMs(loadCqMinEveryIntervalInMs(properties,
false));
+ }
+
+ private long loadHeartbeatIntervalInMs(TrimProperties properties, boolean
rejectInvalid)
+ throws IOException {
+ long heartbeatIntervalInMs =
+ Long.parseLong(
+ properties.getProperty(
+ "heartbeat_interval_in_ms",
String.valueOf(conf.getHeartbeatIntervalInMs())));
+ if (heartbeatIntervalInMs <= 0) {
+ String warning =
+ "heartbeat_interval_in_ms should be greater than 0, but was "
+ + heartbeatIntervalInMs
+ + ", using previous value "
+ + conf.getHeartbeatIntervalInMs()
+ + ".";
+ if (rejectInvalid) {
+ throw new IOException(warning);
+ }
+ LOGGER.warn(
+ "heartbeat_interval_in_ms should be greater than 0, but was {},
using previous value {}.",
+ heartbeatIntervalInMs,
+ conf.getHeartbeatIntervalInMs());
+ heartbeatIntervalInMs = conf.getHeartbeatIntervalInMs();
+ }
+ return heartbeatIntervalInMs;
+ }
+
+ private long loadCqMinEveryIntervalInMs(TrimProperties properties, boolean
rejectInvalid)
+ throws IOException {
long cqMinEveryIntervalInMs =
Long.parseLong(
properties.getProperty(
"continuous_query_min_every_interval_in_ms",
String.valueOf(conf.getCqMinEveryIntervalInMs())));
if (cqMinEveryIntervalInMs <= 0) {
+ if (rejectInvalid) {
+ throw new IOException(
+ "continuous_query_min_every_interval_in_ms should be greater than
0, but current value is "
+ + cqMinEveryIntervalInMs
+ + ".");
+ }
LOGGER.warn(
ConfigNodeMessages.CONTINUOUS_QUERY_MIN_EVERY_INTERVAL_IN_MS_SHOULD_BE_GREATER,
cqMinEveryIntervalInMs,
conf.getCqMinEveryIntervalInMs());
cqMinEveryIntervalInMs = conf.getCqMinEveryIntervalInMs();
}
- conf.setCqMinEveryIntervalInMs(cqMinEveryIntervalInMs);
+ return cqMinEveryIntervalInMs;
+ }
+
+ private RegionGroupExtensionConfig
loadRegionGroupExtensionConfig(TrimProperties properties)
+ throws IOException {
+ RegionGroupExtensionPolicy schemaRegionGroupExtensionPolicy =
+ RegionGroupExtensionPolicy.parse(
+ properties.getProperty(
+ "schema_region_group_extension_policy",
+ conf.getSchemaRegionGroupExtensionPolicy().getPolicy()));
+ int defaultSchemaRegionGroupNumPerDatabase =
+ Integer.parseInt(
+ properties.getProperty(
+ "default_schema_region_group_num_per_database",
+
String.valueOf(conf.getDefaultSchemaRegionGroupNumPerDatabase())));
+ int schemaRegionPerDataNode =
+ (int)
+ Double.parseDouble(
+ properties.getProperty(
+ "schema_region_per_data_node",
+ String.valueOf(conf.getSchemaRegionPerDataNode())));
+ RegionGroupExtensionPolicy dataRegionGroupExtensionPolicy =
+ RegionGroupExtensionPolicy.parse(
+ properties.getProperty(
+ "data_region_group_extension_policy",
+ conf.getDataRegionGroupExtensionPolicy().getPolicy()));
+ int defaultDataRegionGroupNumPerDatabase =
+ Integer.parseInt(
+ properties.getProperty(
+ "default_data_region_group_num_per_database",
+
String.valueOf(conf.getDefaultDataRegionGroupNumPerDatabase())));
+ int dataRegionPerDataNode =
+ (int)
+ Double.parseDouble(
+ properties.getProperty(
+ "data_region_per_data_node",
String.valueOf(conf.getDataRegionPerDataNode())));
+
Review Comment:
Parsing `data_region_per_data_node` via `Double.parseDouble(...); (int)`
will silently truncate non-integer values (e.g., `"2.7"` -> `2`). Since this is
documented as an Integer, parse with `Integer.parseInt` to avoid unintended
hot-reload values.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/conf/ConfigNodeDescriptor.java:
##########
@@ -782,20 +735,156 @@ private void loadCQConfig(TrimProperties properties) {
}
conf.setCqSubmitThread(cqSubmitThread);
+ conf.setCqMinEveryIntervalInMs(loadCqMinEveryIntervalInMs(properties,
false));
+ }
+
+ private long loadHeartbeatIntervalInMs(TrimProperties properties, boolean
rejectInvalid)
+ throws IOException {
+ long heartbeatIntervalInMs =
+ Long.parseLong(
+ properties.getProperty(
+ "heartbeat_interval_in_ms",
String.valueOf(conf.getHeartbeatIntervalInMs())));
+ if (heartbeatIntervalInMs <= 0) {
+ String warning =
+ "heartbeat_interval_in_ms should be greater than 0, but was "
+ + heartbeatIntervalInMs
+ + ", using previous value "
+ + conf.getHeartbeatIntervalInMs()
+ + ".";
+ if (rejectInvalid) {
+ throw new IOException(warning);
+ }
+ LOGGER.warn(
+ "heartbeat_interval_in_ms should be greater than 0, but was {},
using previous value {}.",
+ heartbeatIntervalInMs,
+ conf.getHeartbeatIntervalInMs());
+ heartbeatIntervalInMs = conf.getHeartbeatIntervalInMs();
+ }
+ return heartbeatIntervalInMs;
+ }
+
+ private long loadCqMinEveryIntervalInMs(TrimProperties properties, boolean
rejectInvalid)
+ throws IOException {
long cqMinEveryIntervalInMs =
Long.parseLong(
properties.getProperty(
"continuous_query_min_every_interval_in_ms",
String.valueOf(conf.getCqMinEveryIntervalInMs())));
if (cqMinEveryIntervalInMs <= 0) {
+ if (rejectInvalid) {
+ throw new IOException(
+ "continuous_query_min_every_interval_in_ms should be greater than
0, but current value is "
+ + cqMinEveryIntervalInMs
+ + ".");
+ }
LOGGER.warn(
ConfigNodeMessages.CONTINUOUS_QUERY_MIN_EVERY_INTERVAL_IN_MS_SHOULD_BE_GREATER,
cqMinEveryIntervalInMs,
conf.getCqMinEveryIntervalInMs());
cqMinEveryIntervalInMs = conf.getCqMinEveryIntervalInMs();
}
- conf.setCqMinEveryIntervalInMs(cqMinEveryIntervalInMs);
+ return cqMinEveryIntervalInMs;
+ }
+
+ private RegionGroupExtensionConfig
loadRegionGroupExtensionConfig(TrimProperties properties)
+ throws IOException {
+ RegionGroupExtensionPolicy schemaRegionGroupExtensionPolicy =
+ RegionGroupExtensionPolicy.parse(
+ properties.getProperty(
+ "schema_region_group_extension_policy",
+ conf.getSchemaRegionGroupExtensionPolicy().getPolicy()));
+ int defaultSchemaRegionGroupNumPerDatabase =
+ Integer.parseInt(
+ properties.getProperty(
+ "default_schema_region_group_num_per_database",
+
String.valueOf(conf.getDefaultSchemaRegionGroupNumPerDatabase())));
+ int schemaRegionPerDataNode =
+ (int)
+ Double.parseDouble(
+ properties.getProperty(
+ "schema_region_per_data_node",
+ String.valueOf(conf.getSchemaRegionPerDataNode())));
+ RegionGroupExtensionPolicy dataRegionGroupExtensionPolicy =
Review Comment:
Parsing `schema_region_per_data_node` via `Double.parseDouble(...); (int)`
will silently accept non-integer values (e.g., `"1.9"` becomes `1`), which
contradicts the documented Integer datatype and can lead to surprising
hot-reload behavior. Parse as an integer and fail fast on invalid input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
