Skip to content

Commit 00dfdf5

Browse files
authored
Represent lists as actual lists inside Settings (#26878)
Today we represent each value of a list setting with it's own dedicated key that ends with the index of the value in the list. Aside of the obvious weirdness this has several issues especially if lists are massive since it causes massive runtime penalties when validating settings. Like a list of 100k words will literally cause a create index call to timeout and in-turn massive slowdown on all subsequent validations runs. With this change we use a simple string list to represent the list. This change also forbids to add a settings that ends with a .0 which was internally used to detect a list setting. Once this has been rolled out for an entire major version all the internal .0 handling can be removed since all settings will be converted. Relates to #26723
1 parent dca787e commit 00dfdf5

File tree

14 files changed

+311
-202
lines changed

14 files changed

+311
-202
lines changed

core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ protected AbstractScopedSettings(Settings settings, Set<Setting<?>> settingsSet,
8686

8787
protected void validateSettingKey(Setting setting) {
8888
if (isValidKey(setting.getKey()) == false && (setting.isGroupSetting() && isValidGroupKey(setting.getKey())
89-
|| isValidAffixKey(setting.getKey())) == false) {
89+
|| isValidAffixKey(setting.getKey())) == false || setting.getKey().endsWith(".0")) {
9090
throw new IllegalArgumentException("illegal settings key: [" + setting.getKey() + "]");
9191
}
9292
}
@@ -534,7 +534,7 @@ private static boolean applyDeletes(Set<String> deletes, Settings.Builder builde
534534
boolean changed = false;
535535
for (String entry : deletes) {
536536
Set<String> keysToRemove = new HashSet<>();
537-
Set<String> keySet = builder.internalMap().keySet();
537+
Set<String> keySet = builder.keys();
538538
for (String key : keySet) {
539539
if (Regex.simpleMatch(entry, key) && canRemove.test(key)) {
540540
// we have to re-check with canRemove here since we might have a wildcard expression foo.* that matches

core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public Settings getValue(Settings current, Settings previous) {
127127
Settings.Builder builder = Settings.builder();
128128
builder.put(current.filter(loggerPredicate));
129129
for (String key : previous.keySet()) {
130-
if (loggerPredicate.test(key) && builder.internalMap().containsKey(key) == false) {
130+
if (loggerPredicate.test(key) && builder.keys().contains(key) == false) {
131131
if (ESLoggerFactory.LOG_LEVEL_SETTING.getConcreteSetting(key).exists(settings) == false) {
132132
builder.putNull(key);
133133
} else {

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -820,12 +820,6 @@ boolean hasComplexMatcher() {
820820
return true;
821821
}
822822

823-
@Override
824-
public boolean exists(Settings settings) {
825-
boolean exists = super.exists(settings);
826-
return exists || settings.get(getKey() + ".0") != null;
827-
}
828-
829823
@Override
830824
public void diff(Settings.Builder builder, Settings source, Settings defaultSettings) {
831825
if (exists(source) == false) {

0 commit comments

Comments
 (0)