From bde8317969b42b78962fe915712b721886f33bec Mon Sep 17 00:00:00 2001 From: Ryan Bogan <10944539+ryanbogan@users.noreply.github.com> Date: Mon, 12 Dec 2022 17:17:50 -0800 Subject: [PATCH] [Feature/extensions] Update settings registration changes to reflect main (#5532) * Update settings changes to reflect main Signed-off-by: Ryan Bogan * Update CHANGELOG Signed-off-by: Ryan Bogan * Fix bwc for cluster manager throttling settings (#5305) Signed-off-by: Dhwanil Patel Signed-off-by: Ryan Bogan Signed-off-by: Dhwanil Patel Co-authored-by: Dhwanil Patel --- CHANGELOG.md | 1 + .../settings/AbstractScopedSettings.java | 8 +++++++ .../common/settings/ClusterSettings.java | 4 ---- .../common/settings/IndexScopedSettings.java | 4 ---- .../common/settings/SettingsModule.java | 24 +++++++++++++++---- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0be1e2e873622..a67286c286f9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Modified local node request to return Discovery Node ([#4862](https://github.com/opensearch-project/OpenSearch/pull/4862)) - Enforce type safety for NamedWriteableRegistryParseRequest ([#4923](https://github.com/opensearch-project/OpenSearch/pull/4923)) - Renaming to match merge to main branch ([5362](https://github.com/opensearch-project/OpenSearch/pull/5362)) + - Updated settings registration changes to reflect main ([5532](https://github.com/opensearch-project/OpenSearch/pull/5532)) ## [Unreleased 2.x] ### Added diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java index d825861e0e75f..8a19d309975df 100644 --- a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java @@ -153,6 +153,14 @@ public boolean registerSetting(Setting setting) { } } + public boolean unregisterSetting(Setting setting) { + if (setting.hasComplexMatcher()) { + return setting != complexMatchers.remove(setting.getKey()); + } else { + return setting != keySettings.remove(setting.getKey()); + } + } + /** * Returns true iff the given key is a valid settings key otherwise false */ diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 6576e64d4e980..7e9c7bd3123c5 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -173,10 +173,6 @@ public ClusterSettings(final Settings nodeSettings, final Set> settin addSettingsUpdater(new LoggingSettingUpdater(nodeSettings)); } - public boolean registerSetting(Setting setting) { - return super.registerSetting(setting); - } - private static final class LoggingSettingUpdater implements SettingUpdater { final Predicate loggerPredicate = Loggers.LOG_LEVEL_SETTING::match; private final Settings settings; diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index 5a2734723ff83..079fc38415328 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -251,10 +251,6 @@ public IndexScopedSettings copy(Settings settings, IndexMetadata metadata) { return new IndexScopedSettings(settings, this, metadata); } - public boolean registerSetting(Setting setting) { - return super.registerSetting(setting); - } - @Override protected void validateSettingKey(Setting setting) { if (setting.getKey().startsWith("index.") == false) { diff --git a/server/src/main/java/org/opensearch/common/settings/SettingsModule.java b/server/src/main/java/org/opensearch/common/settings/SettingsModule.java index 10dd780af7503..df16c5a499ebe 100644 --- a/server/src/main/java/org/opensearch/common/settings/SettingsModule.java +++ b/server/src/main/java/org/opensearch/common/settings/SettingsModule.java @@ -186,15 +186,31 @@ public void configure(Binder binder) { * @return boolean value is set to true when successfully registered, else returns false */ public boolean registerDynamicSetting(Setting setting) { + boolean onNodeSetting = false; + boolean onIndexSetting = false; try { - registerSetting(setting); if (setting.hasNodeScope()) { - return clusterSettings.registerSetting(setting); + onNodeSetting = clusterSettings.registerSetting(setting); } if (setting.hasIndexScope()) { - return indexScopedSettings.registerSetting(setting); + onIndexSetting = indexScopedSettings.registerSetting(setting); + } + try { + registerSetting(setting); + if (onNodeSetting || onIndexSetting) { + logger.info("Registered new Setting: " + setting.getKey() + " successfully "); + return true; + } + } catch (IllegalArgumentException ex) { + if (onNodeSetting) { + clusterSettings.unregisterSetting(setting); + } + + if (onIndexSetting) { + indexScopedSettings.unregisterSetting(setting); + } + throw ex; } - logger.info("Registered new Setting: " + setting.getKey() + " successfully "); } catch (Exception e) { logger.error("Could not register setting " + setting.getKey()); throw new SettingsException("Could not register setting:" + setting.getKey());