From 070ea7eff1d67a054bc23a6557ea081ebdff57d4 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Wed, 18 Mar 2020 12:31:30 -0600 Subject: [PATCH] Scripting: Per-context script cache, default off (#52855) * Adds per context settings: `script.context.${CONTEXT}.cache_max_size` ~ `script.cache.max_size` `script.context.${CONTEXT}.cache_expire` ~ `script.cache.expire` `script.context.${CONTEXT}.max_compilations_rate` ~ `script.max_compilations_rate` * Context cache is used if: `script.max_compilations_rate=use-context`. This value is dynamically updatable, so users can switch back to the general cache if desired. * Settings for context caches take the first value that applies: 1) Context specific settings if set, eg `script.context.ingest.cache_max_size` 2) Correlated general setting is set to the non-default value, eg `script.cache.max_size` 3) Context default The reason for 2's inclusion is to allow an easy transition for users who've customized their general cache settings. Using the general cache settings for the context caches results in higher effective settings, since they are multiplied across the number of contexts. So a general cache max size of 200 will become 200 * # of contexts. However, this behavior it will avoid users snapping to a value that is too low for them. Refs: #50152 --- .../settings/AbstractScopedSettings.java | 12 + .../common/settings/ClusterSettings.java | 5 +- .../common/settings/Setting.java | 6 + .../org/elasticsearch/script/ScriptCache.java | 66 ++--- .../elasticsearch/script/ScriptService.java | 233 ++++++++++++++++-- .../org/elasticsearch/script/ScriptStats.java | 16 ++ .../common/settings/SettingTests.java | 52 ++++ .../indices/IndicesServiceCloseTests.java | 2 +- .../script/ScriptCacheTests.java | 27 +- .../script/ScriptServiceTests.java | 203 ++++++++++++++- .../test/InternalTestCluster.java | 4 +- .../elasticsearch/xpack/CcrIntegTestCase.java | 2 +- 12 files changed, 544 insertions(+), 84 deletions(-) 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 99c651ef7df80..c7e88a3c07c60 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 synchronized void addSettingsUpdateConsumer(Consumer consumer, 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 f4f53f939982d..d121752017c58 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -364,10 +364,13 @@ public void apply(Settings value, Settings current, Settings previous) { NetworkService.TCP_RECEIVE_BUFFER_SIZE, 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 4e926f6c3ca11..0680dd7c2ccfd 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Setting.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Setting.java @@ -668,7 +668,12 @@ public String toString() { 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) { @@ -691,6 +696,7 @@ public boolean hasChanged(Settings current, Settings previous) { @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 e5b351e223965..a37851cc81050 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 ) { + this.cacheSize = cacheMaxSize; + this.cacheExpire = cacheExpire; + CacheBuilder cacheBuilder = CacheBuilder.builder(); - if (cacheMaxSize >= 0) { - cacheBuilder.setMaximumWeight(cacheMaxSize); + if (this.cacheSize >= 0) { + cacheBuilder.setMaximumWeight(this.cacheSize); } - if (cacheExpire.getNanos() != 0) { - cacheBuilder.setExpireAfterAccess(cacheExpire); + if (this.cacheExpire.getNanos() != 0) { + cacheBuilder.setExpireAfterAccess(this.cacheExpire); } - logger.debug("using script cache with max_size [{}], expire [{}]", cacheMaxSize, cacheExpire); + logger.debug("using script cache with max_size [{}], expire [{}]", this.cacheSize, this.cacheExpire); this.cache = cacheBuilder.removalListener(new ScriptCacheRemovalListener()).build(); - this.lastInlineCompileTime = System.nanoTime(); - - this.cacheSize = cacheMaxSize; - this.cacheExpire = cacheExpire; - this.setMaxCompilationRate(maxCompilationRate); - } + this.rate = maxCompilationRate; + this.scriptsPerTimeWindow = this.rate.v1(); + this.compilesAllowedPerNano = ((double) rate.v1()) / rate.v2().nanos(); - private Cache buildCache() { - CacheBuilder cacheBuilder = CacheBuilder.builder(); - if (cacheSize >= 0) { - cacheBuilder.setMaximumWeight(cacheSize); - } - if (cacheExpire.getNanos() != 0) { - cacheBuilder.setExpireAfterAccess(cacheExpire); - } - return cacheBuilder.removalListener(new ScriptCacheRemovalListener()).build(); + this.lastInlineCompileTime = System.nanoTime(); + this.scriptMetrics = new ScriptMetrics(); } FactoryType compile( @@ -185,22 +179,6 @@ void checkCompilationLimit() { } } - /** - * 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 7cf23d26187ad..66ed3fee0d6ca 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -45,13 +45,16 @@ import java.io.Closeable; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; 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 +64,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 +101,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 +142,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 +152,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 +232,10 @@ public ScriptService(Settings settings, Map engines, Map(0, TimeValue.ZERO) - ); + + // Validation requires knowing which contexts exist. + this.validateCacheSettings(settings); + cacheHolder = new AtomicReference<>(new CacheHolder(settings, contexts.keySet(), compilationLimitsEnabled())); } /** @@ -219,7 +247,66 @@ boolean compilationLimitsEnabled() { 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), + List.of(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)), + List.of(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 = List.of(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 +391,9 @@ public FactoryType compile(Script script, ScriptContext> contextCache; + + final Set contexts; + final boolean compilationLimitsEnabled; + + CacheHolder(Settings settings, Set contexts, boolean compilationLimitsEnabled) { + this.compilationLimitsEnabled = compilationLimitsEnabled; + this.contexts = Set.copyOf(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 86a6a0d13569b..953b3a18ae92c 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptStats.java @@ -79,4 +79,20 @@ static final class Fields { 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 56d20d30d7f87..a129b314bd73b 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 void testAffixNamespacesWithGroupSetting() { 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 96bb4c13cb8aa..29b9f12484470 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 @@ private Node startNode() throws NodeValidationException { .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 279456321aad7..dfa946099306f 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 814578a29f2f7..77ce3fe127c7f 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -38,10 +38,14 @@ import java.io.IOException; 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 +226,7 @@ public void testMultipleCompilationsCountedInCompilationStats() throws IOExcepti 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 +243,7 @@ public void testIndexedScriptCountedInCompilationStats() throws IOException { 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 +313,201 @@ public void testMaxSizeLimit() throws Exception { 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 = Set.of("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 = Set.of("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 fef1dcacba309..e913f1bfc8a02 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -495,10 +495,10 @@ private static Settings getRandomNodeSettings(long seed) { } 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 c349ea764101f..9a177e74ab2f0 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 @@ private NodeConfigurationSource createNodeConfigurationSource(final String leade 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