diff --git a/docs/reference/migration/migrate_7_16.asciidoc b/docs/reference/migration/migrate_7_16.asciidoc index 794ca0b9c9548..290a8bb910c86 100644 --- a/docs/reference/migration/migrate_7_16.asciidoc +++ b/docs/reference/migration/migrate_7_16.asciidoc @@ -321,4 +321,77 @@ avoid deprecation warnings and opt into the future behaviour, include the query parameter `?return_200_for_cluster_health_timeout` in your request. ==== +[[script-context-cache-removed]] +.The `script.max_compilations_rate` setting cannot be `use-context`. +[%collapsible] +==== +*Details* + +The `script.max_compilations_rate` node setting cannot be `use-context`. +Previously, you could set this setting to `use-context` to have a +script cache, compilation rate limit settings, cache expiration and +cache size per cluster. + +However, these settings were used to handle system scripts triggering +compilation rate limits and to compensate for an undersized general script +cache. +Now, system scripts are exempt from compilation rate limiting and +the general script cache size is dynamically with a higher default +size. + +Do not use `use-context` as a value for this setting. Instead, remove +the setting to use the default or set it to a custom rate limit, such as +`200/10m`, which allows 200 compilations for every 10 minute period. + +*Impact* + +Discontinue use of the removed setting value. Specifying the setting value +in `elasticsearch.yml` will result in an error on startup. +==== + +[[script-context-cache-max-compile-removed]] +.The `script.context.$CONTEXT.max_compilations_rate` setting has been removed. +[%collapsible] +==== +*Details* + +Script context-specific compilation rate limit settings have been removed. Use +`script.max_compilations_rate` to set the rate limit for all user scripts. + +The context-specific settings were used to handle system scripts triggering +compilation rate limits and to compensate for an undersized general script +cache. + +Now, system scripts are exempt from compilation rate limiting and +the general script cache size is dynamically updatable with a higher default +size. + +*Impact* + +Discontinue use of the removed setting. Specifying the setting in +`elasticsearch.yml` will result in an error on startup. +==== + +[[script-context-cache-size-removed]] +.The `script.context.$CONTEXT.cache_max_size` setting has been removed. +[%collapsible] +==== +*Details* + +Script context-specific cache settings have been removed. Use +`script.cache.max_size` to set the size of the script cache for all scripts. + +*Impact* + +Discontinue use of the removed setting. Specifying the setting in +`elasticsearch.yml` will result in an error on startup. +==== + +[[script-context-cache-expire-removed]] +.The `script.context.$CONTEXT.cache_expire` setting has been removed. +[%collapsible] +==== +*Details* + +Script context-specific cache settings have been removed. Use +`script.cache.expire` to set the expiration of scripts in the script cache +for all scripts. + +*Impact* + +Discontinue use of the removed setting. Specifying the setting in +`elasticsearch.yml` will result in an error on startup. +==== // end::notable-breaking-changes[] diff --git a/docs/reference/modules/indices/circuit_breaker.asciidoc b/docs/reference/modules/indices/circuit_breaker.asciidoc index b128ce5dfb371..161f3a8876f18 100644 --- a/docs/reference/modules/indices/circuit_breaker.asciidoc +++ b/docs/reference/modules/indices/circuit_breaker.asciidoc @@ -126,11 +126,11 @@ within a period of time. See the "prefer-parameters" section of the <> documentation for more information. -`script.context.$CONTEXT.max_compilations_rate`:: +`script.max_compilations_rate`:: (<>) Limit for the number of unique dynamic scripts within a certain interval - that are allowed to be compiled for a given context. Defaults to `75/5m`, - meaning 75 every 5 minutes. + that are allowed to be compiled. Defaults to `150/5m`, + meaning 150 every 5 minutes. [[regex-circuit-breaker]] [discrete] diff --git a/docs/reference/scripting/using.asciidoc b/docs/reference/scripting/using.asciidoc index f49be226c2d37..e1eaeaaedad93 100644 --- a/docs/reference/scripting/using.asciidoc +++ b/docs/reference/scripting/using.asciidoc @@ -120,12 +120,8 @@ the `multiplier` parameter without {es} recompiling the script. } ---- -For most contexts, you can compile up to 75 scripts per 5 minutes by default. -For ingest contexts, the default script compilation rate is unlimited. You -can change these settings dynamically by setting -`script.context.$CONTEXT.max_compilations_rate`. For example, the following -setting limits script compilation to 100 scripts every 10 minutes for the -{painless}/painless-field-context.html[field context]: +You can compile up to 150 scripts per 5 minutes by default. +For ingest contexts, the default script compilation rate is unlimited. [source,js] ---- @@ -406,8 +402,8 @@ small. All scripts are cached by default so that they only need to be recompiled when updates occur. By default, scripts do not have a time-based expiration. -You can change this behavior by using the `script.context.$CONTEXT.cache_expire` setting. -Use the `script.context.$CONTEXT.cache_max_size` setting to configure the size of the cache. +You can change this behavior by using the `script.cache.expire` setting. +Use the `script.cache.max_size` setting to configure the size of the cache. NOTE: The size of scripts is limited to 65,535 bytes. Set the value of `script.max_size_in_bytes` to increase that soft limit. If your scripts are really large, then consider using a diff --git a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java index 14c780c08ba73..73ae88fdebc57 100644 --- a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java +++ b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java @@ -52,7 +52,7 @@ static ScriptContext newContext(String name, Class factoryClass) { * source of runaway script compilations. We think folks will * mostly reuse scripts though. */ - ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), + false, /* * Disable runtime fields scripts from being allowed * to be stored as part of the script meta data. diff --git a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java index 430f6c22a116f..caa6cbbe0164b 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java @@ -21,7 +21,7 @@ public abstract class IngestConditionalScript { /** The context used to compile {@link IngestConditionalScript} factories. */ public static final ScriptContext CONTEXT = new ScriptContext<>("processor_conditional", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); /** The generic runtime parameters for the script. */ private final Map params; diff --git a/server/src/main/java/org/elasticsearch/script/IngestScript.java b/server/src/main/java/org/elasticsearch/script/IngestScript.java index a0fa0d9bbdde8..bb444b132d1d0 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestScript.java @@ -22,7 +22,7 @@ public abstract class IngestScript { /** The context used to compile {@link IngestScript} factories. */ public static final ScriptContext CONTEXT = new ScriptContext<>("ingest", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); /** The generic runtime parameters for the script. */ private final Map params; diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCache.java b/server/src/main/java/org/elasticsearch/script/ScriptCache.java index 6e1ab496105da..b2fcdc9f24572 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptCache.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptCache.java @@ -44,12 +44,7 @@ public class ScriptCache { private final double compilesAllowedPerNano; private final String contextRateSetting; - ScriptCache( - int cacheMaxSize, - TimeValue cacheExpire, - CompilationRate maxCompilationRate, - String contextRateSetting - ) { + ScriptCache(int cacheMaxSize, TimeValue cacheExpire, CompilationRate maxCompilationRate, String contextRateSetting) { this.cacheSize = cacheMaxSize; this.cacheExpire = cacheExpire; this.contextRateSetting = contextRateSetting; @@ -94,8 +89,10 @@ FactoryType compile( logger.trace("context [{}]: compiling script, type: [{}], lang: [{}], options: [{}]", context.name, type, lang, options); } - // Check whether too many compilations have happened - checkCompilationLimit(); + if (context.compilationRateLimited) { + // Check whether too many compilations have happened + checkCompilationLimit(); + } Object compiledScript = scriptEngine.compile(id, idOrCode, context, options); // Since the cache key is the script content itself we don't need to // invalidate/check the cache if an indexed script changes. diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java b/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java index d580f94a65a68..28183c1d46308 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java @@ -21,7 +21,7 @@ import java.util.Objects; import java.util.stream.Collectors; -// This class is deprecated in favor of ScriptStats and ScriptContextStats. It is removed in 8. +// This class is deprecated in favor of ScriptStats and ScriptContextStats public class ScriptCacheStats implements Writeable, ToXContentFragment { private final Map context; private final ScriptStats general; diff --git a/server/src/main/java/org/elasticsearch/script/ScriptContext.java b/server/src/main/java/org/elasticsearch/script/ScriptContext.java index eb158f444096c..1e84d36b08b13 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptContext.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptContext.java @@ -47,6 +47,8 @@ * be {@code boolean needs_score()}. */ public final class ScriptContext { + /** The default compilation rate limit for contexts with compilation rate limiting enabled */ + public static final Tuple DEFAULT_COMPILATION_RATE_LIMIT = new Tuple<>(150, TimeValue.timeValueMinutes(5)); /** A unique identifier for this context. */ public final String name; @@ -66,15 +68,15 @@ public final class ScriptContext { /** The default expiration of a script in the cache for the context, if not overridden */ public final TimeValue cacheExpireDefault; - /** The default max compilation rate for scripts in this context. Script compilation is throttled if this is exceeded */ - public final Tuple maxCompilationRateDefault; + /** Is compilation rate limiting enabled for this context? */ + public final boolean compilationRateLimited; /** Determines if the script can be stored as part of the cluster state. */ public final boolean allowStoredScript; /** Construct a context with the related instance and compiled classes with caller provided cache defaults */ public ScriptContext(String name, Class factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault, - Tuple maxCompilationRateDefault, boolean allowStoredScript) { + boolean compilationRateLimited, boolean allowStoredScript) { this.name = name; this.factoryClazz = factoryClazz; Method newInstanceMethod = findMethod("FactoryType", factoryClazz, "newInstance"); @@ -98,15 +100,15 @@ public ScriptContext(String name, Class factoryClazz, int cacheSize this.cacheSizeDefault = cacheSizeDefault; this.cacheExpireDefault = cacheExpireDefault; - this.maxCompilationRateDefault = maxCompilationRateDefault; + this.compilationRateLimited = compilationRateLimited; this.allowStoredScript = allowStoredScript; } /** Construct a context with the related instance and compiled classes with defaults for cacheSizeDefault, cacheExpireDefault and - * maxCompilationRateDefault and allow scripts of this context to be stored scripts */ + * compilationRateLimited and allow scripts of this context to be stored scripts */ public ScriptContext(String name, Class factoryClazz) { // cache size default, cache expire default, max compilation rate are defaults from ScriptService. - this(name, factoryClazz, 100, TimeValue.timeValueMillis(0), new Tuple<>(75, TimeValue.timeValueMinutes(5)), true); + this(name, factoryClazz, 100, TimeValue.timeValueMillis(0), true, true); } /** Returns a method with the given name, or throws an exception if multiple are found. */ diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index d760978b1d93e..8ee5814f94666 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -23,6 +23,8 @@ import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationCategory; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; @@ -50,6 +52,7 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptCompiler { private static final Logger logger = LogManager.getLogger(ScriptService.class); + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ScriptService.class); static final String DISABLE_DYNAMIC_SCRIPTING_SETTING = "script.disable_dynamic"; @@ -59,16 +62,22 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp static final String USE_CONTEXT_RATE_KEY = "use-context"; public static final Setting SCRIPT_GENERAL_CACHE_SIZE_SETTING = - Setting.intSetting("script.cache.max_size", 100, 0, Property.NodeScope); + Setting.intSetting("script.cache.max_size", 3000, 0, Property.Dynamic, Property.NodeScope); public static final Setting SCRIPT_GENERAL_CACHE_EXPIRE_SETTING = - Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope); + Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.Dynamic, 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_GENERAL_MAX_COMPILATIONS_RATE_SETTING = - new Setting<>("script.max_compilations_rate", USE_CONTEXT_RATE_KEY, + new Setting<>("script.max_compilations_rate", "150/5m", (String value) -> value.equals(USE_CONTEXT_RATE_KEY) ? USE_CONTEXT_RATE_VALUE: new ScriptCache.CompilationRate(value), Property.Dynamic, Property.NodeScope); + public static final String USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE = "[" + USE_CONTEXT_RATE_KEY + "] is deprecated for the setting [" + + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey() + "] as system scripts are now exempt from the rate limit." + + "Set to a value such as [150/5m] (a rate of 150 compilations per five minutes) to rate limit user scripts in case the" + + "script cache [" + SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey() + "] is undersized causing script compilation thrashing."; + + // Per-context settings static final String CONTEXT_PREFIX = "script.context."; @@ -77,13 +86,14 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp 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)); + key -> Setting.intSetting(key, SCRIPT_GENERAL_CACHE_SIZE_SETTING, 0, + Property.NodeScope, Property.Dynamic, Property.Deprecated)); 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)); + Property.NodeScope, Property.Dynamic, Property.Deprecated)); // Unlimited compilation rate for context-specific script caches static final String UNLIMITED_COMPILATION_RATE_KEY = "unlimited"; @@ -94,7 +104,7 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp key -> new Setting(key, "75/5m", (String value) -> value.equals(UNLIMITED_COMPILATION_RATE_KEY) ? ScriptCache.UNLIMITED_COMPILATION_RATE: new ScriptCache.CompilationRate(value), - Property.NodeScope, Property.Dynamic)); + Property.NodeScope, Property.Dynamic, Property.Deprecated)); private static final ScriptCache.CompilationRate SCRIPT_COMPILATION_RATE_ZERO = new ScriptCache.CompilationRate(0, TimeValue.ZERO); @@ -205,6 +215,10 @@ public ScriptService(Settings settings, Map engines, Map setCacheHolder(settings), + this::setCacheHolder, Arrays.asList(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING, SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, SCRIPT_GENERAL_CACHE_SIZE_SETTING, @@ -249,6 +263,10 @@ void registerClusterSettingsListeners(ClusterSettings clusterSettings) { */ void validateCacheSettings(Settings settings) { boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE); + if (useContext) { + deprecationLogger.critical(DeprecationCategory.SCRIPTING, "scripting-context-cache", + USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); + } List> affixes = Arrays.asList(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, SCRIPT_CACHE_EXPIRE_SETTING, SCRIPT_CACHE_SIZE_SETTING); List customRates = new ArrayList<>(); @@ -277,7 +295,7 @@ void validateCacheSettings(Settings settings) { String.join(", ", customRates) + "] if compile rates disabled via [" + SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.getKey() + "]"); } - if (useContext == false) { + if (useContext == false && SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.exists(settings)) { throw new IllegalArgumentException("Cannot set custom general compilation rates [" + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey() + "] to [" + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings) + "] if compile rates disabled via [" + @@ -565,8 +583,10 @@ void setCacheHolder(Settings settings) { } else if (current.general == null) { // Flipping to general cacheHolder.set(generalCacheHolder(settings)); - } else if (current.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 + } else if (current.general.rate.equals(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings)) == false || + current.general.cacheExpire.equals(SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(settings)) == false || + current.general.cacheSize != SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(settings)) { + // General compilation rate, cache expiration or cache size changed cacheHolder.set(generalCacheHolder(settings)); } } @@ -592,13 +612,15 @@ ScriptCache contextCache(Settings settings, ScriptContext context) { Setting rateSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context.name); - ScriptCache.CompilationRate rate = null; - if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings) || compilationLimitsEnabled() == false) { + ScriptCache.CompilationRate rate; + if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings) + || compilationLimitsEnabled() == false + || context.compilationRateLimited == false) { rate = SCRIPT_COMPILATION_RATE_ZERO; } else if (rateSetting.existsOrFallbackExists(settings)) { rate = rateSetting.get(settings); } else { - rate = new ScriptCache.CompilationRate(context.maxCompilationRateDefault); + rate = new ScriptCache.CompilationRate(ScriptContext.DEFAULT_COMPILATION_RATE_LIMIT); } return new ScriptCache(cacheSize, cacheExpire, rate, rateSetting.getKey()); diff --git a/server/src/main/java/org/elasticsearch/script/TemplateScript.java b/server/src/main/java/org/elasticsearch/script/TemplateScript.java index d1777316434cf..7356be0bbdc32 100644 --- a/server/src/main/java/org/elasticsearch/script/TemplateScript.java +++ b/server/src/main/java/org/elasticsearch/script/TemplateScript.java @@ -42,5 +42,5 @@ public interface Factory { // rate limiting. MustacheScriptEngine explicitly checks for TemplateScript. Rather than complicating the implementation there by // creating a new Script class (as would be customary), this context is used to avoid the default rate limit. public static final ScriptContext INGEST_CONTEXT = new ScriptContext<>("ingest_template", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); } diff --git a/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java b/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java index 6dc3cb3db72fc..efcbf07cd3f76 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java @@ -8,14 +8,54 @@ package org.elasticsearch.script; import org.elasticsearch.common.breaker.CircuitBreakingException; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.ESTestCase; +import java.util.stream.Collectors; + 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 { + String context = randomFrom( + ScriptModule.CORE_CONTEXTS.values().stream().filter( + c -> c.compilationRateLimited + ).collect(Collectors.toList()) + ).name; + final TimeValue expire = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY); + final Integer size = ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY); + Setting rateSetting = + ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context); + ScriptCache.CompilationRate rate = + ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY); + String rateSettingName = rateSetting.getKey(); + ScriptCache cache = new ScriptCache(size, expire, + new ScriptCache.CompilationRate(1, TimeValue.timeValueMinutes(1)), rateSettingName); + cache.checkCompilationLimit(); // should pass + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(2, TimeValue.timeValueMinutes(1)), rateSettingName); + cache.checkCompilationLimit(); // should pass + cache.checkCompilationLimit(); // should pass + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + int count = randomIntBetween(5, 50); + cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(count, TimeValue.timeValueMinutes(1)), rateSettingName); + for (int i = 0; i < count; i++) { + cache.checkCompilationLimit(); // should pass + } + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(0, TimeValue.timeValueMinutes(1)), rateSettingName); + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, + new ScriptCache.CompilationRate(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)), rateSettingName); + int largeLimit = randomIntBetween(1000, 10000); + for (int i = 0; i < largeLimit; i++) { + cache.checkCompilationLimit(); + } + } + + public void testGeneralCompilationCircuitBreaking() throws Exception { final TimeValue expire = ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(Settings.EMPTY); final Integer size = ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(Settings.EMPTY); String settingName = ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(); @@ -35,7 +75,7 @@ public void testCompilationCircuitBreaking() throws Exception { cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(0, TimeValue.timeValueMinutes(1)), settingName); expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); cache = new ScriptCache(size, expire, - new ScriptCache.CompilationRate(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)), settingName); + new ScriptCache.CompilationRate(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)), settingName); int largeLimit = randomIntBetween(1000, 10000); for (int i = 0; i < largeLimit; i++) { cache.checkCompilationLimit(); @@ -43,6 +83,25 @@ public void testCompilationCircuitBreaking() throws Exception { } public void testUnlimitedCompilationRate() { + String context = randomFrom( + ScriptModule.CORE_CONTEXTS.values().stream().filter( + c -> c.compilationRateLimited + ).collect(Collectors.toList()) + ).name; + final Integer size = ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY); + final TimeValue expire = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY); + String settingName = ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context).getKey(); + ScriptCache cache = new ScriptCache(size, expire, ScriptCache.UNLIMITED_COMPILATION_RATE, settingName); + ScriptCache.TokenBucketState initialState = cache.tokenBucketState.get(); + for(int i=0; i < 3000; i++) { + cache.checkCompilationLimit(); + ScriptCache.TokenBucketState currentState = cache.tokenBucketState.get(); + assertEquals(initialState.lastInlineCompileTime, currentState.lastInlineCompileTime); + assertEquals(initialState.availableTokens, currentState.availableTokens, 0.0); // delta of 0.0 because it should never change + } + } + + public void testGeneralUnlimitedCompilationRate() { final Integer size = ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(Settings.EMPTY); final TimeValue expire = ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(Settings.EMPTY); String settingName = ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 826340d6e2233..1bd249e3ff38b 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.TimeValue; import org.elasticsearch.xcontent.XContentFactory; @@ -27,17 +28,21 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.BiFunction; import java.util.function.Function; +import java.util.stream.Collectors; import static org.elasticsearch.script.ScriptService.SCRIPT_CACHE_EXPIRE_SETTING; import static org.elasticsearch.script.ScriptService.SCRIPT_CACHE_SIZE_SETTING; import static org.elasticsearch.script.ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING; -import static org.elasticsearch.script.ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING; import static org.elasticsearch.script.ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING; +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.elasticsearch.script.ScriptService.USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE; +import static org.elasticsearch.script.ScriptService.USE_CONTEXT_RATE_KEY; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -51,6 +56,7 @@ public class ScriptServiceTests extends ESTestCase { private ScriptService scriptService; private Settings baseSettings; private ClusterSettings clusterSettings; + private Map> rateLimitedContexts; @Before public void setup() throws IOException { @@ -69,6 +75,7 @@ public void setup() throws IOException { engines.put(scriptEngine.getType(), scriptEngine); engines.put("test", new MockScriptEngine("test", scripts, Collections.emptyMap())); logger.info("--> setup script service"); + rateLimitedContexts = compilationRateLimitedContexts(); } private void buildScriptService(Settings additionalSettings) throws IOException { @@ -215,7 +222,35 @@ public void testMultipleCompilationsCountedInCompilationStats() throws IOExcepti assertEquals(numberOfCompilations, scriptService.stats().getCompilations()); } - public void testCompilationStatsOnCacheHit() throws IOException { + public void testCompilationGeneralStatsOnCacheHit() throws IOException { + buildScriptService(Settings.EMPTY); + Script script = new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()); + ScriptContext context = randomFrom(contexts.values()); + scriptService.compile(script, context); + scriptService.compile(script, context); + assertEquals(1L, scriptService.stats().getCompilations()); + } + + public void testIndexedScriptCountedInGeneralCompilationStats() throws IOException { + buildScriptService(Settings.EMPTY); + ScriptContext ctx = randomFrom(contexts.values()); + scriptService.compile(new Script(ScriptType.STORED, null, "script", Collections.emptyMap()), ctx); + assertEquals(1L, scriptService.stats().getCompilations()); + } + + public void testContextCompilationStatsOnCacheHit() throws IOException { + buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .build()); + Script script = new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()); + ScriptContext context = randomFrom(contexts.values()); + scriptService.compile(script, context); + scriptService.compile(script, context); + assertEquals(1L, scriptService.stats().getCompilations()); + assertWarnings(USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); + } + + public void testGeneralCompilationStatsOnCacheHit() throws IOException { Settings.Builder builder = Settings.builder() .put(SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), 1) .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "2/1m"); @@ -227,15 +262,40 @@ public void testCompilationStatsOnCacheHit() throws IOException { assertEquals(1L, scriptService.stats().getCompilations()); } - public void testIndexedScriptCountedInCompilationStats() throws IOException { + public void testGeneralIndexedScriptCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); ScriptContext ctx = randomFrom(contexts.values()); scriptService.compile(new Script(ScriptType.STORED, null, "script", Collections.emptyMap()), ctx); assertEquals(1L, scriptService.stats().getCompilations()); + } + + public void testContextIndexedScriptCountedInCompilationStats() throws IOException { + buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .build()); + ScriptContext ctx = randomFrom(contexts.values()); + scriptService.compile(new Script(ScriptType.STORED, null, "script", Collections.emptyMap()), ctx); + assertEquals(1L, scriptService.stats().getCompilations()); assertEquals(1L, scriptService.cacheStats().getContextStats().get(ctx.name).getCompilations()); + assertWarnings(USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); } public void testCacheEvictionCountedInCacheEvictionsStats() throws IOException { + ScriptContext context = randomFrom(contexts.values()); + Setting contextCacheSizeSetting = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context.name); + buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(contextCacheSizeSetting.getKey(), 1) + .build() + ); + scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), context); + scriptService.compile(new Script(ScriptType.INLINE, "test", "2+2", Collections.emptyMap()), context); + assertEquals(2L, scriptService.stats().getCompilations()); + assertEquals(1L, scriptService.stats().getCacheEvictions()); + assertSettingDeprecationsAndWarnings(new Setting[]{contextCacheSizeSetting}, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); + } + + public void testGeneralCacheEvictionCountedInCacheEvictionsStats() throws IOException { Settings.Builder builder = Settings.builder(); builder.put(SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), 1); builder.put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "10/1m"); @@ -249,20 +309,27 @@ public void testCacheEvictionCountedInCacheEvictionsStats() throws IOException { } public void testContextCacheStats() throws IOException { - ScriptContext contextA = randomFrom(contexts.values()); + ScriptContext contextA = randomFrom(rateLimitedContexts.values()); String aRate = "2/10m"; - ScriptContext contextB = randomValueOtherThan(contextA, () -> randomFrom(contexts.values())); + ScriptContext contextB = randomValueOtherThan(contextA, () -> randomFrom(rateLimitedContexts.values())); String bRate = "3/10m"; BiFunction msg = (rate, ctx) -> ( "[script] Too many dynamic script compilations within, max: [" + rate + "]; please use indexed, or scripts with parameters instead; this limit can be changed by the [script.context." + ctx + ".max_compilations_rate] setting" ); + Setting cacheSizeA = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextA.name); + Setting compilationRateA = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextA.name); + + Setting cacheSizeB = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextB.name); + Setting compilationRateB = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextB.name); + buildScriptService(Settings.builder() - .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextA.name).getKey(), 1) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextA.name).getKey(), aRate) - .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextB.name).getKey(), 2) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextB.name).getKey(), bRate) + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(cacheSizeA.getKey(), 1) + .put(compilationRateA.getKey(), aRate) + .put(cacheSizeB.getKey(), 2) + .put(compilationRateB.getKey(), bRate) .build()); // Context A @@ -286,20 +353,29 @@ public void testContextCacheStats() throws IOException { assertEquals(CircuitBreakingException.class, gse.getRootCause().getClass()); // Context specific - ScriptCacheStats stats = scriptService.cacheStats(); - assertEquals(2L, stats.getContextStats().get(contextA.name).getCompilations()); - assertEquals(1L, stats.getContextStats().get(contextA.name).getCacheEvictions()); - assertEquals(1L, stats.getContextStats().get(contextA.name).getCompilationLimitTriggered()); + ScriptStats stats = scriptService.stats(); + assertEquals(2L, getByContext(stats, contextA.name).getCompilations()); + assertEquals(1L, getByContext(stats, contextA.name).getCacheEvictions()); + assertEquals(1L, getByContext(stats, contextA.name).getCompilationLimitTriggered()); - assertEquals(3L, stats.getContextStats().get(contextB.name).getCompilations()); - assertEquals(1L, stats.getContextStats().get(contextB.name).getCacheEvictions()); - assertEquals(2L, stats.getContextStats().get(contextB.name).getCompilationLimitTriggered()); - assertNull(scriptService.cacheStats().getGeneralStats()); + assertEquals(3L, getByContext(stats, contextB.name).getCompilations()); + assertEquals(1L, getByContext(stats, contextB.name).getCacheEvictions()); + assertEquals(2L, getByContext(stats, contextB.name).getCompilationLimitTriggered()); // Summed up assertEquals(5L, scriptService.stats().getCompilations()); assertEquals(2L, scriptService.stats().getCacheEvictions()); assertEquals(3L, scriptService.stats().getCompilationLimitTriggered()); + + assertSettingDeprecationsAndWarnings(new Setting[]{cacheSizeA, compilationRateA, cacheSizeB, compilationRateB}, + USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); + } + + private ScriptContextStats getByContext(ScriptStats stats, String context) { + List maybeContextStats = stats.getContextStats().stream().filter(c -> c.getContext().equals(context)) + .collect(Collectors.toList()); + assertEquals(1, maybeContextStats.size()); + return maybeContextStats.get(0); } public void testStoreScript() throws Exception { @@ -397,12 +473,19 @@ public void testConflictContextSettings() throws IOException { illegal.getMessage() ); + Setting ingestExpire = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("ingest"); + Setting fieldSize = ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("field"); + Setting scoreCompilation = ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("score"); + 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(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(ingestExpire.getKey(), "5m") + .put(fieldSize.getKey(), 123) + .put(scoreCompilation.getKey(), "50/5m") .build()); + assertSettingDeprecationsAndWarnings(new Setting[]{ingestExpire, fieldSize, scoreCompilation}, + USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); } public void testFallbackContextSettings() { @@ -414,11 +497,13 @@ public void testFallbackContextSettings() { String cacheExpireFoo = randomValueOtherThan(cacheExpireBackup, () -> randomTimeValue(1, 1000, "h")); TimeValue cacheExpireFooParsed = TimeValue.parseTimeValue(cacheExpireFoo, ""); + Setting cacheSizeSetting = ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo"); + Setting cacheExpireSetting = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo"); Settings s = Settings.builder() .put(SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), cacheSizeBackup) - .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo").getKey(), cacheSizeFoo) + .put(cacheSizeSetting.getKey(), cacheSizeFoo) .put(SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), cacheExpireBackup) - .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo").getKey(), cacheExpireFoo) + .put(cacheExpireSetting.getKey(), cacheExpireFoo) .build(); assertEquals(cacheSizeFoo, ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo").get(s).intValue()); @@ -426,12 +511,14 @@ public void testFallbackContextSettings() { assertEquals(cacheExpireFooParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo").get(s)); assertEquals(cacheExpireBackupParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("bar").get(s)); + assertSettingDeprecationsAndWarnings(new Setting[]{cacheExpireSetting, cacheExpireSetting}); } public void testUseContextSettingValue() { + Setting contextMaxCompilationRate = ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("foo"); 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(), + .put(contextMaxCompilationRate.getKey(), ScriptService.USE_CONTEXT_RATE_KEY) .build(); @@ -442,6 +529,7 @@ public void testUseContextSettingValue() { }); assertEquals("parameter must contain a positive integer and a timevalue, i.e. 10/1m, but was [use-context]", illegal.getMessage()); + assertSettingDeprecationsAndWarnings(new Setting[]{contextMaxCompilationRate}); } public void testCacheHolderGeneralConstructor() throws IOException { @@ -458,14 +546,17 @@ public void testCacheHolderGeneralConstructor() throws IOException { } public void testCacheHolderContextConstructor() throws IOException { - String a = randomFrom(contexts.keySet()); - String b = randomValueOtherThan(a, () -> randomFrom(contexts.keySet())); + String a = randomFrom(rateLimitedContexts.keySet()); + String b = randomValueOtherThan(a, () -> randomFrom(rateLimitedContexts.keySet())); String aCompilationRate = "77/5m"; String bCompilationRate = "78/6m"; + Setting aSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a); + Setting bSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b); buildScriptService(Settings.builder() - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a).getKey(), aCompilationRate) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b).getKey(), bCompilationRate) + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(aSetting.getKey(), aCompilationRate) + .put(bSetting.getKey(), bCompilationRate) .build()); assertNull(scriptService.cacheHolder.get().general); @@ -476,6 +567,7 @@ public void testCacheHolderContextConstructor() throws IOException { scriptService.cacheHolder.get().contextCache.get(a).get().rate); assertEquals(new ScriptCache.CompilationRate(bCompilationRate), scriptService.cacheHolder.get().contextCache.get(b).get().rate); + assertSettingDeprecationsAndWarnings(new Setting[]{aSetting, bSetting}, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); } public void testCompilationRateUnlimitedContextOnly() throws IOException { @@ -486,19 +578,22 @@ public void testCompilationRateUnlimitedContextOnly() throws IOException { }); assertEquals("parameter must contain a positive integer and a timevalue, i.e. 10/1m, but was [unlimited]", illegal.getMessage()); + + Setting field = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("field"); + Setting ingest = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("ingest"); // Should not throw. buildScriptService(Settings.builder() - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("ingest").getKey(), - ScriptService.UNLIMITED_COMPILATION_RATE_KEY) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("field").getKey(), - ScriptService.UNLIMITED_COMPILATION_RATE_KEY) + .put(field.getKey(), ScriptService.UNLIMITED_COMPILATION_RATE_KEY) + .put(ingest.getKey(), ScriptService.UNLIMITED_COMPILATION_RATE_KEY) .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY) .build()); + assertSettingDeprecationsAndWarnings(new Setting[]{field, ingest}, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); } public void testDisableCompilationRateSetting() throws IOException { IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> { buildScriptService(Settings.builder() + .put("script.max_compilations_rate", "use-context") .put("script.context.ingest.max_compilations_rate", "76/10m") .put("script.context.field.max_compilations_rate", "77/10m") .put("script.disable_max_compilations_rate", true) @@ -522,10 +617,14 @@ public void testDisableCompilationRateSetting() throws IOException { buildScriptService(Settings.builder() .put("script.disable_max_compilations_rate", true) .build()); + assertSettingDeprecationsAndWarnings(new Setting[]{ + SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("field"), + SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("ingest")}, + USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); } public void testCacheHolderChangeSettings() throws IOException { - Set contextNames = contexts.keySet(); + Set contextNames = rateLimitedContexts.keySet(); String a = randomFrom(contextNames); String aRate = "77/5m"; String b = randomValueOtherThan(a, () -> randomFrom(contextNames)); @@ -546,11 +645,15 @@ public void testCacheHolderChangeSettings() throws IOException { assertNull(scriptService.cacheHolder.get().contextCache); assertEquals(generalRate, scriptService.cacheHolder.get().general.rate); + Setting compilationRateA = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a); + Setting compilationRateB = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b); + Setting compilationRateC = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(c); + scriptService.setCacheHolder(Settings.builder() - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a).getKey(), aRate) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b).getKey(), bRate) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(c).getKey(), - ScriptService.UNLIMITED_COMPILATION_RATE_KEY) + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(compilationRateA.getKey(), aRate) + .put(compilationRateB.getKey(), bRate) + .put(compilationRateC.getKey(), ScriptService.UNLIMITED_COMPILATION_RATE_KEY) .build() ); @@ -593,6 +696,8 @@ public void testCacheHolderChangeSettings() throws IOException { Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), bRate).build() ); assertEquals(holder, scriptService.cacheHolder.get()); + + assertSettingDeprecationsAndWarnings(new Setting[]{compilationRateA, compilationRateB, compilationRateC}); } public void testFallbackToContextDefaults() throws IOException { @@ -605,13 +710,17 @@ public void testFallbackToContextDefaults() throws IOException { Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "75/5m").build() ); - String name = "ingest"; + String name = "score"; + Setting cacheSizeContextSetting = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(name); + Setting cacheExpireContextSetting = SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(name); + Setting compilationRateContextSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(name); // Use context specific scriptService.setCacheHolder(Settings.builder() - .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextCacheSize) - .put(SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextExpire) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextRateStr) + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(cacheSizeContextSetting.getKey(), contextCacheSize) + .put(cacheExpireContextSetting.getKey(), contextExpire) + .put(compilationRateContextSetting.getKey(), contextRateStr) .build() ); @@ -624,18 +733,31 @@ public void testFallbackToContextDefaults() throws IOException { assertEquals(contextCacheSize, holder.contextCache.get(name).get().cacheSize); assertEquals(contextExpire, holder.contextCache.get(name).get().cacheExpire); - ScriptContext ingest = contexts.get(name); + ScriptContext score = contexts.get(name); // Fallback to context defaults - buildScriptService(Settings.EMPTY); + buildScriptService(Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY).build()); holder = scriptService.cacheHolder.get(); assertNotNull(holder.contextCache); assertNotNull(holder.contextCache.get(name)); assertNotNull(holder.contextCache.get(name).get()); - assertEquals(ingest.maxCompilationRateDefault, holder.contextCache.get(name).get().rate.asTuple()); - assertEquals(ingest.cacheSizeDefault, holder.contextCache.get(name).get().cacheSize); - assertEquals(ingest.cacheExpireDefault, holder.contextCache.get(name).get().cacheExpire); + assertEquals(ScriptContext.DEFAULT_COMPILATION_RATE_LIMIT, holder.contextCache.get(name).get().rate.asTuple()); + assertEquals(score.cacheSizeDefault, holder.contextCache.get(name).get().cacheSize); + assertEquals(score.cacheExpireDefault, holder.contextCache.get(name).get().cacheExpire); + + assertSettingDeprecationsAndWarnings(new Setting[]{cacheSizeContextSetting, cacheExpireContextSetting, + compilationRateContextSetting}, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); + } + + protected HashMap> compilationRateLimitedContexts() { + HashMap> rateLimited = new HashMap<>(); + for (Map.Entry> entry: contexts.entrySet()) { + if (entry.getValue().compilationRateLimited) { + rateLimited.put(entry.getKey(), entry.getValue()); + } + } + return rateLimited; } private void assertCompileRejected(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) { diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java index 28a219006d102..cebdbb1b76f2d 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java @@ -130,7 +130,11 @@ private DeprecationChecks() { NodeDeprecationChecks::checkRolesCacheTTLSizeSetting, NodeDeprecationChecks::checkMaxLocalStorageNodesSetting, NodeDeprecationChecks::checkSamlNameIdFormatSetting, - NodeDeprecationChecks::checkClusterRoutingAllocationIncludeRelocationsSetting + NodeDeprecationChecks::checkClusterRoutingAllocationIncludeRelocationsSetting, + NodeDeprecationChecks::checkScriptContextCache, + NodeDeprecationChecks::checkScriptContextCompilationsRateLimitSetting, + NodeDeprecationChecks::checkScriptContextCacheSizeSetting, + NodeDeprecationChecks::checkScriptContextCacheExpirationSetting ) ).collect(Collectors.toList()); } diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java index 6579574fa7b9a..66d6091f73bd2 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java @@ -35,6 +35,7 @@ import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.node.Node; import org.elasticsearch.node.NodeRoleSettings; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.transport.RemoteClusterService; import org.elasticsearch.transport.SniffConnectionStrategy; @@ -987,4 +988,90 @@ static DeprecationIssue checkSamlNameIdFormatSetting(final Settings settings, return new DeprecationIssue(DeprecationIssue.Level.WARNING, message, url, details, false, null); } } + + static DeprecationIssue checkScriptContextCache(final Settings settings, + final PluginsAndModules pluginsAndModules, + final ClusterState clusterState, + final XPackLicenseState licenseState) { + if (ScriptService.isUseContextCacheSet(settings)) { + return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + ScriptService.USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE, + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-removed", + "found script context caches in use, change setting to compilation rate or remove setting to use the default.", + false, null); + } + return null; + } + + static DeprecationIssue checkScriptContextCompilationsRateLimitSetting(final Settings settings, + final PluginsAndModules pluginsAndModules, + final ClusterState clusterState, + final XPackLicenseState licenseState) { + Setting.AffixSetting maxSetting = ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING; + Set contextCompilationRates = maxSetting.getAsMap(settings).keySet(); + if (contextCompilationRates.isEmpty() == false) { + String maxSettings = contextCompilationRates.stream().sorted().map( + c -> maxSetting.getConcreteSettingForNamespace(c).getKey() + ).collect(Collectors.joining(",")); + return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + String.format(Locale.ROOT, "settings [" + maxSettings + "] for context specific rate limits are deprecated and will" + + " not be available in a future version." + + " Use [" + ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey() + "] to rate limit the compilation" + + " of user scripts, system scripts are exempt.", maxSettings), + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-max-compile-removed", + String.format(Locale.ROOT, "found [%s] settings. Discontinue use of these settings.", maxSettings), + false, null); + } + return null; + } + + static DeprecationIssue checkScriptContextCacheSizeSetting(final Settings settings, + final PluginsAndModules pluginsAndModules, + final ClusterState clusterState, + final XPackLicenseState licenseState) { + Setting.AffixSetting cacheSizeSetting = ScriptService.SCRIPT_CACHE_SIZE_SETTING; + Set contextCacheSizes = cacheSizeSetting.getAsMap(settings).keySet(); + if (contextCacheSizes.isEmpty() == false) { + String cacheSizeSettings = contextCacheSizes.stream().sorted().map( + c -> cacheSizeSetting.getConcreteSettingForNamespace(c).getKey() + ).collect(Collectors.joining(",")); + return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + String.format(Locale.ROOT, "settings [" + cacheSizeSettings + "] for context specific script cache sizes are" + + " deprecated and will not be available in a future version." + + " Use [" + ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey() + "] to size the context cache for all" + + " scripts.", + cacheSizeSettings), + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-size-removed", + String.format(Locale.ROOT, "found [%s] settings. Discontinue use of these settings.", cacheSizeSettings), + false, null); + } + return null; + } + + static DeprecationIssue checkScriptContextCacheExpirationSetting(final Settings settings, + final PluginsAndModules pluginsAndModules, + final ClusterState clusterState, + final XPackLicenseState licenseState) { + Setting.AffixSetting cacheExpireSetting = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING; + Set contextCacheExpires = cacheExpireSetting.getAsMap(settings).keySet(); + if (contextCacheExpires.isEmpty() == false) { + String cacheExpireSettings = contextCacheExpires.stream().sorted().map( + c -> cacheExpireSetting.getConcreteSettingForNamespace(c).getKey() + ).collect(Collectors.joining(",")); + return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + String.format(Locale.ROOT, "settings [" + cacheExpireSettings + "] for context specific script cache expiration are" + + " deprecated and will not be available in a future version." + + " Use [" + ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey() + "]" + + " to set the cache expiration all scripts.", + cacheExpireSettings), + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-expire-removed", + String.format(Locale.ROOT, "found [%s] settings. Discontinue use of these settings.", cacheExpireSettings), + false, null); + } + return null; + } } diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java index d799f2637db88..f20c8a92e4b08 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.license.License; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.node.Node; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.transport.RemoteClusterService; import org.elasticsearch.xpack.core.deprecation.DeprecationIssue; @@ -1448,4 +1449,105 @@ public void testCheckSamlNameIdFormatSetting() { equalTo(expectedIssue) ); } + + public void testScriptContextCacheSetting() { + Settings settings = Settings.builder() + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "use-context") + .build(); + + assertThat( + NodeDeprecationChecks.checkScriptContextCache(settings, null, null, null), + equalTo( + new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + ScriptService.USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE, + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-removed", + "found script context caches in use, change setting to compilation rate or remove setting to use the default.", + false, null)) + ); + } + + public void testScriptContextCompilationsRateLimitSetting() { + List contexts = org.elasticsearch.core.List.of("field", "score"); + Settings settings = Settings.builder() + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "use-context") + .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contexts.get(0)).getKey(), "123/5m") + .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contexts.get(1)).getKey(), "456/7m") + .build(); + + assertThat( + NodeDeprecationChecks.checkScriptContextCompilationsRateLimitSetting(settings, null, null, null), + equalTo( + new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "settings [script.context.field.max_compilations_rate,script.context.score.max_compilations_rate] for context" + + " specific rate limits are deprecated and will not be available in a future version." + + " Use [script.max_compilations_rate] to rate limit the compilation" + + " of user scripts, system scripts are exempt.", + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-max-compile-removed", + "found [script.context.field.max_compilations_rate,script.context.score.max_compilations_rate] settings." + + " Discontinue use of these settings.", + false, null))); + + assertWarnings( + "[script.context.field.max_compilations_rate] setting was deprecated in Elasticsearch and will be" + + " removed in a future release! See the breaking changes documentation for the next major version.", + "[script.context.score.max_compilations_rate] setting was deprecated in Elasticsearch and will be removed in a future" + + " release! See the breaking changes documentation for the next major version."); + } + + public void testScriptContextCacheSizeSetting() { + List contexts = org.elasticsearch.core.List.of("filter", "update"); + Settings settings = Settings.builder() + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "use-context") + .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contexts.get(0)).getKey(), 80) + .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contexts.get(1)).getKey(), 200) + .build(); + + assertThat( + NodeDeprecationChecks.checkScriptContextCacheSizeSetting(settings, null, null, null), + equalTo( + new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "settings [script.context.filter.cache_max_size,script.context.update.cache_max_size] for context" + + " specific script cache sizes are deprecated and will not be available in a future version." + + " Use [script.cache.max_size] to size the context cache for all scripts.", + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-size-removed", + "found [script.context.filter.cache_max_size,script.context.update.cache_max_size] settings." + + " Discontinue use of these settings.", + false, null))); + + assertWarnings("[script.context.update.cache_max_size] setting was deprecated in Elasticsearch and will be" + + " removed in a future release! See the breaking changes documentation for the next major version.", + "[script.context.filter.cache_max_size] setting was deprecated in Elasticsearch and will be removed in a future" + + " release! See the breaking changes documentation for the next major version."); + } + + public void testScriptContextCacheExpirationSetting() { + List contexts = org.elasticsearch.core.List.of("interval", "moving-function"); + Settings settings = Settings.builder() + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "use-context") + .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(contexts.get(0)).getKey(), "100m") + .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(contexts.get(1)).getKey(), "2d") + .build(); + + assertThat( + NodeDeprecationChecks.checkScriptContextCacheExpirationSetting(settings, null, null, null), + equalTo( + new DeprecationIssue(DeprecationIssue.Level.CRITICAL, + "settings [script.context.interval.cache_expire,script.context.moving-function.cache_expire] for context" + + " specific script cache expiration are deprecated and will not be available in a future version." + + " Use [script.cache.expire] to set the cache expiration all scripts.", + "https://www.elastic.co/guide/en/elasticsearch/reference/7.16/" + + "breaking-changes-7.16.html#deprecate-script-context-cache-expire-removed", + "found [script.context.interval.cache_expire,script.context.moving-function.cache_expire] settings." + + " Discontinue use of these settings.", + false, null))); + + + assertWarnings("[script.context.interval.cache_expire] setting was deprecated in Elasticsearch and will be" + + " removed in a future release! See the breaking changes documentation for the next major version.", + "[script.context.moving-function.cache_expire] setting was deprecated in Elasticsearch and will be removed in a future" + + " release! See the breaking changes documentation for the next major version."); + } } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java index 6dabe0fd2ddf4..badfb05261c1d 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watcher.java @@ -53,7 +53,6 @@ import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; -import org.elasticsearch.script.ScriptCache; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.TemplateScript; @@ -237,7 +236,7 @@ public class Watcher extends Plugin implements SystemIndexPlugin, ScriptPlugin, public static final ScriptContext SCRIPT_TEMPLATE_CONTEXT = new ScriptContext<>("xpack_template", TemplateScript.Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); private static final Logger logger = LogManager.getLogger(Watcher.class); private WatcherIndexingListener listener; diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java index bc38efd0c6572..04d4b97a8508c 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.watcher.condition; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.script.ScriptCache; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.watcher.support.Variables; @@ -45,5 +44,5 @@ public interface Factory { } public static ScriptContext CONTEXT = new ScriptContext<>("watcher_condition", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java index 465f87cc26bd8..3a48e25635f1d 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.watcher.transform.script; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.script.ScriptCache; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.core.watcher.watch.Payload; @@ -46,5 +45,5 @@ public interface Factory { } public static ScriptContext CONTEXT = new ScriptContext<>("watcher_transform", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); }