Caideyipi commented on code in PR #18467:
URL: https://github.com/apache/iotdb/pull/18467#discussion_r3781573771
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java:
##########
@@ -167,8 +167,8 @@ protected IoTDBDescriptor() {
.getConfig()
.setCustomizedProperties(loader.getCustomizedProperties());
}
- // if there are no properties, we need to init memory config
- if (!hasProperties) {
+ // If no configuration source initialized the memory config, initialize it
with defaults.
+ if (!hasLoadedProperties && !hasProperties) {
Review Comment:
Could we add a regression test that exercises this constructor branch with a
real system configuration source? The 11 tests listed in the PR are unchanged
from the parent commit: IoTDBDescriptorTest only checks URL resolution, while
DataNodeMemoryConfigTest tests the calculation/default paths directly. As a
result, removing `&& !hasLoadedProperties` would still leave all of them
passing. A test that initializes a fresh descriptor (ideally in an isolated
JVM/classloader) with `datanode_memory_proportion=1:1:1:1:1:5`, activates the
RPC buffer memory control, and verifies a `maxMemory / 4` budget instead of the
default `maxMemory / 20` would cover the reported regression. It would also be
useful to retain an assertion for the no-configuration fallback.
--
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]
