Caideyipi commented on PR #17975:
URL: https://github.com/apache/iotdb/pull/17975#issuecomment-4737291429

   I found a few issues that look worth fixing before merge:
   
   1. `heartbeat_interval_in_ms` hot reload is incomplete. The reload path 
reschedules `HeartbeatService`, `StatisticsService`, `EventService`, and retry 
tasks, but existing `LoadCache` node/region caches and `TopologyService` keep 
`PhiAccrualDetector` instances that were constructed with the old heartbeat 
interval. `PhiAccrualDetector.minHeartbeatStdNs` is final, so after changing 
the heartbeat interval the node/region/topology failure detection thresholds 
still use the previous interval. Relevant locations: `ConfigManager.java`, 
`LoadManager.java`, `AbstractLoadCache.java`, and `TopologyService.java`.
   
   2. Rejected hot-reload values can still appear in `show configuration` as 
applied. Both ConfigNode and DataNode call 
`ConfigurationFileUtils.updateAppliedProperties(...)` before validating and 
applying the merged hot-reload properties. If a reload later fails, for example 
`set configuration heartbeat_interval_in_ms=-1`, the config file is not 
written, but `lastAppliedProperties` has already been updated, so `show 
configuration` can report a value that was not actually applied. Relevant 
locations: `ConfigNodeDescriptor.loadHotModifiedProps`, 
`IoTDBDescriptor.loadHotModifiedProps`, and 
`ConfigurationFileUtils.updateAppliedProperties`.
   
   3. This PR changes `schema_region_per_data_node` and 
`data_region_per_data_node` from accepting numeric strings through 
`Double.parseDouble(...).intValue()` to strict `Integer.parseInt(...)`. 
Existing configs like `data_region_per_data_node=5.0`, which were previously 
accepted and match the old IT API type, will now fail startup or any ConfigNode 
hot reload because the merged config file is revalidated. If this behavior 
change is intentional, it should probably be called out; otherwise consider 
preserving backward compatibility.


-- 
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]

Reply via email to