[Feature/extensions] Update settings registration changes to reflect …

…main (#5532) * Update settings changes to reflect main Signed-off-by: Ryan Bogan <[email protected]> * Update CHANGELOG Signed-off-by: Ryan Bogan <[email protected]> * Fix bwc for cluster manager throttling settings (#5305) Signed-off-by: Dhwanil Patel <[email protected]> Signed-off-by: Ryan Bogan <[email protected]> Signed-off-by: Dhwanil Patel <[email protected]> Co-authored-by: Dhwanil Patel <[email protected]>
opensearch-project · Dec 13, 2022 · bde8317 · bde8317
1 parent e6e5ff9
commit bde8317
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 12 deletions.
diff --git 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
@@ -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 <code>true</code> iff the given key is a valid settings key otherwise <code>false</code>
      */

diff --git 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<Setting<?>> settin
         addSettingsUpdater(new LoggingSettingUpdater(nodeSettings));
     }
 
-    public boolean registerSetting(Setting<?> setting) {
-        return super.registerSetting(setting);
-    }
-
     private static final class LoggingSettingUpdater implements SettingUpdater<Settings> {
         final Predicate<String> 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
@@ -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
@@ -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());