Uncomment commented set-configuration-able items in the config template (fix enable_topology_probing & topology_probing_* hot/restart reload)#17933
Open
CRZbulabula wants to merge 2 commits into
Conversation
…ig template enable_topology_probing declares effectiveMode: hot_reload and the ConfigNode implements the hot-reload start/stop logic, but 'set configuration "enable_topology_probing"="true"' was rejected with 301 "immutable or undefined". Root cause: the item was left commented out (# enable_topology_probing=false) in iotdb-system.properties.template. ConfigurationFileUtils.getConfigurationItemsFromTemplate only parses uncommented key=value lines into the default-value map, so filterInvalidConfigItems treated the key as undefined and dropped it before it could reach the hot-reload path. Uncommenting it makes it a bare default like every other working hot_reload item (e.g. slow_query_threshold). Add regression tests: - ConfigurationFileUtilsTest.checkHotReloadItemsAreUncommentedInTemplate scans the template and fails if any hot_reload item is left commented out (while tolerating keys that also have an uncommented variant, e.g. the Windows/Unix path pairs). - IoTDBSetConfigurationIT.testSetHotReloadTopologyProbing verifies 'set configuration' accepts the item and persists it to the ConfigNode config file.
…mplate Following the enable_topology_probing fix, a generalized regression test (checkSettableItemsAreUncommentedInTemplate) revealed that several other items whose effectiveMode permits 'set configuration' (everything except FIRST_START) were also left commented out in the template, hitting the same root cause: getConfigurationItemsFromTemplate only parses uncommented lines, so filterInvalidConfigItems treats them as undefined and rejects the SQL with '301 immutable or undefined'. Uncomment these so 'set configuration' can reach them (all restart mode, all with real descriptor bindings and sensible defaults): - topology_probing_base_interval_in_ms=5000 - topology_probing_timeout_ratio=0.5 - cn_max_idle_client_count_for_each_node_in_client_manager=1000 - dn_max_idle_client_count_for_each_node_in_client_manager=1000 - region_group_allocate_policy=GCR Broaden the unit-test guard from hot_reload-only to every settable mode (hot_reload / restart / first_start_or_set_configuration), so any future settable item left commented is caught. Extend the integration test to also set the restart-mode topology_probing_* items end-to-end.
CRZbulabula
changed the title
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #17933 +/- ##
=========================================
Coverage 41.07% 41.07%
Complexity 318 318
=========================================
Files 5257 5257
Lines 365010 365010
Branches 47180 47180
=========================================
+ Hits 149918 149923 +5
+ Misses 215092 215087 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Several configuration items declared as dynamically settable were left commented out in
iotdb-system.properties.template, soset configurationrejected them with:The most visible case (PingCode V2-995) was
enable_topology_probing(added in #17595): it is declaredeffectiveMode: hot_reloadand the ConfigNode already implements the hot-reload start/stop logic (ConfigManager.handleTopologyProbingHotReload), yetset configuration "enable_topology_probing"="true"was rejected — so operators could only toggle the TopologyService by editing the file and restarting the ConfigNode (which triggers a leader election and a brief availability hit).Root cause
ConfigurationFileUtils.getConfigurationItemsFromTemplateonly parses uncommentedkey=valuelines intoconfiguration2DefaultValue. When an item's value line is commented out, the key never enters that map, sofilterInvalidConfigItemstreats it asundefinedand drops it before it can reach the apply path. The onlyeffectiveModethatset configurationis supposed to reject isFIRST_START; every other mode (hot_reload,restart,first_start_or_set_configuration) should be accepted (written to the file, applied now for hot_reload or on next restart otherwise).Fix
Uncomment the affected items so they become bare defaults like every other working settable item (e.g.
slow_query_threshold,wal_mode). No Java logic changes are required — the entire downstream chain was already correct; the commented template line was the only blocker.Items uncommented:
enable_topology_probing=falsetopology_probing_base_interval_in_ms=5000topology_probing_timeout_ratio=0.5cn_max_idle_client_count_for_each_node_in_client_manager=1000dn_max_idle_client_count_for_each_node_in_client_manager=1000region_group_allocate_policy=GCRAll have real descriptor bindings (
ConfigNodeDescriptor/CommonDescriptor/IoTDBDescriptor) and validated defaults, so uncommenting them is safe and behavior-preserving for fresh deployments.Tests
ConfigurationFileUtilsTest.checkSettableItemsAreUncommentedInTemplate— a regression guard that scans the raw template and fails if any item whoseeffectiveModepermitsset configuration(i.e. anything exceptFIRST_START) is left commented out. It tolerates keys that also have an uncommented variant elsewhere (e.g. the Windows/Unix path pairs such asdn_data_dirs), so it only flags items that are genuinely unreachable. This is what surfaced the additionalrestart-mode items beyondenable_topology_probing.IoTDBSetConfigurationIT.testSetTopologyProbingConfiguration— end-to-end: assertsset configurationacceptsenable_topology_probing(hot_reload) and the twotopology_probing_*items (restart) and persists them to the ConfigNode config file.This PR has:
Key changed/added classes (or packages if there are too many classes) in this PR
iotdb-system.properties.template(node-commons config template)ConfigurationFileUtilsTest(datanode unit test)IoTDBSetConfigurationIT(integration test)