Generalize search.remote settings to cluster.remote #33413
Conversation
Deprecating a some setting specializations (e.g., list settings) does not cause deprecation warning headers and deprecation log messages to appear. This is due to a missed check for deprecation. This commit fixes this for all setting specializations, and ensures that this can not be missed again.
With features like CCR building on the CCS infrastructure, the settings prefix search.remote makes less sense as the namespace for these remote cluster settings than does a more general namespace like cluster.remote. This commit replaces these settings with cluster.remote with a fallback to the deprecated settings search.remote.
jasontedor
added
review
:Distributed Indexing/CCR
|
Pinging @elastic/es-distributed |
|
This PR is based on #33412; it was discovered in this work that list settings are not properly deprecated. |
|
@martijnvg Yup! I will do that in a separate PR targeting 6.5.0 only (for deprecation) and 7.0.0 (for removal). |
|
I have some questions before I go into reviews:
|
* master: Fix deprecated setting specializations (elastic#33412) HLRC: split cluster request converters (elastic#33400) HLRC: Add ML get influencers API (elastic#33389) Add conditional token filter to elasticsearch (elastic#31958) Build: Merge xpack checkstyle config into core (elastic#33399) Disable IndexRecoveryIT.testRerouteRecovery. INGEST: Implement Drop Processor (elastic#32278) [ML] Add field stats to log structure finder (elastic#33351) Add interval response parameter to AutoDateInterval histogram (elastic#33254) MINOR+CORE: Remove Dead Methods ClusterService (elastic#33346)
Yes, my plan is to address this in a follow-up indeed.
This question is moot since I plan to automatically upgrade the settings.
I would rather not as there are only a handful of settings here, it's straightforward enough to manage them individually. Are you okay with this? |
++ LGTM |
With features like CCR building on the CCS infrastructure, the settings prefix search.remote makes less sense as the namespace for these remote cluster settings than does a more general namespace like cluster.remote. This commit replaces these settings with cluster.remote with a fallback to the deprecated settings search.remote.
|
@jasontedor when I add the deprecated settings to the config file against master, I get the deprecation warning but the setting seems to be ignored, it does not do anything. You can repro this by adding some unreachable cluster, if you use the new settings the node won't start, with the older setting the node starts. If the remote cluster is reachable, using the deprecated settings I can never connect to it through ccs, _remote/info always returns empty. |
Fallback settings were introduced in elastic#33413 where the `search.remote.*` settings were generalized to `cluster.remote.*`. This commit removes both the fallback settings and the upgraders that were introduced in elastic#33537
With features like CCR building on the CCS infrastructure, the settings prefix
search.remotemakes less sense as the namespace for these remote cluster settings than does a more general namespace likecluster.remote. This commit replaces these settings withcluster.remotewith a fallback to the deprecated settingssearch.remote.