honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[7.10] 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.
This commit is contained in:
Gordon Brown 2020-12-08 13:21:00 -07:00 committed by GitHub
parent 43fa468076
commit fb65fd8723
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
server/src/main/java/org/elasticsearch/common/settings/Setting.java
View File

@ -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
server/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java
View File

@ -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);