From 00dfdf50cfee9142daf987d38adf3eff3a971573 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Thu, 5 Oct 2017 09:27:08 +0200 Subject: [PATCH] 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 --- .../settings/AbstractScopedSettings.java | 4 +- .../common/settings/ClusterSettings.java | 2 +- .../common/settings/Setting.java | 6 - .../common/settings/Settings.java | 279 +++++++++--------- .../common/settings/SettingsFilter.java | 8 +- .../index/analysis/Analysis.java | 11 +- .../node/InternalSettingsPreparer.java | 10 +- .../common/settings/ScopedSettingsTests.java | 12 +- .../common/settings/SettingTests.java | 4 +- .../common/settings/SettingsTests.java | 117 ++++++-- .../analysis/common/MassiveWordListTests.java | 50 ++++ .../discovery/ec2/AwsEc2Service.java | 4 +- .../ec2/AwsEc2UnicastHostsProvider.java | 4 +- .../elasticsearch/test/ESIntegTestCase.java | 2 +- 14 files changed, 311 insertions(+), 202 deletions(-) create mode 100644 modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/MassiveWordListTests.java diff --git a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index f71b7d31551..61f32c67c20 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -86,7 +86,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { protected void validateSettingKey(Setting setting) { if (isValidKey(setting.getKey()) == false && (setting.isGroupSetting() && isValidGroupKey(setting.getKey()) - || isValidAffixKey(setting.getKey())) == false) { + || isValidAffixKey(setting.getKey())) == false || setting.getKey().endsWith(".0")) { throw new IllegalArgumentException("illegal settings key: [" + setting.getKey() + "]"); } } @@ -534,7 +534,7 @@ public abstract class AbstractScopedSettings extends AbstractComponent { boolean changed = false; for (String entry : deletes) { Set keysToRemove = new HashSet<>(); - Set keySet = builder.internalMap().keySet(); + Set keySet = builder.keys(); for (String key : keySet) { if (Regex.simpleMatch(entry, key) && canRemove.test(key)) { // we have to re-check with canRemove here since we might have a wildcard expression foo.* that matches diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 4472860b277..1ade10e4c7d 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -127,7 +127,7 @@ public final class ClusterSettings extends AbstractScopedSettings { Settings.Builder builder = Settings.builder(); builder.put(current.filter(loggerPredicate)); for (String key : previous.keySet()) { - if (loggerPredicate.test(key) && builder.internalMap().containsKey(key) == false) { + if (loggerPredicate.test(key) && builder.keys().contains(key) == false) { if (ESLoggerFactory.LOG_LEVEL_SETTING.getConcreteSetting(key).exists(settings) == false) { builder.putNull(key); } else { diff --git a/core/src/main/java/org/elasticsearch/common/settings/Setting.java b/core/src/main/java/org/elasticsearch/common/settings/Setting.java index 56a52456dfb..ee6e422e826 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -820,12 +820,6 @@ public class Setting implements ToXContentObject { return true; } - @Override - public boolean exists(Settings settings) { - boolean exists = super.exists(settings); - return exists || settings.get(getKey() + ".0") != null; - } - @Override public void diff(Settings.Builder builder, Settings source, Settings defaultSettings) { if (exists(source) == false) { diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index bf196a0643a..bfdb93b8fb6 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -39,7 +39,6 @@ import org.elasticsearch.common.unit.SizeValue; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContentFragment; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; @@ -57,23 +56,18 @@ import java.util.AbstractSet; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Dictionary; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.NoSuchElementException; -import java.util.Objects; import java.util.Set; import java.util.TreeMap; import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.UnaryOperator; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -87,10 +81,9 @@ import static org.elasticsearch.common.unit.TimeValue.parseTimeValue; public final class Settings implements ToXContentFragment { public static final Settings EMPTY = new Builder().build(); - private static final Pattern ARRAY_PATTERN = Pattern.compile("(.*)\\.\\d+$"); /** The raw settings from the full key to raw string value. */ - private final Map settings; + private final Map settings; /** The secure settings storage associated with these settings. */ private final SecureSettings secureSettings; @@ -104,7 +97,7 @@ public final class Settings implements ToXContentFragment { */ private final SetOnce> keys = new SetOnce<>(); - Settings(Map settings, SecureSettings secureSettings) { + Settings(Map settings, SecureSettings secureSettings) { // we use a sorted map for consistent serialization when using getAsMap() this.settings = Collections.unmodifiableSortedMap(new TreeMap<>(settings)); this.secureSettings = secureSettings; @@ -120,7 +113,7 @@ public final class Settings implements ToXContentFragment { private Map getAsStructuredMap() { Map map = new HashMap<>(2); - for (Map.Entry entry : settings.entrySet()) { + for (Map.Entry entry : settings.entrySet()) { processSetting(map, "", entry.getKey(), entry.getValue()); } for (Map.Entry entry : map.entrySet()) { @@ -133,7 +126,7 @@ public final class Settings implements ToXContentFragment { return map; } - private void processSetting(Map map, String prefix, String setting, String value) { + private void processSetting(Map map, String prefix, String setting, Object value) { int prefixLength = setting.indexOf('.'); if (prefixLength == -1) { @SuppressWarnings("unchecked") Map innerMap = (Map) map.get(prefix + setting); @@ -237,7 +230,7 @@ public final class Settings implements ToXContentFragment { * @return The setting value, null if it does not exists. */ public String get(String setting) { - return settings.get(setting); + return toString(settings.get(setting)); } /** @@ -373,83 +366,60 @@ public final class Settings implements ToXContentFragment { } /** - * The values associated with a setting prefix as an array. The settings array is in the format of: - * settingPrefix.[index]. + * The values associated with a setting key as an array. *

* It will also automatically load a comma separated list under the settingPrefix and merge with * the numbered format. * - * @param settingPrefix The setting prefix to load the array by + * @param key The setting prefix to load the array by * @return The setting array values */ - public String[] getAsArray(String settingPrefix) throws SettingsException { - return getAsArray(settingPrefix, Strings.EMPTY_ARRAY, true); + public String[] getAsArray(String key) throws SettingsException { + return getAsArray(key, Strings.EMPTY_ARRAY, true); } /** - * The values associated with a setting prefix as an array. The settings array is in the format of: - * settingPrefix.[index]. + * The values associated with a setting key as an array. *

* If commaDelimited is true, it will automatically load a comma separated list under the settingPrefix and merge with * the numbered format. * - * @param settingPrefix The setting prefix to load the array by + * @param key The setting key to load the array by * @return The setting array values */ - public String[] getAsArray(String settingPrefix, String[] defaultArray) throws SettingsException { - return getAsArray(settingPrefix, defaultArray, true); + public String[] getAsArray(String key, String[] defaultArray) throws SettingsException { + return getAsArray(key, defaultArray, true); } /** - * The values associated with a setting prefix as an array. The settings array is in the format of: - * settingPrefix.[index]. + * The values associated with a setting key as an array. *

* It will also automatically load a comma separated list under the settingPrefix and merge with * the numbered format. * - * @param settingPrefix The setting prefix to load the array by + * @param key The setting key to load the array by * @param defaultArray The default array to use if no value is specified * @param commaDelimited Whether to try to parse a string as a comma-delimited value * @return The setting array values */ - public String[] getAsArray(String settingPrefix, String[] defaultArray, Boolean commaDelimited) throws SettingsException { + public String[] getAsArray(String key, String[] defaultArray, Boolean commaDelimited) throws SettingsException { List result = new ArrayList<>(); - - final String valueFromPrefix = get(settingPrefix); - final String valueFromPreifx0 = get(settingPrefix + ".0"); - - if (valueFromPrefix != null && valueFromPreifx0 != null) { - final String message = String.format( - Locale.ROOT, - "settings object contains values for [%s=%s] and [%s=%s]", - settingPrefix, - valueFromPrefix, - settingPrefix + ".0", - valueFromPreifx0); - throw new IllegalStateException(message); - } - - if (get(settingPrefix) != null) { - if (commaDelimited) { - String[] strings = Strings.splitStringByCommaToArray(get(settingPrefix)); + final Object valueFromPrefix = settings.get(key); + if (valueFromPrefix != null) { + if (valueFromPrefix instanceof List) { + result = ((List) valueFromPrefix); + } else if (commaDelimited) { + String[] strings = Strings.splitStringByCommaToArray(get(key)); if (strings.length > 0) { for (String string : strings) { result.add(string.trim()); } } } else { - result.add(get(settingPrefix).trim()); + result.add(get(key).trim()); } } - int counter = 0; - while (true) { - String value = get(settingPrefix + '.' + (counter++)); - if (value == null) { - break; - } - result.add(value.trim()); - } if (result.isEmpty()) { return defaultArray; } @@ -550,7 +520,7 @@ public final class Settings implements ToXContentFragment { */ public String toDelimitedString(char delimiter) { StringBuilder sb = new StringBuilder(); - for (Map.Entry entry : settings.entrySet()) { + for (Map.Entry entry : settings.entrySet()) { sb.append(entry.getKey()).append("=").append(entry.getValue()).append(delimiter); } return sb.toString(); @@ -575,19 +545,52 @@ public final class Settings implements ToXContentFragment { public static Settings readSettingsFromStream(StreamInput in) throws IOException { Builder builder = new Builder(); int numberOfSettings = in.readVInt(); - for (int i = 0; i < numberOfSettings; i++) { - builder.put(in.readString(), in.readOptionalString()); + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + for (int i = 0; i < numberOfSettings; i++) { + String key = in.readString(); + Object value = in.readGenericValue(); + if (value == null) { + builder.putNull(key); + } else if (value instanceof List) { + builder.putArray(key, (List) value); + } else { + builder.put(key, value.toString()); + } + } + } else { + for (int i = 0; i < numberOfSettings; i++) { + String key = in.readString(); + String value = in.readOptionalString(); + builder.put(key, value); + } } return builder.build(); } public static void writeSettingsToStream(Settings settings, StreamOutput out) throws IOException { // pull settings to exclude secure settings in size() - Set> entries = settings.settings.entrySet(); - out.writeVInt(entries.size()); - for (Map.Entry entry : entries) { - out.writeString(entry.getKey()); - out.writeOptionalString(entry.getValue()); + Set> entries = settings.settings.entrySet(); + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { + out.writeVInt(entries.size()); + for (Map.Entry entry : entries) { + out.writeString(entry.getKey()); + out.writeGenericValue(entry.getValue()); + } + } else { + int size = entries.stream().mapToInt(e -> e.getValue() instanceof List ? ((List)e.getValue()).size() : 1).sum(); + out.writeVInt(size); + for (Map.Entry entry : entries) { + if (entry.getValue() instanceof List) { + int idx = 0; + for (String value : (List)entry.getValue()) { + out.writeString(entry.getKey() + "." + idx++); + out.writeOptionalString(value); + } + } else { + out.writeString(entry.getKey()); + out.writeOptionalString(toString(entry.getValue())); + } + } } } @@ -606,7 +609,7 @@ public final class Settings implements ToXContentFragment { builder.field(entry.getKey(), entry.getValue()); } } else { - for (Map.Entry entry : settings.settings.entrySet()) { + for (Map.Entry entry : settings.settings.entrySet()) { builder.field(entry.getKey(), entry.getValue()); } } @@ -622,9 +625,7 @@ public final class Settings implements ToXContentFragment { return fromXContent(parser, true, false); } - private static Settings fromXContent(XContentParser parser, boolean allowNullValues, - boolean validateEndOfStream) - throws IOException { + private static Settings fromXContent(XContentParser parser, boolean allowNullValues, boolean validateEndOfStream) throws IOException { if (parser.currentToken() == null) { parser.nextToken(); } @@ -766,7 +767,7 @@ public final class Settings implements ToXContentFragment { public static final Settings EMPTY_SETTINGS = new Builder().build(); // we use a sorted map for consistent serialization when using getAsMap() - private final Map map = new TreeMap<>(); + private final Map map = new TreeMap<>(); private SetOnce secureSettings = new SetOnce<>(); @@ -774,22 +775,22 @@ public final class Settings implements ToXContentFragment { } - public Map internalMap() { - return this.map; + public Set keys() { + return this.map.keySet(); } /** * Removes the provided setting from the internal map holding the current list of settings. */ public String remove(String key) { - return map.remove(key); + return Settings.toString(map.remove(key)); } /** * Returns a setting value based on the setting key. */ public String get(String key) { - return map.get(key); + return Settings.toString(map.get(key)); } /** Return the current secure settings, or {@code null} if none have been set. */ @@ -892,10 +893,17 @@ public final class Settings implements ToXContentFragment { } public Builder copy(String key, String sourceKey, Settings source) { - if (source.keySet().contains(sourceKey) == false) { + if (source.settings.containsKey(sourceKey) == false) { throw new IllegalArgumentException("source key not found in the source settings"); } - return put(key, source.get(sourceKey)); + final Object value = source.settings.get(sourceKey); + if (value instanceof List) { + return putArray(key, (List)value); + } else if (value == null) { + return putNull(key); + } else { + return put(key, Settings.toString(value)); + } } /** @@ -1027,16 +1035,7 @@ public final class Settings implements ToXContentFragment { */ public Builder putArray(String setting, List values) { remove(setting); - int counter = 0; - while (true) { - String value = map.remove(setting + '.' + (counter++)); - if (value == null) { - break; - } - } - for (int i = 0; i < values.size(); i++) { - put(setting + "." + i, values.get(i)); - } + map.put(setting, Collections.unmodifiableList(new ArrayList<>(values))); return this; } @@ -1069,55 +1068,41 @@ public final class Settings implements ToXContentFragment { * @param copySecureSettings if true all settings including secure settings are copied. */ public Builder put(Settings settings, boolean copySecureSettings) { - removeNonArraysFieldsIfNewSettingsContainsFieldAsArray(settings.settings); - map.putAll(settings.settings); + Map settingsMap = new HashMap<>(settings.settings); + processLegacyLists(settingsMap); + map.putAll(settingsMap); if (copySecureSettings && settings.getSecureSettings() != null) { setSecureSettings(settings.getSecureSettings()); } return this; } - /** - * Removes non array values from the existing map, if settings contains an array value instead - * - * Example: - * Existing map contains: {key:value} - * New map contains: {key:[value1,value2]} (which has been flattened to {}key.0:value1,key.1:value2}) - * - * This ensure that that the 'key' field gets removed from the map in order to override all the - * data instead of merging - */ - private void removeNonArraysFieldsIfNewSettingsContainsFieldAsArray(Map settings) { - List prefixesToRemove = new ArrayList<>(); - for (final Map.Entry entry : settings.entrySet()) { - final Matcher matcher = ARRAY_PATTERN.matcher(entry.getKey()); - if (matcher.matches()) { - prefixesToRemove.add(matcher.group(1)); - } else if (map.keySet().stream().anyMatch(key -> key.startsWith(entry.getKey() + "."))) { - prefixesToRemove.add(entry.getKey()); - } - } - for (String prefix : prefixesToRemove) { - Iterator> iterator = map.entrySet().iterator(); - while (iterator.hasNext()) { - Map.Entry entry = iterator.next(); - if (entry.getKey().startsWith(prefix + ".") || entry.getKey().equals(prefix)) { - iterator.remove(); + private void processLegacyLists(Map map) { + String[] array = map.keySet().toArray(new String[map.size()]); + for (String key : array) { + if (key.endsWith(".0")) { // let's only look at the head of the list and convert in order starting there. + int counter = 0; + String prefix = key.substring(0, key.lastIndexOf('.')); + if (map.containsKey(prefix)) { + throw new IllegalStateException("settings builder can't contain values for [" + prefix + "=" + map.get(prefix) + + "] and [" + key + "=" + map.get(key) + "]"); + } + List values = new ArrayList<>(); + while (true) { + String listKey = prefix + '.' + (counter++); + String value = get(listKey); + if (value == null) { + map.put(prefix, values); + break; + } else { + values.add(value); + map.remove(listKey); + } } } } } - /** - * Sets all the provided settings. - */ - public Builder put(Dictionary properties) { - for (Object key : Collections.list(properties.keys())) { - map.put(Objects.toString(key), Objects.toString(properties.get(key))); - } - return this; - } - /** * Loads settings from the actual string content that represents them using {@link #fromXContent(XContentParser)} */ @@ -1195,7 +1180,7 @@ public final class Settings implements ToXContentFragment { if (value != null) { return value; } - return map.get(placeholderName); + return Settings.toString(map.get(placeholderName)); } @Override @@ -1215,14 +1200,14 @@ public final class Settings implements ToXContentFragment { } }; - Iterator> entryItr = map.entrySet().iterator(); + Iterator> entryItr = map.entrySet().iterator(); while (entryItr.hasNext()) { - Map.Entry entry = entryItr.next(); - if (entry.getValue() == null) { + Map.Entry entry = entryItr.next(); + if (entry.getValue() == null || entry.getValue() instanceof List) { // a null value obviously can't be replaced continue; } - String value = propertyPlaceholder.replacePlaceholders(entry.getValue(), placeholderResolver); + String value = propertyPlaceholder.replacePlaceholders(Settings.toString(entry.getValue()), placeholderResolver); // if the values exists and has length, we should maintain it in the map // otherwise, the replace process resolved into removing it if (Strings.hasLength(value)) { @@ -1240,10 +1225,10 @@ public final class Settings implements ToXContentFragment { * If a setting doesn't start with the prefix, the builder appends the prefix to such setting. */ public Builder normalizePrefix(String prefix) { - Map replacements = new HashMap<>(); - Iterator> iterator = map.entrySet().iterator(); + Map replacements = new HashMap<>(); + Iterator> iterator = map.entrySet().iterator(); while(iterator.hasNext()) { - Map.Entry entry = iterator.next(); + Map.Entry entry = iterator.next(); if (entry.getKey().startsWith(prefix) == false) { replacements.put(prefix + entry.getKey(), entry.getValue()); iterator.remove(); @@ -1258,30 +1243,31 @@ public final class Settings implements ToXContentFragment { * set on this builder. */ public Settings build() { + processLegacyLists(map); return new Settings(map, secureSettings.get()); } } // TODO We could use an FST internally to make things even faster and more compact - private static final class FilteredMap extends AbstractMap { - private final Map delegate; + private static final class FilteredMap extends AbstractMap { + private final Map delegate; private final Predicate filter; private final String prefix; // we cache that size since we have to iterate the entire set // this is safe to do since this map is only used with unmodifiable maps private int size = -1; @Override - public Set> entrySet() { - Set> delegateSet = delegate.entrySet(); - AbstractSet> filterSet = new AbstractSet>() { + public Set> entrySet() { + Set> delegateSet = delegate.entrySet(); + AbstractSet> filterSet = new AbstractSet>() { @Override - public Iterator> iterator() { - Iterator> iter = delegateSet.iterator(); + public Iterator> iterator() { + Iterator> iter = delegateSet.iterator(); - return new Iterator>() { + return new Iterator>() { private int numIterated; - private Entry currentElement; + private Entry currentElement; @Override public boolean hasNext() { if (currentElement != null) { @@ -1304,29 +1290,29 @@ public final class Settings implements ToXContentFragment { } @Override - public Entry next() { + public Entry next() { if (currentElement == null && hasNext() == false) { // protect against no #hasNext call or not respecting it throw new NoSuchElementException("make sure to call hasNext first"); } - final Entry current = this.currentElement; + final Entry current = this.currentElement; this.currentElement = null; if (prefix == null) { return current; } - return new Entry() { + return new Entry() { @Override public String getKey() { return current.getKey().substring(prefix.length()); } @Override - public String getValue() { + public Object getValue() { return current.getValue(); } @Override - public String setValue(String value) { + public Object setValue(Object value) { throw new UnsupportedOperationException(); } }; @@ -1342,14 +1328,14 @@ public final class Settings implements ToXContentFragment { return filterSet; } - private FilteredMap(Map delegate, Predicate filter, String prefix) { + private FilteredMap(Map delegate, Predicate filter, String prefix) { this.delegate = delegate; this.filter = filter; this.prefix = prefix; } @Override - public String get(Object key) { + public Object get(Object key) { if (key instanceof String) { final String theKey = prefix == null ? (String)key : prefix + key; if (filter.test(theKey)) { @@ -1437,4 +1423,9 @@ public final class Settings implements ToXContentFragment { throw new UncheckedIOException(e); } } + + private static String toString(Object o) { + return o == null ? null : o.toString(); + } + } diff --git a/core/src/main/java/org/elasticsearch/common/settings/SettingsFilter.java b/core/src/main/java/org/elasticsearch/common/settings/SettingsFilter.java index 32c5e7a0da3..1c67318e282 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/SettingsFilter.java +++ b/core/src/main/java/org/elasticsearch/common/settings/SettingsFilter.java @@ -30,8 +30,6 @@ import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Map; -import java.util.Map.Entry; import java.util.Set; /** @@ -107,10 +105,10 @@ public final class SettingsFilter extends AbstractComponent { } if (!simpleMatchPatternList.isEmpty()) { String[] simpleMatchPatterns = simpleMatchPatternList.toArray(new String[simpleMatchPatternList.size()]); - Iterator> iterator = builder.internalMap().entrySet().iterator(); + Iterator iterator = builder.keys().iterator(); while (iterator.hasNext()) { - Map.Entry current = iterator.next(); - if (Regex.simpleMatch(simpleMatchPatterns, current.getKey())) { + String key = iterator.next(); + if (Regex.simpleMatch(simpleMatchPatterns, key)) { iterator.remove(); } } diff --git a/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java b/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java index f53b245e14c..dabf984a1bc 100644 --- a/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java +++ b/core/src/main/java/org/elasticsearch/index/analysis/Analysis.java @@ -101,13 +101,8 @@ public class Analysis { public static CharArraySet parseStemExclusion(Settings settings, CharArraySet defaultStemExclusion) { String value = settings.get("stem_exclusion"); - if (value != null) { - if ("_none_".equals(value)) { - return CharArraySet.EMPTY_SET; - } else { - // LUCENE 4 UPGRADE: Should be settings.getAsBoolean("stem_exclusion_case", false)? - return new CharArraySet(Strings.commaDelimitedListToSet(value), false); - } + if ("_none_".equals(value)) { + return CharArraySet.EMPTY_SET; } String[] stemExclusion = settings.getAsArray("stem_exclusion", null); if (stemExclusion != null) { @@ -164,7 +159,7 @@ public class Analysis { if ("_none_".equals(value)) { return CharArraySet.EMPTY_SET; } else { - return resolveNamedWords(Strings.commaDelimitedListToSet(value), namedWords, ignoreCase); + return resolveNamedWords(Arrays.asList(settings.getAsArray(name)), namedWords, ignoreCase); } } List pathLoadedWords = getWordList(env, settings, name); diff --git a/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java index 93c5a18222c..a2c7663ec9e 100644 --- a/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java +++ b/core/src/main/java/org/elasticsearch/node/InternalSettingsPreparer.java @@ -134,7 +134,7 @@ public class InternalSettingsPreparer { private static void finalizeSettings(Settings.Builder output, Terminal terminal) { // allow to force set properties based on configuration of the settings provided List forcedSettings = new ArrayList<>(); - for (String setting : output.internalMap().keySet()) { + for (String setting : output.keys()) { if (setting.startsWith("force.")) { forcedSettings.add(setting); } @@ -156,13 +156,13 @@ public class InternalSettingsPreparer { private static void replacePromptPlaceholders(Settings.Builder settings, Terminal terminal) { List secretToPrompt = new ArrayList<>(); List textToPrompt = new ArrayList<>(); - for (Map.Entry entry : settings.internalMap().entrySet()) { - switch (entry.getValue()) { + for (String key : settings.keys()) { + switch (settings.get(key)) { case SECRET_PROMPT_VALUE: - secretToPrompt.add(entry.getKey()); + secretToPrompt.add(key); break; case TEXT_PROMPT_VALUE: - textToPrompt.add(entry.getKey()); + textToPrompt.add(key); break; } } diff --git a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java index 1c64bcb2724..24f9550a78d 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java @@ -468,21 +468,21 @@ public class ScopedSettingsTests extends ESTestCase { ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(fooBar, fooBarBaz, foorBarQuux, someGroup, someAffix))); Settings diff = settings.diff(Settings.builder().put("foo.bar", 5).build(), Settings.EMPTY); - assertEquals(4, diff.size()); // 4 since foo.bar.quux has 3 values essentially + assertEquals(2, diff.size()); assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(1)); assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"a", "b", "c"}); diff = settings.diff( Settings.builder().put("foo.bar", 5).build(), Settings.builder().put("foo.bar.baz", 17).putArray("foo.bar.quux", "d", "e", "f").build()); - assertEquals(4, diff.size()); // 4 since foo.bar.quux has 3 values essentially + assertEquals(2, diff.size()); assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(17)); assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"d", "e", "f"}); diff = settings.diff( Settings.builder().put("some.group.foo", 5).build(), Settings.builder().put("some.group.foobar", 17).put("some.group.foo", 25).build()); - assertEquals(6, diff.size()); // 6 since foo.bar.quux has 3 values essentially + assertEquals(4, diff.size()); assertThat(diff.getAsInt("some.group.foobar", null), equalTo(17)); assertNull(diff.get("some.group.foo")); assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"a", "b", "c"}); @@ -492,7 +492,7 @@ public class ScopedSettingsTests extends ESTestCase { diff = settings.diff( Settings.builder().put("some.prefix.foo.somekey", 5).build(), Settings.builder().put("some.prefix.foobar.somekey", 17).put("some.prefix.foo.somekey", 18).build()); - assertEquals(6, diff.size()); // 6 since foo.bar.quux has 3 values essentially + assertEquals(4, diff.size()); assertThat(diff.getAsInt("some.prefix.foobar.somekey", null), equalTo(17)); assertNull(diff.get("some.prefix.foo.somekey")); assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"a", "b", "c"}); @@ -518,7 +518,7 @@ public class ScopedSettingsTests extends ESTestCase { diff = settings.diff( Settings.builder().put("foo.bar", 5).build(), Settings.builder().put("foo.bar.baz", 17).putArray("foo.bar.quux", "d", "e", "f").build()); - assertEquals(4, diff.size()); + assertEquals(2, diff.size()); assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(17)); assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"d", "e", "f"}); @@ -548,7 +548,7 @@ public class ScopedSettingsTests extends ESTestCase { .putArray("foo.bar.quux", "x", "y", "z") .putArray("foo.baz.quux", "d", "e", "f") .build()); - assertEquals(9, diff.size()); + assertEquals(5, diff.size()); assertThat(diff.getAsInt("some.prefix.foobar.somekey", null), equalTo(17)); assertNull(diff.get("some.prefix.foo.somekey")); assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"x", "y", "z"}); diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java index 57dbd78c840..4dfedf519bd 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -514,11 +514,11 @@ public class SettingTests extends ESTestCase { List input = Arrays.asList("test", "test1, test2", "test", ",,,,"); Settings.Builder builder = Settings.builder().putArray("foo.bar", input.toArray(new String[0])); // try to parse this really annoying format - for (String key : builder.internalMap().keySet()) { + for (String key : builder.keys()) { assertTrue("key: " + key + " doesn't match", listSetting.match(key)); } builder = Settings.builder().put("foo.bar", "1,2,3"); - for (String key : builder.internalMap().keySet()) { + for (String key : builder.keys()) { assertTrue("key: " + key + " doesn't match", listSetting.match(key)); } assertFalse(listSetting.match("foo_bar")); diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index 67e5e6f1e00..2ab7d086441 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -20,6 +20,8 @@ package org.elasticsearch.common.settings; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.xcontent.ToXContent; @@ -28,6 +30,7 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import org.hamcrest.CoreMatchers; import java.io.ByteArrayInputStream; @@ -36,6 +39,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.NoSuchElementException; @@ -255,7 +259,7 @@ public class SettingsTests extends ESTestCase { .put(Settings.builder().put("value.data", "1").build()) .build(); assertThat(settings.get("value.data"), is("1")); - assertThat(settings.get("value"), is(nullValue())); + assertThat(settings.get("value"), is("[4, 5]")); } public void testPrefixNormalization() { @@ -470,13 +474,18 @@ public class SettingsTests extends ESTestCase { secureSettings.setString("test.key2.bog", "somethingsecure"); Settings.Builder builder = Settings.builder(); builder.put("test.key1.baz", "blah1"); + builder.putNull("test.key3.bar"); + builder.putArray("test.key4.foo", "1", "2"); builder.setSecureSettings(secureSettings); - assertEquals(5, builder.build().size()); + assertEquals(7, builder.build().size()); Settings.writeSettingsToStream(builder.build(), out); StreamInput in = StreamInput.wrap(out.bytes().toBytesRef().bytes); Settings settings = Settings.readSettingsFromStream(in); - assertEquals(1, settings.size()); + assertEquals(3, settings.size()); assertEquals("blah1", settings.get("test.key1.baz")); + assertNull(settings.get("test.key3.bar")); + assertTrue(settings.keySet().contains("test.key3.bar")); + assertArrayEquals(new String[] {"1", "2"}, settings.getAsArray("test.key4.foo")); } public void testSecureSettingConflict() { @@ -487,14 +496,12 @@ public class SettingsTests extends ESTestCase { } public void testGetAsArrayFailsOnDuplicates() { - final Settings settings = - Settings.builder() - .put("foobar.0", "bar") - .put("foobar.1", "baz") - .put("foobar", "foo") - .build(); - final IllegalStateException e = expectThrows(IllegalStateException.class, () -> settings.getAsArray("foobar")); - assertThat(e, hasToString(containsString("settings object contains values for [foobar=foo] and [foobar.0=bar]"))); + final IllegalStateException e = expectThrows(IllegalStateException.class, () -> Settings.builder() + .put("foobar.0", "bar") + .put("foobar.1", "baz") + .put("foobar", "foo") + .build()); + assertThat(e, hasToString(containsString("settings builder can't contain values for [foobar=foo] and [foobar.0=bar]"))); } public void testToAndFromXContent() throws IOException { @@ -512,7 +519,7 @@ public class SettingsTests extends ESTestCase { builder.endObject(); XContentParser parser = createParser(builder); Settings build = Settings.fromXContent(parser); - assertEquals(7, build.size()); // each list element is it's own key hence 7 and not 5 + assertEquals(5, build.size()); assertArrayEquals(new String[] {"1", "2", "3"}, build.getAsArray("foo.bar.baz")); assertEquals(2, build.getAsInt("foo.foobar", 0).intValue()); assertEquals("test", build.get("rootfoo")); @@ -531,8 +538,8 @@ public class SettingsTests extends ESTestCase { assertThat(settings.getAsInt("test1.test2.value3", -1), equalTo(2)); // check array - assertThat(settings.get("test1.test3.0"), equalTo("test3-1")); - assertThat(settings.get("test1.test3.1"), equalTo("test3-2")); + assertNull(settings.get("test1.test3.0")); + assertNull(settings.get("test1.test3.1")); assertThat(settings.getAsArray("test1.test3").length, equalTo(2)); assertThat(settings.getAsArray("test1.test3")[0], equalTo("test3-1")); assertThat(settings.getAsArray("test1.test3")[1], equalTo("test3-2")); @@ -571,7 +578,7 @@ public class SettingsTests extends ESTestCase { builder.startObject(); test.toXContent(builder, new ToXContent.MapParams(Collections.emptyMap())); builder.endObject(); - assertEquals("{\"foo\":{\"bar\":{\"0\":\"1\",\"1\":\"2\",\"2\":\"3\",\"baz\":\"test\"}}}", builder.string()); + assertEquals("{\"foo\":{\"bar.baz\":\"test\",\"bar\":[\"1\",\"2\",\"3\"]}}", builder.string()); test = Settings.builder().putArray("foo.bar", "1", "2", "3").build(); builder = XContentBuilder.builder(XContentType.JSON.xContent()); @@ -584,7 +591,7 @@ public class SettingsTests extends ESTestCase { builder.startObject(); test.toXContent(builder, new ToXContent.MapParams(Collections.singletonMap("flat_settings", "true"))); builder.endObject(); - assertEquals("{\"foo.bar.0\":\"1\",\"foo.bar.1\":\"2\",\"foo.bar.2\":\"3\"}", builder.string()); + assertEquals("{\"foo.bar\":[\"1\",\"2\",\"3\"]}", builder.string()); } public void testLoadEmptyStream() throws IOException { @@ -604,8 +611,8 @@ public class SettingsTests extends ESTestCase { assertThat(settings.getAsInt("test1.test2.value3", -1), equalTo(2)); // check array - assertThat(settings.get("test1.test3.0"), equalTo("test3-1")); - assertThat(settings.get("test1.test3.1"), equalTo("test3-2")); + assertNull(settings.get("test1.test3.0")); + assertNull(settings.get("test1.test3.1")); assertThat(settings.getAsArray("test1.test3").length, equalTo(2)); assertThat(settings.getAsArray("test1.test3")[0], equalTo("test3-1")); assertThat(settings.getAsArray("test1.test3")[1], equalTo("test3-2")); @@ -638,4 +645,78 @@ public class SettingsTests extends ESTestCase { e.getMessage(), e.getMessage().contains("null-valued setting found for key [foo] found at line number [1], column number [5]")); } + + public void testReadLegacyFromStream() throws IOException { + BytesStreamOutput output = new BytesStreamOutput(); + output.setVersion(VersionUtils.getPreviousVersion(Version.CURRENT)); + output.writeVInt(5); + output.writeString("foo.bar.1"); + output.writeOptionalString("1"); + output.writeString("foo.bar.0"); + output.writeOptionalString("0"); + output.writeString("foo.bar.2"); + output.writeOptionalString("2"); + output.writeString("foo.bar.3"); + output.writeOptionalString("3"); + output.writeString("foo.bar.baz"); + output.writeOptionalString("baz"); + StreamInput in = StreamInput.wrap(BytesReference.toBytes(output.bytes())); + in.setVersion(VersionUtils.getPreviousVersion(Version.CURRENT)); + Settings settings = Settings.readSettingsFromStream(in); + assertEquals(2, settings.size()); + assertArrayEquals(new String[]{"0", "1", "2", "3"}, settings.getAsArray("foo.bar")); + assertEquals("baz", settings.get("foo.bar.baz")); + } + + public void testWriteLegacyOutput() throws IOException { + BytesStreamOutput output = new BytesStreamOutput(); + output.setVersion(VersionUtils.getPreviousVersion(Version.CURRENT)); + Settings settings = Settings.builder().putArray("foo.bar", "0", "1", "2", "3") + .put("foo.bar.baz", "baz").putNull("foo.null").build(); + Settings.writeSettingsToStream(settings, output); + StreamInput in = StreamInput.wrap(BytesReference.toBytes(output.bytes())); + assertEquals(6, in.readVInt()); + Map keyValues = new HashMap<>(); + for (int i = 0; i < 6; i++){ + keyValues.put(in.readString(), in.readOptionalString()); + } + assertEquals(keyValues.get("foo.bar.0"), "0"); + assertEquals(keyValues.get("foo.bar.1"), "1"); + assertEquals(keyValues.get("foo.bar.2"), "2"); + assertEquals(keyValues.get("foo.bar.3"), "3"); + assertEquals(keyValues.get("foo.bar.baz"), "baz"); + assertTrue(keyValues.containsKey("foo.null")); + assertNull(keyValues.get("foo.null")); + + in = StreamInput.wrap(BytesReference.toBytes(output.bytes())); + in.setVersion(output.getVersion()); + Settings readSettings = Settings.readSettingsFromStream(in); + assertEquals(3, readSettings.size()); + assertArrayEquals(new String[] {"0", "1", "2", "3"}, readSettings.getAsArray("foo.bar")); + assertEquals(readSettings.get("foo.bar.baz"), "baz"); + assertTrue(readSettings.keySet().contains("foo.null")); + assertNull(readSettings.get("foo.null")); + } + + public void testReadWriteArray() throws IOException { + BytesStreamOutput output = new BytesStreamOutput(); + output.setVersion(Version.CURRENT); + Settings settings = Settings.builder().putArray("foo.bar", "0", "1", "2", "3").put("foo.bar.baz", "baz").build(); + Settings.writeSettingsToStream(settings, output); + StreamInput in = StreamInput.wrap(BytesReference.toBytes(output.bytes())); + Settings build = Settings.readSettingsFromStream(in); + assertEquals(2, build.size()); + assertArrayEquals(build.getAsArray("foo.bar"), new String[] {"0", "1", "2", "3"}); + assertEquals(build.get("foo.bar.baz"), "baz"); + } + + public void testCopy() { + Settings settings = Settings.builder().putArray("foo.bar", "0", "1", "2", "3").put("foo.bar.baz", "baz").putNull("test").build(); + assertArrayEquals(new String[] {"0", "1", "2", "3"}, Settings.builder().copy("foo.bar", settings).build().getAsArray("foo.bar")); + assertEquals("baz", Settings.builder().copy("foo.bar.baz", settings).build().get("foo.bar.baz")); + assertNull(Settings.builder().copy("foo.bar.baz", settings).build().get("test")); + assertTrue(Settings.builder().copy("test", settings).build().keySet().contains("test")); + IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> Settings.builder().copy("not_there", settings)); + assertEquals("source key not found in the source settings", iae.getMessage()); + } } diff --git a/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/MassiveWordListTests.java b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/MassiveWordListTests.java new file mode 100644 index 00000000000..081580a6ae9 --- /dev/null +++ b/modules/analysis-common/src/test/java/org/elasticsearch/analysis/common/MassiveWordListTests.java @@ -0,0 +1,50 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.analysis.common; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.ESSingleNodeTestCase; + +import java.util.Collection; +import java.util.Collections; + +public class MassiveWordListTests extends ESSingleNodeTestCase { + + @Override + protected Collection> getPlugins() { + return Collections.singleton(CommonAnalysisPlugin.class); + } + + public void testCreateIndexWithMassiveWordList() { + String[] wordList = new String[100000]; + for (int i = 0; i < wordList.length; i++) { + wordList[i] = "hello world"; + } + client().admin().indices().prepareCreate("test").setSettings(Settings.builder() + .put("index.number_of_shards", 1) + .put("analysis.analyzer.test_analyzer.type", "custom") + .put("analysis.analyzer.test_analyzer.tokenizer", "standard") + .putArray("analysis.analyzer.test_analyzer.filter", "dictionary_decompounder", "lowercase") + .put("analysis.filter.dictionary_decompounder.type", "dictionary_decompounder") + .putArray("analysis.filter.dictionary_decompounder.word_list", wordList) + ).get(); + } +} diff --git a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2Service.java b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2Service.java index 58e34136f2e..880be6c0373 100644 --- a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2Service.java +++ b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2Service.java @@ -115,8 +115,8 @@ interface AwsEc2Service { * instances with a tag key set to stage, and a value of dev. Several tags set will require all of those tags to be set for the * instance to be included. */ - Setting.AffixSetting TAG_SETTING = Setting.prefixKeySetting("discovery.ec2.tag.", - key -> Setting.simpleString(key, Property.NodeScope)); + Setting.AffixSetting> TAG_SETTING = Setting.prefixKeySetting("discovery.ec2.tag.", + key -> Setting.listSetting(key, Collections.emptyList(), Function.identity(), Property.NodeScope)); AmazonEC2 client(); } diff --git a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java index 117490593d3..f291413d408 100644 --- a/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java +++ b/plugins/discovery-ec2/src/main/java/org/elasticsearch/discovery/ec2/AwsEc2UnicastHostsProvider.java @@ -65,7 +65,7 @@ class AwsEc2UnicastHostsProvider extends AbstractComponent implements UnicastHos private final Set groups; - private final Map tags; + private final Map> tags; private final Set availabilityZones; @@ -206,7 +206,7 @@ class AwsEc2UnicastHostsProvider extends AbstractComponent implements UnicastHos new Filter("instance-state-name").withValues("running", "pending") ); - for (Map.Entry tagFilter : tags.entrySet()) { + for (Map.Entry> tagFilter : tags.entrySet()) { // for a given tag key, OR relationship for multiple different values describeInstancesRequest.withFilters( new Filter("tag:" + tagFilter.getKey()).withValues(tagFilter.getValue()) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index f4da904c53d..b0b0ffc9df8 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -416,7 +416,7 @@ public abstract class ESIntegTestCase extends ESTestCase { randomSettingsBuilder.put("index.codec", CodecService.LUCENE_DEFAULT_CODEC); } - for (String setting : randomSettingsBuilder.internalMap().keySet()) { + for (String setting : randomSettingsBuilder.keys()) { assertThat("non index. prefix setting set on index template, its a node setting...", setting, startsWith("index.")); } // always default delayed allocation to 0 to make sure we have tests are not delayed