Skip to content

Commit b125033

Browse files
authored
[7.x] 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.
1 parent d13c4b3 commit b125033

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

server/src/main/java/org/elasticsearch/common/settings/Setting.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,14 @@ private T get(Settings settings, boolean validate) {
487487
*/
488488
public void diff(Settings.Builder builder, Settings source, Settings defaultSettings) {
489489
if (exists(source) == false) {
490-
builder.put(getKey(), getRaw(defaultSettings));
490+
if (exists(defaultSettings)) {
491+
// If the setting is only in the defaults, use the value from the defaults
492+
builder.put(getKey(), getRaw(defaultSettings));
493+
} else {
494+
// If the setting is in neither `source` nor `default`, get the value
495+
// from `source` as it may depend on the value of other settings
496+
builder.put(getKey(), getRaw(source));
497+
}
491498
}
492499
}
493500

server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,28 @@ public void testDiff() throws IOException {
737737
assertThat(diff.getAsInt("foo.bar", null), equalTo(1));
738738
}
739739

740+
public void testDiffWithDependentSettings() {
741+
final String dependedSettingName = "this.setting.is.depended.on";
742+
Setting<Integer> dependedSetting = Setting.intSetting(dependedSettingName, 1, Property.Dynamic, Property.NodeScope);
743+
744+
final String dependentSettingName = "this.setting.depends.on.another";
745+
Setting<Integer> dependentSetting = new Setting<>(dependentSettingName,
746+
(s) -> Integer.toString(dependedSetting.get(s) + 10),
747+
(s) -> Setting.parseInt(s, 1, dependentSettingName),
748+
Property.Dynamic, Property.NodeScope);
749+
750+
ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(dependedSetting, dependentSetting)));
751+
752+
// Ensure that the value of the dependent setting is correctly calculated based on the "source" settings
753+
Settings diff = settings.diff(Settings.builder().put(dependedSettingName, 2).build(), Settings.EMPTY);
754+
assertThat(diff.getAsInt(dependentSettingName, null), equalTo(12));
755+
756+
// Ensure that the value is correctly calculated if neither is set
757+
diff = settings.diff(Settings.EMPTY, Settings.EMPTY);
758+
assertThat(diff.getAsInt(dependedSettingName, null), equalTo(1));
759+
assertThat(diff.getAsInt(dependentSettingName, null), equalTo(11));
760+
}
761+
740762
public void testDiffWithAffixAndComplexMatcher() {
741763
Setting<Integer> fooBarBaz = Setting.intSetting("foo.bar.baz", 1, Property.NodeScope);
742764
Setting<Integer> fooBar = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope);

0 commit comments

Comments
 (0)