From 02119d7061c0b6efdd872a3d2dce050f31ae3d95 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Mon, 18 Oct 2021 20:57:46 -0500 Subject: [PATCH 1/6] Script: Restore the scripting general cache Restore the scripting general cache in preparation for deprecation of the context cache, then removal of the context cache in master. Brings back the general cache settings: * `script.max_compilations_rate` * `script.cache.expire` * `script.cache.max_size` `script.cache.max_size` and `script.cache.expire` are now dynamic settings. The context cache is still default, the switch to general cache as default will happen on context cache deprecation. System script contexts can now opt-out of compilation rate limiting using a flag rather than a sentinel rate limit value. Duplicated defaults for system contexts have been coalesced. Other than that, this code is the same as what was in 7.x to make a `master`-first commit and backport strategy easy. Refs: #62899 Backport: #79414 --- .../script/AbstractFieldScript.java | 44 +++++-------- .../script/IngestConditionalScript.java | 5 +- .../elasticsearch/script/IngestScript.java | 5 +- .../org/elasticsearch/script/ScriptCache.java | 13 ++-- .../script/ScriptCacheStats.java | 2 +- .../elasticsearch/script/ScriptContext.java | 24 ++++++-- .../elasticsearch/script/ScriptService.java | 15 +++-- .../org/elasticsearch/script/ScriptStats.java | 7 +++ .../elasticsearch/script/TemplateScript.java | 5 +- .../script/ScriptCacheTests.java | 61 ++++++++++++++++++- .../script/ScriptServiceTests.java | 35 ++++++++--- .../script/ScriptStatsTests.java | 9 +++ .../elasticsearch/xpack/watcher/Watcher.java | 4 +- .../condition/WatcherConditionScript.java | 5 +- .../script/WatcherTransformScript.java | 5 +- 15 files changed, 155 insertions(+), 84 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java index 14c780c08ba73..f2b1f2a6ac7e0 100644 --- a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java +++ b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java @@ -19,8 +19,6 @@ import java.util.Map; import java.util.function.Function; -import static org.elasticsearch.core.TimeValue.timeValueMillis; - /** * Abstract base for scripts to execute to build scripted fields. Inspired by * {@link AggregationScript} but hopefully with less historical baggage. @@ -32,33 +30,21 @@ public abstract class AbstractFieldScript extends DocBasedScript { public static final int MAX_VALUES = 100; static ScriptContext newContext(String name, Class factoryClass) { - return new ScriptContext<>( - name, - factoryClass, - /* - * We rely on the script cache in two ways: - * 1. It caches the "heavy" part of mappings generated at runtime. - * 2. Mapping updates tend to try to compile the script twice. Not - * for any good reason. They just do. - * Thus we use the default 100. - */ - 100, - timeValueMillis(0), - /* - * Disable compilation rate limits for runtime fields so we - * don't prevent mapping updates because we've performed too - * many recently. That'd just be lame. We also compile these - * scripts during search requests so this could totally be a - * source of runaway script compilations. We think folks will - * mostly reuse scripts though. - */ - ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), - /* - * Disable runtime fields scripts from being allowed - * to be stored as part of the script meta data. - */ - false - ); + /* + * We rely on the script cache in two ways: + * 1. It caches the "heavy" part of mappings generated at runtime. + * 2. Mapping updates tend to try to compile the script twice. Not + * for any good reason. They just do. + * Thus we use the default cache size. + * + * We also disable compilation rate limits for runtime fields so we + * don't prevent mapping updates because we've performed too + * many recently. That'd just be lame. We also compile these + * scripts during search requests so this could totally be a + * source of runaway script compilations. We think folks will + * mostly reuse scripts though. + */ + return new ScriptContext<>(name, factoryClass, false); } private static final Map> PARAMS_FUNCTIONS = org.elasticsearch.core.Map.of( diff --git a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java index 430f6c22a116f..424f30e35973f 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java @@ -8,8 +8,6 @@ package org.elasticsearch.script; -import org.elasticsearch.core.TimeValue; - import java.util.Map; /** @@ -20,8 +18,7 @@ public abstract class IngestConditionalScript { public static final String[] PARAMETERS = { "ctx" }; /** 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); + public static final ScriptContext CONTEXT = new ScriptContext<>("processor_conditional", Factory.class, 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..4e9a7aa35a3a7 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestScript.java @@ -9,8 +9,6 @@ package org.elasticsearch.script; -import org.elasticsearch.core.TimeValue; - import java.util.Map; /** @@ -21,8 +19,7 @@ public abstract class IngestScript { public static final String[] PARAMETERS = { "ctx" }; /** 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); + public static final ScriptContext CONTEXT = new ScriptContext<>("ingest", Factory.class, 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..aafee11d82b07 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) { + ScriptContext(String name, Class factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault, + boolean compilationRateLimited, boolean allowStoredScript) { this.name = name; this.factoryClazz = factoryClazz; Method newInstanceMethod = findMethod("FactoryType", factoryClazz, "newInstance"); @@ -98,7 +100,7 @@ public ScriptContext(String name, Class factoryClazz, int cacheSize this.cacheSizeDefault = cacheSizeDefault; this.cacheExpireDefault = cacheExpireDefault; - this.maxCompilationRateDefault = maxCompilationRateDefault; + this.compilationRateLimited = compilationRateLimited; this.allowStoredScript = allowStoredScript; } @@ -106,7 +108,17 @@ public ScriptContext(String name, Class factoryClazz, int cacheSize * maxCompilationRateDefault 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); + } + + /** + * Construct a context for a system script usage, using the: + * - default cache size (only relevant when context caching enabled): 200 + * - default cache expiration: no expiration + * - unlimited compilation rate + */ + public ScriptContext(String name, Class factoryClazz, boolean allowStoredScript) { + this(name, factoryClazz, 200, TimeValue.timeValueMillis(0), false, allowStoredScript); } /** 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..b79c89150656a 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -231,7 +231,7 @@ void registerClusterSettingsListeners(ClusterSettings clusterSettings) { // Handle all settings for context and general caches, this flips between general and context caches. clusterSettings.addSettingsUpdateConsumer( - (settings) -> setCacheHolder(settings), + this::setCacheHolder, Arrays.asList(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING, SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, SCRIPT_GENERAL_CACHE_SIZE_SETTING, @@ -565,8 +565,9 @@ 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)) { cacheHolder.set(generalCacheHolder(settings)); } } @@ -592,13 +593,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/ScriptStats.java b/server/src/main/java/org/elasticsearch/script/ScriptStats.java index 832d44a46771e..b360a669052aa 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptStats.java @@ -109,6 +109,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(Fields.COMPILATIONS, compilations); builder.field(Fields.CACHE_EVICTIONS, cacheEvictions); builder.field(Fields.COMPILATION_LIMIT_TRIGGERED, compilationLimitTriggered); + /* TODO(stu): master only + builder.startArray(Fields.CONTEXTS); + for (ScriptContextStats contextStats: contextStats) { + contextStats.toXContent(builder, params); + } + builder.endArray(); + */ builder.endObject(); return builder; } diff --git a/server/src/main/java/org/elasticsearch/script/TemplateScript.java b/server/src/main/java/org/elasticsearch/script/TemplateScript.java index d1777316434cf..ea7699ff3c883 100644 --- a/server/src/main/java/org/elasticsearch/script/TemplateScript.java +++ b/server/src/main/java/org/elasticsearch/script/TemplateScript.java @@ -8,8 +8,6 @@ package org.elasticsearch.script; -import org.elasticsearch.core.TimeValue; - import java.util.Map; /** @@ -41,6 +39,5 @@ public interface Factory { // Remove compilation rate limit for ingest. Ingest pipelines may use many mustache templates, triggering compilation // 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); + public static final ScriptContext INGEST_CONTEXT = new ScriptContext<>("ingest_template", Factory.class, 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..a201a65390fc8 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -38,6 +38,7 @@ 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_MAX_COMPILATIONS_RATE_SETTING; +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 +52,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 +71,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 { @@ -249,9 +252,9 @@ 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 + @@ -259,6 +262,7 @@ public void testContextCacheStats() throws IOException { ".max_compilations_rate] setting" ); buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) .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) @@ -458,8 +462,8 @@ 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"; @@ -525,7 +529,8 @@ public void testDisableCompilationRateSetting() throws IOException { } public void testCacheHolderChangeSettings() throws IOException { - Set contextNames = contexts.keySet(); + Set contextNames = contexts.entrySet().stream().filter(e -> e.getValue().compilationRateLimited) + .map(Map.Entry::getKey).collect(Collectors.toSet()); String a = randomFrom(contextNames); String aRate = "77/5m"; String b = randomValueOtherThan(a, () -> randomFrom(contextNames)); @@ -605,7 +610,7 @@ public void testFallbackToContextDefaults() throws IOException { Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "75/5m").build() ); - String name = "ingest"; + String name = "score"; // Use context specific scriptService.setCacheHolder(Settings.builder() @@ -624,7 +629,7 @@ 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); @@ -633,9 +638,19 @@ public void testFallbackToContextDefaults() throws IOException { 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); + } + + 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/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java b/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java index adbcc2df1ece5..d7d5614f446b9 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java @@ -40,6 +40,15 @@ public void testXContent() throws IOException { stats.toXContent(builder, ToXContent.EMPTY_PARAMS); builder.endObject(); + /* TODO(stu): master only + String expected = "{\n" + + " \"script\" : {\n" + + " \"compilations\" : 1100,\n" + + " \"cache_evictions\" : 2211,\n" + + " \"compilation_limit_triggered\" : 3322\n" + + " }\n" + + "}"; + */ String expected = "{\n" + " \"script\" : {\n" + " \"compilations\" : 1100,\n" + 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..00d722ab3cc34 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; @@ -236,8 +235,7 @@ public class Watcher extends Plugin implements SystemIndexPlugin, ScriptPlugin, new ByteSizeValue(1, ByteSizeUnit.MB), new ByteSizeValue(10, ByteSizeUnit.MB), NodeScope); public static final ScriptContext SCRIPT_TEMPLATE_CONTEXT - = new ScriptContext<>("xpack_template", TemplateScript.Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + = new ScriptContext<>("xpack_template", TemplateScript.Factory.class, 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..bd6446b82cb9c 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 @@ -6,8 +6,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; @@ -44,6 +42,5 @@ public interface Factory { WatcherConditionScript newInstance(Map params, WatchExecutionContext watcherContext); } - public static ScriptContext CONTEXT = new ScriptContext<>("watcher_condition", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + public static ScriptContext CONTEXT = new ScriptContext<>("watcher_condition", Factory.class, 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..e661ea56711b4 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 @@ -6,8 +6,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; @@ -45,6 +43,5 @@ public interface Factory { WatcherTransformScript newInstance(Map params, WatchExecutionContext watcherContext, Payload payload); } - public static ScriptContext CONTEXT = new ScriptContext<>("watcher_transform", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + public static ScriptContext CONTEXT = new ScriptContext<>("watcher_transform", Factory.class, true); } From 291da1013e40cb230b3f5972139c11bec75835ba Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 19 Oct 2021 07:41:55 -0500 Subject: [PATCH 2/6] Import Collectors --- .../test/java/org/elasticsearch/script/ScriptServiceTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index a201a65390fc8..2ce96e03a639c 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -31,6 +31,7 @@ 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; From bc9e5ad8ccb8e6691beffe2e26e70a58a20cb970 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 19 Oct 2021 12:01:50 -0500 Subject: [PATCH 3/6] Use full ScriptContext constructor, remove comments --- .../script/AbstractFieldScript.java | 44 ++++++++++++------- .../script/IngestConditionalScript.java | 5 ++- .../elasticsearch/script/IngestScript.java | 5 ++- .../elasticsearch/script/ScriptContext.java | 16 ++----- .../org/elasticsearch/script/ScriptStats.java | 7 --- .../elasticsearch/script/TemplateScript.java | 5 ++- .../script/ScriptStatsTests.java | 9 ---- .../elasticsearch/xpack/watcher/Watcher.java | 3 +- .../condition/WatcherConditionScript.java | 4 +- .../script/WatcherTransformScript.java | 4 +- 10 files changed, 52 insertions(+), 50 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java index f2b1f2a6ac7e0..73ae88fdebc57 100644 --- a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java +++ b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java @@ -19,6 +19,8 @@ import java.util.Map; import java.util.function.Function; +import static org.elasticsearch.core.TimeValue.timeValueMillis; + /** * Abstract base for scripts to execute to build scripted fields. Inspired by * {@link AggregationScript} but hopefully with less historical baggage. @@ -30,21 +32,33 @@ public abstract class AbstractFieldScript extends DocBasedScript { public static final int MAX_VALUES = 100; static ScriptContext newContext(String name, Class factoryClass) { - /* - * We rely on the script cache in two ways: - * 1. It caches the "heavy" part of mappings generated at runtime. - * 2. Mapping updates tend to try to compile the script twice. Not - * for any good reason. They just do. - * Thus we use the default cache size. - * - * We also disable compilation rate limits for runtime fields so we - * don't prevent mapping updates because we've performed too - * many recently. That'd just be lame. We also compile these - * scripts during search requests so this could totally be a - * source of runaway script compilations. We think folks will - * mostly reuse scripts though. - */ - return new ScriptContext<>(name, factoryClass, false); + return new ScriptContext<>( + name, + factoryClass, + /* + * We rely on the script cache in two ways: + * 1. It caches the "heavy" part of mappings generated at runtime. + * 2. Mapping updates tend to try to compile the script twice. Not + * for any good reason. They just do. + * Thus we use the default 100. + */ + 100, + timeValueMillis(0), + /* + * Disable compilation rate limits for runtime fields so we + * don't prevent mapping updates because we've performed too + * many recently. That'd just be lame. We also compile these + * scripts during search requests so this could totally be a + * source of runaway script compilations. We think folks will + * mostly reuse scripts though. + */ + false, + /* + * Disable runtime fields scripts from being allowed + * to be stored as part of the script meta data. + */ + false + ); } private static final Map> PARAMS_FUNCTIONS = org.elasticsearch.core.Map.of( diff --git a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java index 424f30e35973f..caa6cbbe0164b 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java @@ -8,6 +8,8 @@ package org.elasticsearch.script; +import org.elasticsearch.core.TimeValue; + import java.util.Map; /** @@ -18,7 +20,8 @@ public abstract class IngestConditionalScript { public static final String[] PARAMETERS = { "ctx" }; /** The context used to compile {@link IngestConditionalScript} factories. */ - public static final ScriptContext CONTEXT = new ScriptContext<>("processor_conditional", Factory.class, true); + public static final ScriptContext CONTEXT = new ScriptContext<>("processor_conditional", Factory.class, + 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 4e9a7aa35a3a7..bb444b132d1d0 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestScript.java @@ -9,6 +9,8 @@ package org.elasticsearch.script; +import org.elasticsearch.core.TimeValue; + import java.util.Map; /** @@ -19,7 +21,8 @@ public abstract class IngestScript { public static final String[] PARAMETERS = { "ctx" }; /** The context used to compile {@link IngestScript} factories. */ - public static final ScriptContext CONTEXT = new ScriptContext<>("ingest", Factory.class, true); + public static final ScriptContext CONTEXT = new ScriptContext<>("ingest", Factory.class, + 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/ScriptContext.java b/server/src/main/java/org/elasticsearch/script/ScriptContext.java index aafee11d82b07..1e84d36b08b13 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptContext.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptContext.java @@ -75,8 +75,8 @@ public final class ScriptContext { public final boolean allowStoredScript; /** Construct a context with the related instance and compiled classes with caller provided cache defaults */ - ScriptContext(String name, Class factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault, - boolean compilationRateLimited, boolean allowStoredScript) { + public ScriptContext(String name, Class factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault, + boolean compilationRateLimited, boolean allowStoredScript) { this.name = name; this.factoryClazz = factoryClazz; Method newInstanceMethod = findMethod("FactoryType", factoryClazz, "newInstance"); @@ -105,22 +105,12 @@ public final class ScriptContext { } /** 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), true, true); } - /** - * Construct a context for a system script usage, using the: - * - default cache size (only relevant when context caching enabled): 200 - * - default cache expiration: no expiration - * - unlimited compilation rate - */ - public ScriptContext(String name, Class factoryClazz, boolean allowStoredScript) { - this(name, factoryClazz, 200, TimeValue.timeValueMillis(0), false, allowStoredScript); - } - /** Returns a method with the given name, or throws an exception if multiple are found. */ private Method findMethod(String type, Class clazz, String methodName) { Method foundMethod = null; diff --git a/server/src/main/java/org/elasticsearch/script/ScriptStats.java b/server/src/main/java/org/elasticsearch/script/ScriptStats.java index b360a669052aa..832d44a46771e 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptStats.java @@ -109,13 +109,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(Fields.COMPILATIONS, compilations); builder.field(Fields.CACHE_EVICTIONS, cacheEvictions); builder.field(Fields.COMPILATION_LIMIT_TRIGGERED, compilationLimitTriggered); - /* TODO(stu): master only - builder.startArray(Fields.CONTEXTS); - for (ScriptContextStats contextStats: contextStats) { - contextStats.toXContent(builder, params); - } - builder.endArray(); - */ builder.endObject(); return builder; } diff --git a/server/src/main/java/org/elasticsearch/script/TemplateScript.java b/server/src/main/java/org/elasticsearch/script/TemplateScript.java index ea7699ff3c883..7356be0bbdc32 100644 --- a/server/src/main/java/org/elasticsearch/script/TemplateScript.java +++ b/server/src/main/java/org/elasticsearch/script/TemplateScript.java @@ -8,6 +8,8 @@ package org.elasticsearch.script; +import org.elasticsearch.core.TimeValue; + import java.util.Map; /** @@ -39,5 +41,6 @@ public interface Factory { // Remove compilation rate limit for ingest. Ingest pipelines may use many mustache templates, triggering compilation // 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, true); + public static final ScriptContext INGEST_CONTEXT = new ScriptContext<>("ingest_template", Factory.class, + 200, TimeValue.timeValueMillis(0), false, true); } diff --git a/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java b/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java index d7d5614f446b9..adbcc2df1ece5 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptStatsTests.java @@ -40,15 +40,6 @@ public void testXContent() throws IOException { stats.toXContent(builder, ToXContent.EMPTY_PARAMS); builder.endObject(); - /* TODO(stu): master only - String expected = "{\n" + - " \"script\" : {\n" + - " \"compilations\" : 1100,\n" + - " \"cache_evictions\" : 2211,\n" + - " \"compilation_limit_triggered\" : 3322\n" + - " }\n" + - "}"; - */ String expected = "{\n" + " \"script\" : {\n" + " \"compilations\" : 1100,\n" + 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 00d722ab3cc34..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 @@ -235,7 +235,8 @@ public class Watcher extends Plugin implements SystemIndexPlugin, ScriptPlugin, new ByteSizeValue(1, ByteSizeUnit.MB), new ByteSizeValue(10, ByteSizeUnit.MB), NodeScope); public static final ScriptContext SCRIPT_TEMPLATE_CONTEXT - = new ScriptContext<>("xpack_template", TemplateScript.Factory.class, true); + = new ScriptContext<>("xpack_template", TemplateScript.Factory.class, + 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 bd6446b82cb9c..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 @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.watcher.condition; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.watcher.support.Variables; @@ -42,5 +43,6 @@ public interface Factory { WatcherConditionScript newInstance(Map params, WatchExecutionContext watcherContext); } - public static ScriptContext CONTEXT = new ScriptContext<>("watcher_condition", Factory.class, true); + public static ScriptContext CONTEXT = new ScriptContext<>("watcher_condition", Factory.class, + 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 e661ea56711b4..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 @@ -6,6 +6,7 @@ */ package org.elasticsearch.xpack.watcher.transform.script; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.core.watcher.watch.Payload; @@ -43,5 +44,6 @@ public interface Factory { WatcherTransformScript newInstance(Map params, WatchExecutionContext watcherContext, Payload payload); } - public static ScriptContext CONTEXT = new ScriptContext<>("watcher_transform", Factory.class, true); + public static ScriptContext CONTEXT = new ScriptContext<>("watcher_transform", Factory.class, + 200, TimeValue.timeValueMillis(0), false, true); } From f82ce902a4589bcd84793861ceb3efd378c24d48 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 19 Oct 2021 12:46:56 -0500 Subject: [PATCH 4/6] Script: Deprecate script context cache Deprecate the script context cache in favor of the general cache. Users should use the following settings: `script.max_compilations_rate` to set the max compilation rate for user scripts such as filter scripts. Certain script contexts that submit scripts outside of the control of the user are exempted from this rate limit. Examples include runtime fields, ingest and watcher. `script.cache.max_size` to set the max size of the cache. `script.cache.expire` to set the expiration time for entries in the cache. Whats deprecated? `script.max_compilations_rate: use-context`. This special setting value was used to turn on the script context-specific caches. `script.context.$CONTEXT.cache_max_size`, use `script.cache.max_size` instead. `script.context.$CONTEXT.cache_expire`, use `script.cache.expire` instead. `script.context.$CONTEXT.max_compilations_rate`, use `script.max_compilations_rate` instead. The default cache size was increased from `100` to `3000`, which was approximately the max cache size when using context-specific caches. The default compilation rate limit was increased from `75/5m` to `150/5m` to account for increasing uses of scripts. Refs: #62899 --- .../reference/migration/migrate_7_16.asciidoc | 79 +++++++- .../modules/indices/circuit_breaker.asciidoc | 6 +- docs/reference/scripting/using.asciidoc | 12 +- .../elasticsearch/script/ScriptService.java | 33 +++- .../script/ScriptServiceTests.java | 180 ++++++++++++++---- .../xpack/deprecation/DeprecationChecks.java | 6 +- .../deprecation/NodeDeprecationChecks.java | 87 +++++++++ .../NodeDeprecationChecksTests.java | 102 ++++++++++ 8 files changed, 446 insertions(+), 59 deletions(-) diff --git a/docs/reference/migration/migrate_7_16.asciidoc b/docs/reference/migration/migrate_7_16.asciidoc index 794ca0b9c9548..d449230d95da6 100644 --- a/docs/reference/migration/migrate_7_16.asciidoc +++ b/docs/reference/migration/migrate_7_16.asciidoc @@ -88,7 +88,7 @@ logging>>. ==== *Details* + In SAML, Identity Providers (IdPs) can either be explicitly configured to -release a `NameID` with a specific format, or configured to attempt to conform +release a `NameID` with a specific format, or configured to attempt to conform with the requirements of a Service Provider (SP). The SP declares its requirements in the `NameIDPolicy` element of a SAML Authentication Request. In {es}, the `nameid_format` SAML realm setting controls the `NameIDPolicy` @@ -103,9 +103,9 @@ IdP. If you want to retain the previous behavior, set `nameid_format` to *Impact* + If you currently don't configure `nameid_format` explicitly, it's possible -that your IdP will reject authentication requests from {es} because the requests +that your IdP will reject authentication requests from {es} because the requests do not specify a `NameID` format (and your IdP is configured to expect one). -This mismatch can result in a broken SAML configuration. If you're unsure whether +This mismatch can result in a broken SAML configuration. If you're unsure whether your IdP is explicitly configured to use a certain `NameID` format and you want to retain current behavior , try setting `nameid_format` to `urn:oasis:names:tc:SAML:2.0:nameid-format:transient` explicitly. ==== @@ -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..8ffe6f16a33c2 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.max_size_in_bytes` 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/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index b79c89150656a..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> 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 [" + @@ -568,6 +586,7 @@ void setCacheHolder(Settings settings) { } 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)); } } diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 2ce96e03a639c..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,6 +28,7 @@ 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; @@ -36,9 +38,10 @@ 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; @@ -219,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"); @@ -231,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"); @@ -262,12 +318,18 @@ public void testContextCacheStats() throws IOException { "]; 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_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) - .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(cacheSizeA.getKey(), 1) + .put(compilationRateA.getKey(), aRate) + .put(cacheSizeB.getKey(), 2) + .put(compilationRateB.getKey(), bRate) .build()); // Context A @@ -291,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 { @@ -402,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() { @@ -419,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()); @@ -431,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(); @@ -447,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 { @@ -468,9 +551,12 @@ public void testCacheHolderContextConstructor() throws IOException { 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); @@ -481,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 { @@ -491,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) @@ -527,11 +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.entrySet().stream().filter(e -> e.getValue().compilationRateLimited) - .map(Map.Entry::getKey).collect(Collectors.toSet()); + Set contextNames = rateLimitedContexts.keySet(); String a = randomFrom(contextNames); String aRate = "77/5m"; String b = randomValueOtherThan(a, () -> randomFrom(contextNames)); @@ -552,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() ); @@ -599,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 { @@ -613,11 +712,15 @@ public void testFallbackToContextDefaults() throws IOException { 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() ); @@ -632,7 +735,7 @@ public void testFallbackToContextDefaults() throws IOException { 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); @@ -642,6 +745,9 @@ public void testFallbackToContextDefaults() throws IOException { 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() { 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."); + } } From 1d6998a7a0d2cd11701070b769461e207327786f Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 19 Oct 2021 13:40:36 -0500 Subject: [PATCH 5/6] Revert space at newline asciidoc changes --- docs/reference/migration/migrate_7_16.asciidoc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/reference/migration/migrate_7_16.asciidoc b/docs/reference/migration/migrate_7_16.asciidoc index d449230d95da6..290a8bb910c86 100644 --- a/docs/reference/migration/migrate_7_16.asciidoc +++ b/docs/reference/migration/migrate_7_16.asciidoc @@ -88,7 +88,7 @@ logging>>. ==== *Details* + In SAML, Identity Providers (IdPs) can either be explicitly configured to -release a `NameID` with a specific format, or configured to attempt to conform +release a `NameID` with a specific format, or configured to attempt to conform with the requirements of a Service Provider (SP). The SP declares its requirements in the `NameIDPolicy` element of a SAML Authentication Request. In {es}, the `nameid_format` SAML realm setting controls the `NameIDPolicy` @@ -103,9 +103,9 @@ IdP. If you want to retain the previous behavior, set `nameid_format` to *Impact* + If you currently don't configure `nameid_format` explicitly, it's possible -that your IdP will reject authentication requests from {es} because the requests +that your IdP will reject authentication requests from {es} because the requests do not specify a `NameID` format (and your IdP is configured to expect one). -This mismatch can result in a broken SAML configuration. If you're unsure whether +This mismatch can result in a broken SAML configuration. If you're unsure whether your IdP is explicitly configured to use a certain `NameID` format and you want to retain current behavior , try setting `nameid_format` to `urn:oasis:names:tc:SAML:2.0:nameid-format:transient` explicitly. ==== From ae168f6608fd61750d51a05f1b21cf4c73278468 Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Tue, 19 Oct 2021 14:28:59 -0500 Subject: [PATCH 6/6] script.max_size_in_bytes -> script.cache.max_size --- docs/reference/scripting/using.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/scripting/using.asciidoc b/docs/reference/scripting/using.asciidoc index 8ffe6f16a33c2..e1eaeaaedad93 100644 --- a/docs/reference/scripting/using.asciidoc +++ b/docs/reference/scripting/using.asciidoc @@ -403,7 +403,7 @@ 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.cache.expire` setting. -Use the `script.max_size_in_bytes` setting to configure the size of the cache. +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