diff --git a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java index 17dcba4f2fb..e00948cd814 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java @@ -224,6 +224,18 @@ public abstract class AbstractScopedSettings { addSettingsUpdater(Setting.groupedSettingsUpdater(consumer, settings)); } + /** + * Adds a settings consumer that is only executed if any setting in the supplied list of settings is changed. In that case all the + * settings are specified in the argument are returned. The validator is run across all specified settings before the settings are + * applied. + * + * Also automatically adds empty consumers for all settings in order to activate logging + */ + public synchronized void addSettingsUpdateConsumer(Consumer consumer, List> settings, + Consumer validator) { + addSettingsUpdater(Setting.groupedSettingsUpdater(consumer, settings, validator)); + } + /** * Adds a settings consumer for affix settings. Affix settings have a namespace associated to it that needs to be available to the * consumer in order to be processed correctly. diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 2d91e5d5546..a0524bf0c34 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -405,10 +405,13 @@ public final class ClusterSettings extends AbstractScopedSettings { NetworkService.TCP_CONNECT_TIMEOUT, IndexSettings.QUERY_STRING_ANALYZE_WILDCARD, IndexSettings.QUERY_STRING_ALLOW_LEADING_WILDCARD, + ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING, + ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, + ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING, ScriptService.SCRIPT_CACHE_SIZE_SETTING, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING, + ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING, ScriptService.SCRIPT_MAX_SIZE_IN_BYTES, - ScriptService.SCRIPT_MAX_COMPILATIONS_RATE, ScriptService.TYPES_ALLOWED_SETTING, ScriptService.CONTEXTS_ALLOWED_SETTING, IndicesService.INDICES_CACHE_CLEAN_INTERVAL_SETTING, diff --git a/server/src/main/java/org/elasticsearch/common/settings/Setting.java b/server/src/main/java/org/elasticsearch/common/settings/Setting.java index 51d9e570018..796123d9cb6 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -667,7 +667,12 @@ public class Setting implements ToXContentObject { static AbstractScopedSettings.SettingUpdater groupedSettingsUpdater(Consumer consumer, final List> configuredSettings) { + return groupedSettingsUpdater(consumer, configuredSettings, (v) -> {}); + } + static AbstractScopedSettings.SettingUpdater groupedSettingsUpdater(Consumer consumer, + final List> configuredSettings, + Consumer validator) { return new AbstractScopedSettings.SettingUpdater() { private Settings get(Settings settings) { @@ -690,6 +695,7 @@ public class Setting implements ToXContentObject { @Override public Settings getValue(Settings current, Settings previous) { + validator.accept(current); return get(current); } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCache.java b/server/src/main/java/org/elasticsearch/script/ScriptCache.java index e5b351e2239..a37851cc810 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptCache.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptCache.java @@ -40,53 +40,47 @@ public class ScriptCache { private static final Logger logger = LogManager.getLogger(ScriptService.class); - private Cache cache; - private final ScriptMetrics scriptMetrics = new ScriptMetrics(); + private final Cache cache; + private final ScriptMetrics scriptMetrics; private final Object lock = new Object(); - private Tuple rate; + // Mutable fields private long lastInlineCompileTime; private double scriptsPerTimeWindow; - private double compilesAllowedPerNano; - // Cache settings - private int cacheSize; - private TimeValue cacheExpire; + // Cache settings or derived from settings + final int cacheSize; + final TimeValue cacheExpire; + final Tuple rate; + private final double compilesAllowedPerNano; - public ScriptCache( + ScriptCache( int cacheMaxSize, TimeValue cacheExpire, Tuple maxCompilationRate ) { - CacheBuilder cacheBuilder = CacheBuilder.builder(); - if (cacheMaxSize >= 0) { - cacheBuilder.setMaximumWeight(cacheMaxSize); - } - - if (cacheExpire.getNanos() != 0) { - cacheBuilder.setExpireAfterAccess(cacheExpire); - } - - logger.debug("using script cache with max_size [{}], expire [{}]", cacheMaxSize, cacheExpire); - this.cache = cacheBuilder.removalListener(new ScriptCacheRemovalListener()).build(); - - this.lastInlineCompileTime = System.nanoTime(); - this.cacheSize = cacheMaxSize; this.cacheExpire = cacheExpire; - this.setMaxCompilationRate(maxCompilationRate); - } - private Cache buildCache() { CacheBuilder cacheBuilder = CacheBuilder.builder(); - if (cacheSize >= 0) { - cacheBuilder.setMaximumWeight(cacheSize); + if (this.cacheSize >= 0) { + cacheBuilder.setMaximumWeight(this.cacheSize); } - if (cacheExpire.getNanos() != 0) { - cacheBuilder.setExpireAfterAccess(cacheExpire); + + if (this.cacheExpire.getNanos() != 0) { + cacheBuilder.setExpireAfterAccess(this.cacheExpire); } - return cacheBuilder.removalListener(new ScriptCacheRemovalListener()).build(); + + logger.debug("using script cache with max_size [{}], expire [{}]", this.cacheSize, this.cacheExpire); + this.cache = cacheBuilder.removalListener(new ScriptCacheRemovalListener()).build(); + + this.rate = maxCompilationRate; + this.scriptsPerTimeWindow = this.rate.v1(); + this.compilesAllowedPerNano = ((double) rate.v1()) / rate.v2().nanos(); + + this.lastInlineCompileTime = System.nanoTime(); + this.scriptMetrics = new ScriptMetrics(); } FactoryType compile( @@ -185,22 +179,6 @@ public class ScriptCache { } } - /** - * This configures the maximum script compilations per five minute window. - * - * @param newRate the new expected maximum number of compilations per five minute window - */ - void setMaxCompilationRate(Tuple newRate) { - synchronized (lock) { - this.rate = newRate; - // Reset the counter to allow new compilations - this.scriptsPerTimeWindow = rate.v1(); - this.compilesAllowedPerNano = ((double) rate.v1()) / newRate.v2().nanos(); - - this.cache = buildCache(); - } - } - /** * A small listener for the script cache that calls each * {@code ScriptEngine}'s {@code scriptRemoved} method when the diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index 47661b53df0..ea9f572669f 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -45,13 +45,17 @@ import org.elasticsearch.core.internal.io.IOUtils; import java.io.Closeable; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.stream.Collectors; @@ -61,6 +65,11 @@ public class ScriptService implements Closeable, ClusterStateApplier { static final String DISABLE_DYNAMIC_SCRIPTING_SETTING = "script.disable_dynamic"; + // Special setting value for SCRIPT_GENERAL_MAX_COMPILATIONS_RATE to indicate the script service should use context + // specific caches + static final Tuple USE_CONTEXT_RATE_VALUE = new Tuple<>(-1, TimeValue.MINUS_ONE); + static final String USE_CONTEXT_RATE_KEY = "use-context"; + // a parsing function that requires a non negative int and a timevalue as arguments split by a slash // this allows you to easily define rates static final Function> MAX_COMPILATION_RATE_FUNCTION = @@ -93,14 +102,39 @@ public class ScriptService implements Closeable, ClusterStateApplier { } }; - public static final Setting SCRIPT_CACHE_SIZE_SETTING = + public static final Setting SCRIPT_GENERAL_CACHE_SIZE_SETTING = Setting.intSetting("script.cache.max_size", 100, 0, Property.NodeScope); - public static final Setting SCRIPT_CACHE_EXPIRE_SETTING = + public static final Setting SCRIPT_GENERAL_CACHE_EXPIRE_SETTING = Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope); public static final Setting SCRIPT_MAX_SIZE_IN_BYTES = Setting.intSetting("script.max_size_in_bytes", 65535, 0, Property.Dynamic, Property.NodeScope); - public static final Setting> SCRIPT_MAX_COMPILATIONS_RATE = - new Setting<>("script.max_compilations_rate", "75/5m", MAX_COMPILATION_RATE_FUNCTION, Property.Dynamic, Property.NodeScope); + public static final Setting> SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING = + new Setting<>("script.max_compilations_rate", "75/5m", + (String value) -> value.equals(USE_CONTEXT_RATE_KEY) ? USE_CONTEXT_RATE_VALUE: MAX_COMPILATION_RATE_FUNCTION.apply(value), + Property.Dynamic, Property.NodeScope); + + // Per-context settings + static final String CONTEXT_PREFIX = "script.context."; + + // script.context..{cache_max_size, cache_expire, max_compilations_rate} + + public static final Setting.AffixSetting SCRIPT_CACHE_SIZE_SETTING = + Setting.affixKeySetting(CONTEXT_PREFIX, + "cache_max_size", + key -> Setting.intSetting(key, SCRIPT_GENERAL_CACHE_SIZE_SETTING, 0, Property.NodeScope, Property.Dynamic)); + + public static final Setting.AffixSetting SCRIPT_CACHE_EXPIRE_SETTING = + Setting.affixKeySetting(CONTEXT_PREFIX, + "cache_expire", + key -> Setting.positiveTimeSetting(key, SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, TimeValue.timeValueMillis(0), + Property.NodeScope, Property.Dynamic)); + + public static final Setting.AffixSetting> SCRIPT_MAX_COMPILATIONS_RATE_SETTING = + Setting.affixKeySetting(CONTEXT_PREFIX, + "max_compilations_rate", + key -> new Setting<>(key, "75/5m", MAX_COMPILATION_RATE_FUNCTION, Property.NodeScope, Property.Dynamic)); + + private static final Tuple SCRIPT_COMPILATION_RATE_ZERO = new Tuple<>(0, TimeValue.ZERO); public static final String ALLOW_NONE = "none"; @@ -109,12 +143,9 @@ public class ScriptService implements Closeable, ClusterStateApplier { public static final Setting> CONTEXTS_ALLOWED_SETTING = Setting.listSetting("script.allowed_contexts", Collections.emptyList(), Function.identity(), Setting.Property.NodeScope); - private final Settings settings; private final Set typesAllowed; private final Set contextsAllowed; - final ScriptCache compiler; - private final Map engines; private final Map> contexts; @@ -122,8 +153,9 @@ public class ScriptService implements Closeable, ClusterStateApplier { private int maxSizeInBytes; + private final AtomicReference cacheHolder; + public ScriptService(Settings settings, Map engines, Map> contexts) { - this.settings = Objects.requireNonNull(settings); this.engines = Objects.requireNonNull(engines); this.contexts = Objects.requireNonNull(contexts); @@ -201,13 +233,10 @@ public class ScriptService implements Closeable, ClusterStateApplier { } this.setMaxSizeInBytes(SCRIPT_MAX_SIZE_IN_BYTES.get(settings)); - compiler = new ScriptCache( - SCRIPT_CACHE_SIZE_SETTING.get(settings), - SCRIPT_CACHE_EXPIRE_SETTING.get(settings), - compilationLimitsEnabled() ? - SCRIPT_MAX_COMPILATIONS_RATE.get(settings): - new Tuple<>(0, TimeValue.ZERO) - ); + + // Validation requires knowing which contexts exist. + this.validateCacheSettings(settings); + cacheHolder = new AtomicReference<>(new CacheHolder(settings, contexts.keySet(), compilationLimitsEnabled())); } /** @@ -219,7 +248,66 @@ public class ScriptService implements Closeable, ClusterStateApplier { void registerClusterSettingsListeners(ClusterSettings clusterSettings) { clusterSettings.addSettingsUpdateConsumer(SCRIPT_MAX_SIZE_IN_BYTES, this::setMaxSizeInBytes); - clusterSettings.addSettingsUpdateConsumer(SCRIPT_MAX_COMPILATIONS_RATE, compiler::setMaxCompilationRate); + + // Handle all updatable per-context settings at once for each context. + for (String context: contexts.keySet()) { + clusterSettings.addSettingsUpdateConsumer( + (settings) -> cacheHolder.get().updateContextSettings(settings, context), + Arrays.asList(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context), + SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context), + SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context), + SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, + // general settings used for fallbacks + SCRIPT_GENERAL_CACHE_SIZE_SETTING) + ); + } + + // Handle all settings for context and general caches, this flips between general and context caches. + clusterSettings.addSettingsUpdateConsumer( + (settings) -> cacheHolder.set(cacheHolder.get().withUpdatedCacheSettings(settings)), + Arrays.asList(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING, + SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, + SCRIPT_GENERAL_CACHE_SIZE_SETTING, + SCRIPT_MAX_COMPILATIONS_RATE_SETTING, + SCRIPT_CACHE_EXPIRE_SETTING, + SCRIPT_CACHE_SIZE_SETTING), + this::validateCacheSettings + ); + } + + /** + * Throw an IllegalArgumentException if any per-context setting does not match a context or if per-context settings are configured + * when using the general cache. + */ + void validateCacheSettings(Settings settings) { + boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE); + List> affixes = Arrays.asList(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, SCRIPT_CACHE_EXPIRE_SETTING, + SCRIPT_CACHE_SIZE_SETTING); + List keys = new ArrayList<>(); + for (Setting.AffixSetting affix: affixes) { + keys.addAll(getConcreteSettingKeys(affix, settings)); + } + if (useContext == false && keys.isEmpty() == false) { + throw new IllegalArgumentException("Context cache settings [" + String.join(", ", keys) + "] requires [" + + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey() + "] to be [" + USE_CONTEXT_RATE_KEY + "]"); + } + } + + /** + * Get concrete settings keys from affix settings for given Settings. Throws an IllegalArgumentException if the namespace of matching + * affix settings do not match any context name. + */ + List getConcreteSettingKeys(Setting.AffixSetting setting, Settings settings) { + List concreteKeys = new ArrayList<>(); + for (String context: setting.getAsMap(settings).keySet()) { + String s = setting.getConcreteSettingForNamespace(context).getKey(); + if (contexts.containsKey(context) == false) { + throw new IllegalArgumentException("Context [" + context + "] doesn't exist for setting [" + s + "]"); + } + concreteKeys.add(s); + } + concreteKeys.sort(Comparator.naturalOrder()); + return concreteKeys; } @Override @@ -304,7 +392,9 @@ public class ScriptService implements Closeable, ClusterStateApplier { logger.trace("compiling lang: [{}] type: [{}] script: {}", lang, type, idOrCode); } - return compiler.compile(context, scriptEngine, id, idOrCode, type, options); + ScriptCache scriptCache = cacheHolder.get().get(context.name); + assert scriptCache != null : "script context [" + context.name + "] has no script cache"; + return scriptCache.compile(context, scriptEngine, id, idOrCode, type, options); } public boolean isLangSupported(String lang) { @@ -468,11 +558,119 @@ public class ScriptService implements Closeable, ClusterStateApplier { } public ScriptStats stats() { - return compiler.stats(); + return cacheHolder.get().stats(); } @Override public void applyClusterState(ClusterChangedEvent event) { clusterState = event.state(); } + + /** + * Container for the ScriptCache(s). This class operates in two modes: + * 1) general mode, if the general script cache is configured. There are no context caches in this case. + * 2) context mode, if the context script cache is configured. There is no general cache in this case. + */ + static class CacheHolder { + final ScriptCache general; + final Map> contextCache; + + final Set contexts; + final boolean compilationLimitsEnabled; + + CacheHolder(Settings settings, Set contexts, boolean compilationLimitsEnabled) { + this.compilationLimitsEnabled = compilationLimitsEnabled; + this.contexts = Collections.unmodifiableSet(contexts); + if (SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE)) { + this.general = null; + Map> contextCache = new HashMap<>(this.contexts.size()); + for (String context : this.contexts) { + contextCache.put(context, new AtomicReference<>(contextFromSettings(settings, context, this.compilationLimitsEnabled))); + } + this.contextCache = Collections.unmodifiableMap(contextCache); + } else { + this.contextCache = null; + this.general = new ScriptCache( + SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(settings), + SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(settings), + compilationLimitsEnabled ? + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings) : + SCRIPT_COMPILATION_RATE_ZERO); + } + } + + /** + * Create a ScriptCache for the given context. + */ + private static ScriptCache contextFromSettings(Settings settings, String context, boolean compilationLimitsEnabled) { + return new ScriptCache(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context).get(settings), + SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context).get(settings), + compilationLimitsEnabled ? + SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context).get(settings) : + SCRIPT_COMPILATION_RATE_ZERO); + } + + /** + * Returns a CacheHolder with the given settings. Flips between general and context caches if necessary. Creates new general + * cache if in general cache mode and {@code script.max_compilations_rate} has changed to any value other than {@code use-context}. + */ + CacheHolder withUpdatedCacheSettings(Settings settings) { + if (SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE)) { + if (general != null) { + // Flipping to context specific + logger.debug("Switching to context cache from general cache"); + return new CacheHolder(settings, contexts, compilationLimitsEnabled); + } + } else if (general == null) { + // Flipping to general + logger.debug("Switching from context cache to general cache"); + return new CacheHolder(settings, contexts, compilationLimitsEnabled); + } else if (general.rate.equals(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings)) == false) { + // General compilation rate changed, that setting is the only dynamically updated general setting + logger.debug("General compilation rate changed from [" + general.rate + "] to [" + + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings) + "], creating new general cache"); + return new CacheHolder(settings, contexts, compilationLimitsEnabled); + } + + // no-op change, this is possible when context settings change while in context mode + return this; + } + + /** + * get the cache appropriate for the context. If in general mode, return the general cache. Otherwise return the ScriptCache for + * the given context. Returns null in context mode if the requested context does not exist. + */ + ScriptCache get(String context) { + if (general != null) { + return general; + } + AtomicReference ref = contextCache.get(context); + if (ref == null) { + return null; + } + return ref.get(); + } + + ScriptStats stats() { + if (general != null) { + return general.stats(); + } + return ScriptStats.sum(contextCache.values().stream().map(AtomicReference::get).map(ScriptCache::stats)::iterator); + } + + /** + * Update settings for the context cache, if we're in the context cache mode otherwise no-op. + */ + void updateContextSettings(Settings settings, String context) { + if (general != null) { + return; + } + AtomicReference ref = contextCache.get(context); + assert ref != null : "expected script cache to exist for context [" + context + "]"; + ScriptCache cache = ref.get(); + assert cache != null : "expected script cache to be non-null for context [" + context + "]"; + ref.set(contextFromSettings(settings, context, compilationLimitsEnabled)); + logger.debug("Replaced context [" + context + "] with new settings"); + } + } } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptStats.java b/server/src/main/java/org/elasticsearch/script/ScriptStats.java index 06eec72c0e0..87a5a1d4916 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptStats.java @@ -82,4 +82,20 @@ public class ScriptStats implements Writeable, ToXContentFragment { static final String CACHE_EVICTIONS = "cache_evictions"; static final String COMPILATION_LIMIT_TRIGGERED = "compilation_limit_triggered"; } + + public static ScriptStats sum(Iterable stats) { + long compilations = 0; + long cacheEvictions = 0; + long compilationLimitTriggered = 0; + for (ScriptStats stat: stats) { + compilations += stat.compilations; + cacheEvictions += stat.cacheEvictions; + compilationLimitTriggered += stat.compilationLimitTriggered; + } + return new ScriptStats( + compilations, + cacheEvictions, + compilationLimitTriggered + ); + } } diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java index aac92f4ce5f..7f87f83e83a 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingTests.java @@ -1079,6 +1079,58 @@ public class SettingTests extends ESTestCase { assertThat(affixSetting.getNamespaces(Settings.builder().put("prefix.infix.suffix.anything", "anything").build()), hasSize(1)); } + public void testGroupSettingUpdaterValidator() { + final Setting.AffixSetting affixSetting = + Setting.affixKeySetting("prefix.","suffix", + (key) -> Setting.intSetting(key, 5, Setting.Property.Dynamic, Setting.Property.NodeScope)); + Setting fixSetting = Setting.intSetting("abc", 1, Property.NodeScope); + + Consumer validator = s -> { + if (affixSetting.getNamespaces(s).contains("foo")) { + if (fixSetting.get(s) == 2) { + throw new IllegalArgumentException("foo and 2 can't go together"); + } + } else if (affixSetting.getNamespaces(s).contains("bar")) { + throw new IllegalArgumentException("no bar"); + } + }; + + AbstractScopedSettings.SettingUpdater updater = Setting.groupedSettingsUpdater(s -> {}, + Arrays.asList(affixSetting, fixSetting), validator); + + IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> { + updater.getValue( + Settings.builder() + .put("prefix.foo.suffix", 5) + .put("abc", 2) + .build(), + Settings.EMPTY + ); + }); + assertEquals("foo and 2 can't go together", illegal.getMessage()); + + illegal = expectThrows(IllegalArgumentException.class, () -> { + updater.getValue( + Settings.builder() + .put("prefix.bar.suffix", 6) + .put("abc", 3) + .build(), + Settings.EMPTY + ); + }); + assertEquals("no bar", illegal.getMessage()); + + Settings s = updater.getValue( + Settings.builder() + .put("prefix.foo.suffix", 5) + .put("prefix.bar.suffix", 5) + .put("abc", 3) + .build(), + Settings.EMPTY + ); + assertNotNull(s); + } + public void testExists() { final Setting fooSetting = Setting.simpleString("foo", Property.NodeScope); assertFalse(fooSetting.exists(Settings.EMPTY)); diff --git a/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java b/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java index e22acfca3ae..41757d5d7b9 100644 --- a/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java +++ b/server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java @@ -69,7 +69,7 @@ public class IndicesServiceCloseTests extends ESTestCase { .put(Environment.PATH_REPO_SETTING.getKey(), tempDir.resolve("repo")) .put(Environment.PATH_SHARED_DATA_SETTING.getKey(), createTempDir().getParent()) .put(Node.NODE_NAME_SETTING.getKey(), nodeName) - .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE.getKey(), "1000/1m") + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "1000/1m") .put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 1) // limit the number of threads created .put("transport.type", getTestTransportType()) .put(Node.NODE_DATA_SETTING.getKey(), true) diff --git a/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java b/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java index 279456321aa..dfa94609930 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java @@ -28,31 +28,28 @@ public class ScriptCacheTests extends ESTestCase { // even though circuit breaking is allowed to be configured per minute, we actually weigh this over five minutes // simply by multiplying by five, so even setting it to one, requires five compilations to break public void testCompilationCircuitBreaking() throws Exception { - ScriptCache cache = new ScriptCache( - ScriptService.SCRIPT_CACHE_SIZE_SETTING.get(Settings.EMPTY), - ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.get(Settings.EMPTY), - ScriptService.SCRIPT_MAX_COMPILATIONS_RATE.get(Settings.EMPTY) - ); - cache.setMaxCompilationRate(Tuple.tuple(1, TimeValue.timeValueMinutes(1))); + final TimeValue expire = ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(Settings.EMPTY); + final Integer size = ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(Settings.EMPTY); + Tuple rate = ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(Settings.EMPTY); + ScriptCache cache = new ScriptCache(size, expire, Tuple.tuple(1, TimeValue.timeValueMinutes(1))); cache.checkCompilationLimit(); // should pass - expectThrows(CircuitBreakingException.class, () -> cache.checkCompilationLimit()); - cache.setMaxCompilationRate(Tuple.tuple(2, TimeValue.timeValueMinutes(1))); + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, (Tuple.tuple(2, TimeValue.timeValueMinutes(1)))); cache.checkCompilationLimit(); // should pass cache.checkCompilationLimit(); // should pass - expectThrows(CircuitBreakingException.class, () -> cache.checkCompilationLimit()); + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); int count = randomIntBetween(5, 50); - cache.setMaxCompilationRate(Tuple.tuple(count, TimeValue.timeValueMinutes(1))); + cache = new ScriptCache(size, expire, (Tuple.tuple(count, TimeValue.timeValueMinutes(1)))); for (int i = 0; i < count; i++) { cache.checkCompilationLimit(); // should pass } - expectThrows(CircuitBreakingException.class, () -> cache.checkCompilationLimit()); - cache.setMaxCompilationRate(Tuple.tuple(0, TimeValue.timeValueMinutes(1))); - expectThrows(CircuitBreakingException.class, () -> cache.checkCompilationLimit()); - cache.setMaxCompilationRate(Tuple.tuple(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1))); + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, (Tuple.tuple(0, TimeValue.timeValueMinutes(1)))); + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, (Tuple.tuple(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)))); int largeLimit = randomIntBetween(1000, 10000); for (int i = 0; i < largeLimit; i++) { cache.checkCompilationLimit(); } } - } diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 814578a29f2..7914c905770 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -36,12 +36,17 @@ import org.elasticsearch.test.ESTestCase; import org.junit.Before; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.function.Function; import static org.elasticsearch.script.ScriptService.MAX_COMPILATION_RATE_FUNCTION; +import static org.elasticsearch.script.ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING; +import static org.elasticsearch.script.ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -222,7 +227,7 @@ public class ScriptServiceTests extends ESTestCase { public void testCompilationStatsOnCacheHit() throws IOException { Settings.Builder builder = Settings.builder(); - builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), 1); + builder.put(ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), 1); buildScriptService(builder.build()); Script script = new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()); ScriptContext context = randomFrom(contexts.values()); @@ -239,7 +244,7 @@ public class ScriptServiceTests extends ESTestCase { public void testCacheEvictionCountedInCacheEvictionsStats() throws IOException { Settings.Builder builder = Settings.builder(); - builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), 1); + builder.put(ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), 1); buildScriptService(builder.build()); scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(contexts.values())); scriptService.compile(new Script(ScriptType.INLINE, "test", "2+2", Collections.emptyMap()), randomFrom(contexts.values())); @@ -309,6 +314,201 @@ public class ScriptServiceTests extends ESTestCase { iae.getMessage()); } + public void testConflictContextSettings() throws IOException { + IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("field").getKey(), 123).build()); + }); + assertEquals("Context cache settings [script.context.field.cache_max_size] requires " + + "[script.max_compilations_rate] to be [use-context]", + illegal.getMessage() + ); + + illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("ingest").getKey(), "5m").build()); + }); + + assertEquals("Context cache settings [script.context.ingest.cache_expire] requires " + + "[script.max_compilations_rate] to be [use-context]", + illegal.getMessage() + ); + + illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("score").getKey(), "50/5m").build()); + }); + + assertEquals("Context cache settings [script.context.score.max_compilations_rate] requires " + + "[script.max_compilations_rate] to be [use-context]", + illegal.getMessage() + ); + + buildScriptService( + Settings.builder() + .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("ingest").getKey(), "5m") + .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("field").getKey(), 123) + .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("score").getKey(), "50/5m") + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY).build()); + } + + public void testFallbackContextSettings() { + int cacheSizeBackup = randomIntBetween(0, 1024); + int cacheSizeFoo = randomValueOtherThan(cacheSizeBackup, () -> randomIntBetween(0, 1024)); + + String cacheExpireBackup = randomTimeValue(1, 1000, "h"); + TimeValue cacheExpireBackupParsed = TimeValue.parseTimeValue(cacheExpireBackup, ""); + String cacheExpireFoo = randomValueOtherThan(cacheExpireBackup, () -> randomTimeValue(1, 1000, "h")); + TimeValue cacheExpireFooParsed = TimeValue.parseTimeValue(cacheExpireFoo, ""); + + Settings s = Settings.builder() + .put(ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), cacheSizeBackup) + .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo").getKey(), cacheSizeFoo) + .put(ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), cacheExpireBackup) + .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo").getKey(), cacheExpireFoo) + .build(); + + assertEquals(cacheSizeFoo, ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo").get(s).intValue()); + assertEquals(cacheSizeBackup, ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("bar").get(s).intValue()); + + assertEquals(cacheExpireFooParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo").get(s)); + assertEquals(cacheExpireBackupParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("bar").get(s)); + } + + public void testUseContextSettingValue() { + Settings s = Settings.builder() + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY) + .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("foo").getKey(), + ScriptService.USE_CONTEXT_RATE_KEY) + .build(); + + assertEquals(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(s), ScriptService.USE_CONTEXT_RATE_VALUE); + + IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> { + ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getAsMap(s); + }); + + assertEquals("parameter must contain a positive integer and a timevalue, i.e. 10/1m, but was [use-context]", illegal.getMessage()); + } + + public void testCacheHolderGeneralConstructor() { + String compilationRate = "77/5m"; + Tuple rate = ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(compilationRate); + boolean compilationLimitsEnabled = true; + ScriptService.CacheHolder holder = new ScriptService.CacheHolder( + Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate).build(), + new HashSet<>(Collections.singleton("foo")), + compilationLimitsEnabled + ); + + assertNotNull(holder.general); + assertNull(holder.contextCache); + assertEquals(holder.general.rate, rate); + + compilationLimitsEnabled = false; + holder = new ScriptService.CacheHolder( + Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate).build(), + new HashSet<>(Collections.singleton("foo")), + compilationLimitsEnabled + ); + + assertNotNull(holder.general); + assertNull(holder.contextCache); + assertEquals(holder.general.rate, new Tuple<>(0, TimeValue.ZERO)); + } + + public void testCacheHolderContextConstructor() { + String fooCompilationRate = "77/5m"; + String barCompilationRate = "78/6m"; + boolean compilationLimitsEnabled = true; + + Settings s = Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY) + .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("foo").getKey(), fooCompilationRate) + .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("bar").getKey(), barCompilationRate) + .build(); + Set contexts = new HashSet<>(Arrays.asList("foo", "bar", "baz")); + ScriptService.CacheHolder holder = new ScriptService.CacheHolder(s, contexts, compilationLimitsEnabled); + + assertNull(holder.general); + assertNotNull(holder.contextCache); + assertEquals(3, holder.contextCache.size()); + assertEquals(contexts, holder.contextCache.keySet()); + + assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(fooCompilationRate), holder.contextCache.get("foo").get().rate); + assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(barCompilationRate), holder.contextCache.get("bar").get().rate); + assertEquals(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getDefault(Settings.EMPTY), + holder.contextCache.get("baz").get().rate); + + Tuple zero = new Tuple<>(0, TimeValue.ZERO); + compilationLimitsEnabled = false; + holder = new ScriptService.CacheHolder(s, contexts, compilationLimitsEnabled); + + assertNotNull(holder.contextCache); + assertEquals(3, holder.contextCache.size()); + assertEquals(zero, holder.contextCache.get("foo").get().rate); + assertEquals(zero, holder.contextCache.get("bar").get().rate); + assertEquals(zero, holder.contextCache.get("baz").get().rate); + } + + public void testCacheHolderChangeSettings() { + String fooCompilationRate = "77/5m"; + String barCompilationRate = "78/6m"; + String compilationRate = "77/5m"; + Tuple generalRate = MAX_COMPILATION_RATE_FUNCTION.apply(compilationRate); + + Settings s = Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate) + .build(); + Set contexts = new HashSet<>(Arrays.asList("foo", "bar", "baz")); + ScriptService.CacheHolder holder = new ScriptService.CacheHolder(s, contexts, true); + + assertNotNull(holder.general); + assertNull(holder.contextCache); + assertEquals(generalRate, holder.general.rate); + + holder = holder.withUpdatedCacheSettings(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY) + .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("foo").getKey(), fooCompilationRate) + .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("bar").getKey(), barCompilationRate) + .build() + ); + + assertNull(holder.general); + assertNotNull(holder.contextCache); + assertEquals(3, holder.contextCache.size()); + assertEquals(contexts, holder.contextCache.keySet()); + + assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(fooCompilationRate), holder.contextCache.get("foo").get().rate); + assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(barCompilationRate), holder.contextCache.get("bar").get().rate); + assertEquals(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getDefault(Settings.EMPTY), + holder.contextCache.get("baz").get().rate); + + holder.updateContextSettings(Settings.builder() + .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("bar").getKey(), fooCompilationRate).build(), + "bar" + ); + assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(fooCompilationRate), holder.contextCache.get("bar").get().rate); + + holder = holder.withUpdatedCacheSettings(s); + assertNotNull(holder.general); + assertNull(holder.contextCache); + assertEquals(generalRate, holder.general.rate); + + holder = holder.withUpdatedCacheSettings( + Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), barCompilationRate).build() + ); + + assertNotNull(holder.general); + assertNull(holder.contextCache); + assertEquals(MAX_COMPILATION_RATE_FUNCTION.apply(barCompilationRate), holder.general.rate); + + ScriptService.CacheHolder update = holder.withUpdatedCacheSettings( + Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), barCompilationRate).build() + ); + assertSame(holder, update); + } + private void assertCompileRejected(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) { try { scriptService.compile(new Script(scriptType, lang, script, Collections.emptyMap()), scriptContext); diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 003f0efa844..ebeaed1a357 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -518,10 +518,10 @@ public final class InternalTestCluster extends TestCluster { } if (random.nextBoolean()) { - builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), RandomNumbers.randomIntBetween(random, 0, 2000)); + builder.put(ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), RandomNumbers.randomIntBetween(random, 0, 2000)); } if (random.nextBoolean()) { - builder.put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getKey(), + builder.put(ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), timeValueMillis(RandomNumbers.randomIntBetween(random, 750, 10000000)).getStringRep()); } diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java index 829c63f67fe..a39560c40e0 100644 --- a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java +++ b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java @@ -215,7 +215,7 @@ public abstract class CcrIntegTestCase extends ESTestCase { builder.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "1b"); builder.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "1b"); builder.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "1b"); - builder.put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE.getKey(), "2048/1m"); + builder.put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "2048/1m"); // wait short time for other active shards before actually deleting, default 30s not needed in tests builder.put(IndicesStore.INDICES_STORE_DELETE_SHARD_TIMEOUT.getKey(), new TimeValue(1, TimeUnit.SECONDS)); builder.putList(DISCOVERY_SEED_HOSTS_SETTING.getKey()); // empty list disables a port scan for other nodes