Ver código fonte

Correctly determine defaults of settings which depend on other settings (#65989)

This commit adjusts the behavior when calculating the diff between two
`AbstractScopedSettings` objects, so that the default values of settings
whose default values depend on the values of other settings are
correctly calculated. Previously, when calculating the diff, the default
value of a depended setting would be calculated based on the default
value of the setting(s) it depends on, rather than the current value of
those settings.
Gordon Brown 4 anos atrás
pai
commit
d566f8718b

+ 8 - 1
server/src/main/java/org/elasticsearch/common/settings/Setting.java

@@ -487,7 +487,14 @@ public class Setting<T> implements ToXContentObject {
      */
     public void diff(Settings.Builder builder, Settings source, Settings defaultSettings) {
         if (exists(source) == false) {
-            builder.put(getKey(), getRaw(defaultSettings));
+            if (exists(defaultSettings)) {
+                // If the setting is only in the defaults, use the value from the defaults
+                builder.put(getKey(), getRaw(defaultSettings));
+            } else {
+                // If the setting is in neither `source` nor `default`, get the value
+                // from `source` as it may depend on the value of other settings
+                builder.put(getKey(), getRaw(source));
+            }
         }
     }
 

+ 22 - 0
server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

@@ -737,6 +737,28 @@ public class ScopedSettingsTests extends ESTestCase {
         assertThat(diff.getAsInt("foo.bar", null), equalTo(1));
     }
 
+    public void testDiffWithDependentSettings() {
+        final String dependedSettingName = "this.setting.is.depended.on";
+        Setting<Integer> dependedSetting = Setting.intSetting(dependedSettingName, 1, Property.Dynamic, Property.NodeScope);
+
+        final String dependentSettingName = "this.setting.depends.on.another";
+        Setting<Integer> dependentSetting = new Setting<>(dependentSettingName,
+            (s) -> Integer.toString(dependedSetting.get(s) + 10),
+            (s) -> Setting.parseInt(s, 1, dependentSettingName),
+            Property.Dynamic, Property.NodeScope);
+
+        ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(dependedSetting, dependentSetting)));
+
+        // Ensure that the value of the dependent setting is correctly calculated based on the "source" settings
+        Settings diff = settings.diff(Settings.builder().put(dependedSettingName, 2).build(), Settings.EMPTY);
+        assertThat(diff.getAsInt(dependentSettingName, null), equalTo(12));
+
+        // Ensure that the value is correctly calculated if neither is set
+        diff = settings.diff(Settings.EMPTY, Settings.EMPTY);
+        assertThat(diff.getAsInt(dependedSettingName, null), equalTo(1));
+        assertThat(diff.getAsInt(dependentSettingName, null), equalTo(11));
+    }
+
     public void testDiffWithAffixAndComplexMatcher() {
         Setting<Integer> fooBarBaz = Setting.intSetting("foo.bar.baz", 1, Property.NodeScope);
         Setting<Integer> fooBar = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope);