From 808b70d2f906892e0b27fe62367723b70a48c8ae Mon Sep 17 00:00:00 2001 From: Stuart Tettemer Date: Thu, 21 Oct 2021 07:57:27 -0500 Subject: [PATCH] Script: Restore the scripting general cache (#79453) 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. System script contexts can now opt-out of compilation rate limiting using a flag rather than a sentinel rate limit value. 7.16: Script: Deprecate script context cache #79508 Refs: #62899 7.16: Script: Opt-out system contexts from script compilation rate limit #79459 Refs: #62899 --- .../modules/indices/circuit_breaker.asciidoc | 6 +- docs/reference/scripting/using.asciidoc | 12 +- .../admin/cluster/node/stats/NodeStats.java | 15 + .../common/settings/ClusterSettings.java | 3 + .../org/elasticsearch/node/NodeService.java | 1 + .../script/AbstractFieldScript.java | 2 +- .../script/IngestConditionalScript.java | 2 +- .../elasticsearch/script/IngestScript.java | 2 +- .../org/elasticsearch/script/ScriptCache.java | 17 +- .../script/ScriptCacheStats.java | 149 ++++++++ .../elasticsearch/script/ScriptContext.java | 14 +- .../elasticsearch/script/ScriptMetrics.java | 7 +- .../elasticsearch/script/ScriptService.java | 166 +++++++- .../org/elasticsearch/script/ScriptStats.java | 33 +- .../elasticsearch/script/TemplateScript.java | 2 +- .../cluster/node/stats/NodeStatsTests.java | 32 +- .../elasticsearch/cluster/DiskUsageTests.java | 18 +- .../script/ScriptCacheTests.java | 45 ++- .../script/ScriptServiceTests.java | 359 ++++++++++++++++-- .../MockInternalClusterInfoService.java | 3 +- .../test/InternalTestCluster.java | 9 +- .../AutoscalingMemoryInfoServiceTests.java | 1 + .../xpack/deprecation/DeprecationChecks.java | 6 +- .../deprecation/NodeDeprecationChecks.java | 77 ++++ .../NodeDeprecationChecksTests.java | 110 ++++++ ...chineLearningInfoTransportActionTests.java | 2 +- ...sportGetTrainedModelsStatsActionTests.java | 2 +- .../node/NodeStatsMonitoringDocTests.java | 3 +- .../elasticsearch/xpack/watcher/Watcher.java | 3 +- .../condition/WatcherConditionScript.java | 3 +- .../script/WatcherTransformScript.java | 3 +- 31 files changed, 990 insertions(+), 117 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java diff --git a/docs/reference/modules/indices/circuit_breaker.asciidoc b/docs/reference/modules/indices/circuit_breaker.asciidoc index b128ce5dfb37..161f3a8876f1 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 f49be226c2d3..e1eaeaaedad9 100644 --- a/docs/reference/scripting/using.asciidoc +++ b/docs/reference/scripting/using.asciidoc @@ -120,12 +120,8 @@ the `multiplier` parameter without {es} recompiling the script. } ---- -For most contexts, you can compile up to 75 scripts per 5 minutes by default. -For ingest contexts, the default script compilation rate is unlimited. You -can change these settings dynamically by setting -`script.context.$CONTEXT.max_compilations_rate`. For example, the following -setting limits script compilation to 100 scripts every 10 minutes for the -{painless}/painless-field-context.html[field context]: +You can compile up to 150 scripts per 5 minutes by default. +For ingest contexts, the default script compilation rate is unlimited. [source,js] ---- @@ -406,8 +402,8 @@ small. All scripts are cached by default so that they only need to be recompiled when updates occur. By default, scripts do not have a time-based expiration. -You can change this behavior by using the `script.context.$CONTEXT.cache_expire` setting. -Use the `script.context.$CONTEXT.cache_max_size` setting to configure the size of the cache. +You can change this behavior by using the `script.cache.expire` setting. +Use the `script.cache.max_size` setting to configure the size of the cache. NOTE: The size of scripts is limited to 65,535 bytes. Set the value of `script.max_size_in_bytes` to increase that soft limit. If your scripts are really large, then consider using a diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java index 7c17367f9ecb..855ed0c8676c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStats.java @@ -27,6 +27,7 @@ import org.elasticsearch.monitor.jvm.JvmStats; import org.elasticsearch.monitor.os.OsStats; import org.elasticsearch.monitor.process.ProcessStats; import org.elasticsearch.node.AdaptiveSelectionStats; +import org.elasticsearch.script.ScriptCacheStats; import org.elasticsearch.script.ScriptStats; import org.elasticsearch.threadpool.ThreadPoolStats; import org.elasticsearch.transport.TransportStats; @@ -71,6 +72,9 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment { @Nullable private ScriptStats scriptStats; + @Nullable + private ScriptCacheStats scriptCacheStats; + @Nullable private DiscoveryStats discoveryStats; @@ -98,6 +102,7 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment { http = in.readOptionalWriteable(HttpStats::new); breaker = in.readOptionalWriteable(AllCircuitBreakerStats::new); scriptStats = in.readOptionalWriteable(ScriptStats::new); + scriptCacheStats = scriptStats != null ? scriptStats.toScriptCacheStats() : null; discoveryStats = in.readOptionalWriteable(DiscoveryStats::new); ingestStats = in.readOptionalWriteable(IngestStats::new); adaptiveSelectionStats = in.readOptionalWriteable(AdaptiveSelectionStats::new); @@ -112,6 +117,7 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment { @Nullable DiscoveryStats discoveryStats, @Nullable IngestStats ingestStats, @Nullable AdaptiveSelectionStats adaptiveSelectionStats, + @Nullable ScriptCacheStats scriptCacheStats, @Nullable IndexingPressureStats indexingPressureStats) { super(node); this.timestamp = timestamp; @@ -128,6 +134,7 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment { this.discoveryStats = discoveryStats; this.ingestStats = ingestStats; this.adaptiveSelectionStats = adaptiveSelectionStats; + this.scriptCacheStats = scriptCacheStats; this.indexingPressureStats = indexingPressureStats; } @@ -223,6 +230,11 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment { return adaptiveSelectionStats; } + @Nullable + public ScriptCacheStats getScriptCacheStats() { + return scriptCacheStats; + } + @Nullable public IndexingPressureStats getIndexingPressureStats() { return indexingPressureStats; @@ -314,6 +326,9 @@ public class NodeStats extends BaseNodeResponse implements ToXContentFragment { if (getAdaptiveSelectionStats() != null) { getAdaptiveSelectionStats().toXContent(builder, params); } + if (getScriptCacheStats() != null) { + getScriptCacheStats().toXContent(builder, params); + } if (getIndexingPressureStats() != null) { getIndexingPressureStats().toXContent(builder, params); } diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index a1f2606782f1..9993eaa34465 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java @@ -372,6 +372,9 @@ public final class ClusterSettings extends AbstractScopedSettings { ScriptService.SCRIPT_CACHE_SIZE_SETTING, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING, ScriptService.SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING, + ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, + ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING, + ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING, ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING, ScriptService.SCRIPT_MAX_SIZE_IN_BYTES, ScriptService.TYPES_ALLOWED_SETTING, diff --git a/server/src/main/java/org/elasticsearch/node/NodeService.java b/server/src/main/java/org/elasticsearch/node/NodeService.java index 590a64ccd7b6..305d0aa99583 100644 --- a/server/src/main/java/org/elasticsearch/node/NodeService.java +++ b/server/src/main/java/org/elasticsearch/node/NodeService.java @@ -118,6 +118,7 @@ public class NodeService implements Closeable { discoveryStats ? coordinator.stats() : null, ingest ? ingestService.stats() : null, adaptiveSelection ? responseCollectorService.getAdaptiveStats(searchTransportService.getPendingSearchRequests()) : null, + scriptCache ? scriptService.cacheStats() : null, indexingPressure ? this.indexingPressure.stats() : null); } diff --git a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java index 2792ce48b94f..7cd479a38828 100644 --- a/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java +++ b/server/src/main/java/org/elasticsearch/script/AbstractFieldScript.java @@ -52,7 +52,7 @@ public abstract class AbstractFieldScript extends DocBasedScript { * source of runaway script compilations. We think folks will * mostly reuse scripts though. */ - ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), + false, /* * Disable runtime fields scripts from being allowed * to be stored as part of the script meta data. diff --git a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java index 430f6c22a116..caa6cbbe0164 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestConditionalScript.java @@ -21,7 +21,7 @@ public abstract class IngestConditionalScript { /** The context used to compile {@link IngestConditionalScript} factories. */ public static final ScriptContext CONTEXT = new ScriptContext<>("processor_conditional", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); /** The generic runtime parameters for the script. */ private final Map params; diff --git a/server/src/main/java/org/elasticsearch/script/IngestScript.java b/server/src/main/java/org/elasticsearch/script/IngestScript.java index a0fa0d9bbdde..bb444b132d1d 100644 --- a/server/src/main/java/org/elasticsearch/script/IngestScript.java +++ b/server/src/main/java/org/elasticsearch/script/IngestScript.java @@ -22,7 +22,7 @@ public abstract class IngestScript { /** The context used to compile {@link IngestScript} factories. */ public static final ScriptContext CONTEXT = new ScriptContext<>("ingest", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); /** The generic runtime parameters for the script. */ private final Map params; diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCache.java b/server/src/main/java/org/elasticsearch/script/ScriptCache.java index 622e9910ac37..b2fcdc9f2457 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 @@ public class ScriptCache { 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. @@ -121,6 +118,10 @@ public class ScriptCache { throw (T) t; } + public ScriptStats stats() { + return scriptMetrics.stats(); + } + public ScriptContextStats stats(String context) { return scriptMetrics.stats(context); } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java b/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java new file mode 100644 index 000000000000..28183c1d4630 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/script/ScriptCacheStats.java @@ -0,0 +1,149 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.script; + +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.xcontent.ToXContentFragment; +import org.elasticsearch.xcontent.XContentBuilder; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; + +// This class is deprecated in favor of ScriptStats and ScriptContextStats +public class ScriptCacheStats implements Writeable, ToXContentFragment { + private final Map context; + private final ScriptStats general; + + public ScriptCacheStats(Map context) { + this.context = Collections.unmodifiableMap(context); + this.general = null; + } + + public ScriptCacheStats(ScriptStats general) { + this.general = Objects.requireNonNull(general); + this.context = null; + } + + public ScriptCacheStats(StreamInput in) throws IOException { + boolean isContext = in.readBoolean(); + if (isContext == false) { + general = new ScriptStats(in); + context = null; + return; + } + + general = null; + int size = in.readInt(); + Map context = new HashMap<>(size); + for (int i=0; i < size; i++) { + String name = in.readString(); + context.put(name, new ScriptStats(in)); + } + this.context = Collections.unmodifiableMap(context); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + if (general != null) { + out.writeBoolean(false); + general.writeTo(out); + return; + } + + out.writeBoolean(true); + out.writeInt(context.size()); + for (String name: context.keySet().stream().sorted().collect(Collectors.toList())) { + out.writeString(name); + context.get(name).writeTo(out); + } + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(Fields.SCRIPT_CACHE_STATS); + builder.startObject(Fields.SUM); + if (general != null) { + builder.field(ScriptStats.Fields.COMPILATIONS, general.getCompilations()); + builder.field(ScriptStats.Fields.CACHE_EVICTIONS, general.getCacheEvictions()); + builder.field(ScriptStats.Fields.COMPILATION_LIMIT_TRIGGERED, general.getCompilationLimitTriggered()); + builder.endObject().endObject(); + return builder; + } + + ScriptStats sum = sum(); + builder.field(ScriptStats.Fields.COMPILATIONS, sum.getCompilations()); + builder.field(ScriptStats.Fields.CACHE_EVICTIONS, sum.getCacheEvictions()); + builder.field(ScriptStats.Fields.COMPILATION_LIMIT_TRIGGERED, sum.getCompilationLimitTriggered()); + builder.endObject(); + + builder.startArray(Fields.CONTEXTS); + for (String name: context.keySet().stream().sorted().collect(Collectors.toList())) { + ScriptStats stats = context.get(name); + builder.startObject(); + builder.field(Fields.CONTEXT, name); + builder.field(ScriptStats.Fields.COMPILATIONS, stats.getCompilations()); + builder.field(ScriptStats.Fields.CACHE_EVICTIONS, stats.getCacheEvictions()); + builder.field(ScriptStats.Fields.COMPILATION_LIMIT_TRIGGERED, stats.getCompilationLimitTriggered()); + builder.endObject(); + } + builder.endArray(); + builder.endObject(); + + return builder; + } + + /** + * Get the context specific stats, null if using general cache + */ + public Map getContextStats() { + return context; + } + + /** + * Get the general stats, null if using context cache + */ + public ScriptStats getGeneralStats() { + return general; + } + + /** + * The sum of all script stats, either the general stats or the sum of all stats of the context stats. + */ + public ScriptStats sum() { + if (general != null) { + return general; + } + long compilations = 0; + long cacheEvictions = 0; + long compilationLimitTriggered = 0; + for (ScriptStats stat: context.values()) { + compilations += stat.getCompilations(); + cacheEvictions += stat.getCacheEvictions(); + compilationLimitTriggered += stat.getCompilationLimitTriggered(); + } + return new ScriptStats( + compilations, + cacheEvictions, + compilationLimitTriggered + ); + } + + static final class Fields { + static final String SCRIPT_CACHE_STATS = "script_cache"; + static final String CONTEXT = "context"; + static final String SUM = "sum"; + static final String CONTEXTS = "contexts"; + } +} diff --git a/server/src/main/java/org/elasticsearch/script/ScriptContext.java b/server/src/main/java/org/elasticsearch/script/ScriptContext.java index eb158f444096..1e84d36b08b1 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 @@ import java.lang.reflect.Method; * be {@code boolean needs_score()}. */ public final class ScriptContext { + /** The default compilation rate limit for contexts with compilation rate limiting enabled */ + public static final Tuple DEFAULT_COMPILATION_RATE_LIMIT = new Tuple<>(150, TimeValue.timeValueMinutes(5)); /** A unique identifier for this context. */ public final String name; @@ -66,15 +68,15 @@ public final class ScriptContext { /** The default expiration of a script in the cache for the context, if not overridden */ public final TimeValue cacheExpireDefault; - /** The default max compilation rate for scripts in this context. Script compilation is throttled if this is exceeded */ - public final Tuple maxCompilationRateDefault; + /** Is compilation rate limiting enabled for this context? */ + public final boolean compilationRateLimited; /** Determines if the script can be stored as part of the cluster state. */ public final boolean allowStoredScript; /** Construct a context with the related instance and compiled classes with caller provided cache defaults */ public ScriptContext(String name, Class factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault, - Tuple maxCompilationRateDefault, boolean allowStoredScript) { + boolean compilationRateLimited, boolean allowStoredScript) { this.name = name; this.factoryClazz = factoryClazz; Method newInstanceMethod = findMethod("FactoryType", factoryClazz, "newInstance"); @@ -98,15 +100,15 @@ public final class ScriptContext { this.cacheSizeDefault = cacheSizeDefault; this.cacheExpireDefault = cacheExpireDefault; - this.maxCompilationRateDefault = maxCompilationRateDefault; + this.compilationRateLimited = compilationRateLimited; this.allowStoredScript = allowStoredScript; } /** Construct a context with the related instance and compiled classes with defaults for cacheSizeDefault, cacheExpireDefault and - * maxCompilationRateDefault and allow scripts of this context to be stored scripts */ + * compilationRateLimited and allow scripts of this context to be stored scripts */ public ScriptContext(String name, Class factoryClazz) { // cache size default, cache expire default, max compilation rate are defaults from ScriptService. - this(name, factoryClazz, 100, TimeValue.timeValueMillis(0), new Tuple<>(75, TimeValue.timeValueMinutes(5)), true); + this(name, factoryClazz, 100, TimeValue.timeValueMillis(0), true, true); } /** Returns a method with the given name, or throws an exception if multiple are found. */ diff --git a/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java b/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java index f853305c3cd4..c4d26bc861c8 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptMetrics.java @@ -27,6 +27,10 @@ public class ScriptMetrics { compilationLimitTriggered.inc(); } + public ScriptStats stats() { + return new ScriptStats(compilationsMetric.count(), cacheEvictionsMetric.count(), compilationLimitTriggered.count()); + } + public ScriptContextStats stats(String context) { return new ScriptContextStats( context, @@ -36,4 +40,5 @@ public class ScriptMetrics { new ScriptContextStats.TimeSeries(), new ScriptContextStats.TimeSeries() ); - }} + } +} diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index d0e9f6542549..104d37467c1c 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.ClusterStateApplier; 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; @@ -34,6 +36,7 @@ import java.io.Closeable; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; @@ -49,11 +52,31 @@ import java.util.stream.Collectors; 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"; + // Special setting value for SCRIPT_GENERAL_MAX_COMPILATIONS_RATE to indicate the script service should use context + // specific caches + static final ScriptCache.CompilationRate USE_CONTEXT_RATE_VALUE = new ScriptCache.CompilationRate(-1, TimeValue.MINUS_ONE); + static final String USE_CONTEXT_RATE_KEY = "use-context"; + + public static final Setting SCRIPT_GENERAL_CACHE_SIZE_SETTING = + 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.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", "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."; @@ -62,11 +85,15 @@ 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, 0, Property.NodeScope, Property.Dynamic)); + "cache_max_size", + 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, TimeValue.timeValueMillis(0), Property.NodeScope, Property.Dynamic)); + "cache_expire", + key -> Setting.positiveTimeSetting(key, SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, TimeValue.timeValueMillis(0), + Property.NodeScope, Property.Dynamic, Property.Deprecated)); // Unlimited compilation rate for context-specific script caches static final String UNLIMITED_COMPILATION_RATE_KEY = "unlimited"; @@ -76,8 +103,8 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp "max_compilations_rate", 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)); + new ScriptCache.CompilationRate(value), + Property.NodeScope, Property.Dynamic, Property.Deprecated)); private static final ScriptCache.CompilationRate SCRIPT_COMPILATION_RATE_ZERO = new ScriptCache.CompilationRate(0, TimeValue.ZERO); @@ -185,7 +212,11 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp // Validation requires knowing which contexts exist. this.validateCacheSettings(settings); - this.cacheHolder.set(contextCacheHolder(settings)); + this.setCacheHolder(settings); + } + + public static boolean isUseContextCacheSet(Settings settings) { + return SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE); } /** @@ -202,12 +233,28 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp for (ScriptContext context: contexts.values()) { clusterSettings.addSettingsUpdateConsumer( (settings) -> cacheHolder.get().set(context.name, contextCache(settings, context)), - List.of(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context.name), - SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context.name), - SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context.name) + Arrays.asList(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context.name), + SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context.name), + SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context.name), + SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, + // general settings used for fallbacks + SCRIPT_GENERAL_CACHE_SIZE_SETTING ) ); } + + // Handle all settings for context and general caches, this flips between general and context caches. + clusterSettings.addSettingsUpdateConsumer( + this::setCacheHolder, + Arrays.asList(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING, + SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, + SCRIPT_GENERAL_CACHE_SIZE_SETTING, + SCRIPT_MAX_COMPILATIONS_RATE_SETTING, + SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING, + SCRIPT_CACHE_EXPIRE_SETTING, + SCRIPT_CACHE_SIZE_SETTING), + this::validateCacheSettings + ); } /** @@ -215,8 +262,13 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp * when using the general cache. */ void validateCacheSettings(Settings settings) { - List> affixes = List.of(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, SCRIPT_CACHE_EXPIRE_SETTING, - SCRIPT_CACHE_SIZE_SETTING); + boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE); + if (useContext) { + deprecationLogger.warn(DeprecationCategory.SCRIPTING, "scripting-context-cache", + USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE); + } + List> affixes = Arrays.asList(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, SCRIPT_CACHE_EXPIRE_SETTING, + SCRIPT_CACHE_SIZE_SETTING); List customRates = new ArrayList<>(); List keys = new ArrayList<>(); for (Setting.AffixSetting affix: affixes) { @@ -231,6 +283,11 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp } } } + if (useContext == false && keys.isEmpty() == false) { + keys.sort(Comparator.naturalOrder()); + throw new IllegalArgumentException("Context cache settings [" + String.join(", ", keys) + "] requires [" + + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey() + "] to be [" + USE_CONTEXT_RATE_KEY + "]"); + } if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings)) { if (customRates.size() > 0) { customRates.sort(Comparator.naturalOrder()); @@ -238,6 +295,12 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp String.join(", ", customRates) + "] if compile rates disabled via [" + SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.getKey() + "]"); } + 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 [" + + SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.getKey() + "]"); + } } } @@ -489,11 +552,50 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp return cacheHolder.get().stats(); } + public ScriptCacheStats cacheStats() { + return cacheHolder.get().cacheStats(); + } + @Override public void applyClusterState(ClusterChangedEvent event) { clusterState = event.state(); } + void setCacheHolder(Settings settings) { + CacheHolder current = cacheHolder.get(); + boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE); + + if (current == null) { + if (useContext) { + cacheHolder.set(contextCacheHolder(settings)); + } else { + cacheHolder.set(generalCacheHolder(settings)); + } + return; + } + + // Update + if (useContext) { + if (current.general != null) { + // Flipping to context specific + cacheHolder.set(contextCacheHolder(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 || + 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)); + } + } + + CacheHolder generalCacheHolder(Settings settings) { + return new CacheHolder(SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(settings), SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(settings), + SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings), SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey()); + } + CacheHolder contextCacheHolder(Settings settings) { Map contextCache = new HashMap<>(contexts.size()); contexts.forEach((k, v) -> contextCache.put(k, contextCache(settings, v))); @@ -510,13 +612,15 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp 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()); @@ -528,12 +632,19 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp * 2) context mode, if the context script cache is configured. There is no general cache in this case. */ static class CacheHolder { + final ScriptCache general; final Map> contextCache; + CacheHolder(int cacheMaxSize, TimeValue cacheExpire, ScriptCache.CompilationRate maxCompilationRate, String contextRateSetting) { + contextCache = null; + general = new ScriptCache(cacheMaxSize, cacheExpire, maxCompilationRate, contextRateSetting); + } + CacheHolder(Map context) { Map> refs = new HashMap<>(context.size()); context.forEach((k, v) -> refs.put(k, new AtomicReference<>(v))); contextCache = Collections.unmodifiableMap(refs); + general = null; } /** @@ -541,6 +652,9 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp * the given context. Returns null in context mode if the requested context does not exist. */ ScriptCache get(String context) { + if (general != null) { + return general; + } AtomicReference ref = contextCache.get(context); if (ref == null) { return null; @@ -549,17 +663,35 @@ public class ScriptService implements Closeable, ClusterStateApplier, ScriptComp } ScriptStats stats() { - List stats = new ArrayList<>(contextCache.size()); - for (Map.Entry> entry : contextCache.entrySet()) { - stats.add(entry.getValue().get().stats(entry.getKey())); + if (general != null) { + return general.stats(); } - return new ScriptStats(stats); + List contextStats = new ArrayList<>(contextCache.size()); + for (Map.Entry> entry : contextCache.entrySet()) { + ScriptCache cache = entry.getValue().get(); + contextStats.add(cache.stats(entry.getKey())); + } + return new ScriptStats(contextStats); + } + + ScriptCacheStats cacheStats() { + if (general != null) { + return new ScriptCacheStats(general.stats()); + } + Map context = new HashMap<>(contextCache.size()); + for (String name: contextCache.keySet()) { + context.put(name, contextCache.get(name).get().stats()); + } + return new ScriptCacheStats(context); } /** * Update a single context cache if we're in the context cache mode otherwise no-op. */ void set(String name, ScriptCache cache) { + if (general != null) { + return; + } AtomicReference ref = contextCache.get(name); assert ref != null : "expected script cache to exist for context [" + name + "]"; ScriptCache oldCache = ref.get(); diff --git a/server/src/main/java/org/elasticsearch/script/ScriptStats.java b/server/src/main/java/org/elasticsearch/script/ScriptStats.java index 55ba2a847f7b..2b8925cb10ce 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptStats.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptStats.java @@ -15,8 +15,11 @@ import org.elasticsearch.xcontent.ToXContentFragment; import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; -import java.util.stream.Collectors; +import java.util.Map; public class ScriptStats implements Writeable, ToXContentFragment { private final List contextStats; @@ -24,9 +27,11 @@ public class ScriptStats implements Writeable, ToXContentFragment { private final long cacheEvictions; private final long compilationLimitTriggered; - public ScriptStats(List contextStats) { - this.contextStats = contextStats.stream().sorted().collect(Collectors.toUnmodifiableList()); + ArrayList ctxStats = new ArrayList<>(contextStats.size()); + ctxStats.addAll(contextStats); + ctxStats.sort(ScriptContextStats::compareTo); + this.contextStats = Collections.unmodifiableList(ctxStats); long compilations = 0; long cacheEvictions = 0; long compilationLimitTriggered = 0; @@ -40,6 +45,17 @@ public class ScriptStats implements Writeable, ToXContentFragment { this.compilationLimitTriggered = compilationLimitTriggered; } + public ScriptStats(long compilations, long cacheEvictions, long compilationLimitTriggered) { + this.contextStats = Collections.emptyList(); + this.compilations = compilations; + this.cacheEvictions = cacheEvictions; + this.compilationLimitTriggered = compilationLimitTriggered; + } + + public ScriptStats(ScriptContextStats context) { + this(context.getCompilations(), context.getCacheEvictions(), context.getCompilationLimitTriggered()); + } + public ScriptStats(StreamInput in) throws IOException { compilations = in.readVLong(); cacheEvictions = in.readVLong(); @@ -71,6 +87,17 @@ public class ScriptStats implements Writeable, ToXContentFragment { return compilationLimitTriggered; } + public ScriptCacheStats toScriptCacheStats() { + if (contextStats.isEmpty()) { + return new ScriptCacheStats(this); + } + Map contexts = new HashMap<>(contextStats.size()); + for (ScriptContextStats contextStats : contextStats) { + contexts.put(contextStats.getContext(), new ScriptStats(contextStats)); + } + return new ScriptCacheStats(contexts); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(Fields.SCRIPT_STATS); diff --git a/server/src/main/java/org/elasticsearch/script/TemplateScript.java b/server/src/main/java/org/elasticsearch/script/TemplateScript.java index d1777316434c..7356be0bbdc3 100644 --- a/server/src/main/java/org/elasticsearch/script/TemplateScript.java +++ b/server/src/main/java/org/elasticsearch/script/TemplateScript.java @@ -42,5 +42,5 @@ public abstract class TemplateScript { // rate limiting. MustacheScriptEngine explicitly checks for TemplateScript. Rather than complicating the implementation there by // creating a new Script class (as would be customary), this context is used to avoid the default rate limit. public static final ScriptContext INGEST_CONTEXT = new ScriptContext<>("ingest_template", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java index 4272dfa77f2c..39e197d7a0f9 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.monitor.os.OsStats; import org.elasticsearch.monitor.process.ProcessStats; import org.elasticsearch.node.AdaptiveSelectionStats; import org.elasticsearch.node.ResponseCollectorService; +import org.elasticsearch.script.ScriptCacheStats; import org.elasticsearch.script.ScriptContextStats; import org.elasticsearch.script.ScriptStats; import org.elasticsearch.test.ESTestCase; @@ -418,6 +419,34 @@ public class NodeStatsTests extends ESTestCase { assertEquals(aStats.responseTime, bStats.responseTime, 0.01); }); } + ScriptCacheStats scriptCacheStats = nodeStats.getScriptCacheStats(); + ScriptCacheStats deserializedScriptCacheStats = deserializedNodeStats.getScriptCacheStats(); + if (scriptCacheStats == null) { + assertNull(deserializedScriptCacheStats); + } else if (deserializedScriptCacheStats.getContextStats() != null) { + Map deserialized = deserializedScriptCacheStats.getContextStats(); + long evictions = 0; + long limited = 0; + long compilations = 0; + Map stats = scriptCacheStats.getContextStats(); + for (String context: stats.keySet()) { + ScriptStats deserStats = deserialized.get(context); + ScriptStats generatedStats = stats.get(context); + + evictions += generatedStats.getCacheEvictions(); + assertEquals(generatedStats.getCacheEvictions(), deserStats.getCacheEvictions()); + + limited += generatedStats.getCompilationLimitTriggered(); + assertEquals(generatedStats.getCompilationLimitTriggered(), deserStats.getCompilationLimitTriggered()); + + compilations += generatedStats.getCompilations(); + assertEquals(generatedStats.getCompilations(), deserStats.getCompilations()); + } + ScriptStats sum = deserializedScriptCacheStats.sum(); + assertEquals(evictions, sum.getCacheEvictions()); + assertEquals(limited, sum.getCompilationLimitTriggered()); + assertEquals(compilations, sum.getCompilations()); + } } } } @@ -688,10 +717,11 @@ public class NodeStatsTests extends ESTestCase { } adaptiveSelectionStats = new AdaptiveSelectionStats(nodeConnections, nodeStats); } + ScriptCacheStats scriptCacheStats = scriptStats != null ? scriptStats.toScriptCacheStats() : null; //TODO NodeIndicesStats are not tested here, way too complicated to create, also they need to be migrated to Writeable yet return new NodeStats(node, randomNonNegativeLong(), null, osStats, processStats, jvmStats, threadPoolStats, fsInfo, transportStats, httpStats, allCircuitBreakerStats, scriptStats, discoveryStats, - ingestStats, adaptiveSelectionStats, null); + ingestStats, adaptiveSelectionStats, scriptCacheStats, null); } private static ScriptContextStats.TimeSeries randomTimeSeries() { diff --git a/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java b/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java index 61466be75cf9..dc89a392c1e2 100644 --- a/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java @@ -151,11 +151,14 @@ public class DiskUsageTests extends ESTestCase { }; List nodeStats = Arrays.asList( new NodeStats(new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null, null), + null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null, + null, null), new NodeStats(new DiscoveryNode("node_2", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null, null), + null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null, + null, null), new NodeStats(new DiscoveryNode("node_3", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null, null) + null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null, + null, null) ); InternalClusterInfoService.fillDiskUsagePerNode(nodeStats, newLeastAvaiableUsages, newMostAvaiableUsages); DiskUsage leastNode_1 = newLeastAvaiableUsages.get("node_1"); @@ -192,11 +195,14 @@ public class DiskUsageTests extends ESTestCase { }; List nodeStats = Arrays.asList( new NodeStats(new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null, null), + null,null,null,null,null,new FsInfo(0, null, node1FSInfo), null,null,null,null,null, null, null, + null, null), new NodeStats(new DiscoveryNode("node_2", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null, null), + null,null,null,null,null, new FsInfo(0, null, node2FSInfo), null,null,null,null,null, null, null, + null, null), new NodeStats(new DiscoveryNode("node_3", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0, - null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null, null) + null,null,null,null,null, new FsInfo(0, null, node3FSInfo), null,null,null,null,null, null, null, + null, null) ); InternalClusterInfoService.fillDiskUsagePerNode(nodeStats, newLeastAvailableUsages, newMostAvailableUsages); DiskUsage leastNode_1 = newLeastAvailableUsages.get("node_1"); diff --git a/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java b/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java index 6ee7149a460e..efcbf07cd3f7 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptCacheTests.java @@ -21,7 +21,7 @@ public class ScriptCacheTests extends ESTestCase { public void testCompilationCircuitBreaking() throws Exception { String context = randomFrom( ScriptModule.CORE_CONTEXTS.values().stream().filter( - c -> c.maxCompilationRateDefault.equals(ScriptCache.UNLIMITED_COMPILATION_RATE) == false + c -> c.compilationRateLimited ).collect(Collectors.toList()) ).name; final TimeValue expire = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY); @@ -55,10 +55,37 @@ public class ScriptCacheTests extends ESTestCase { } } + 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(); + ScriptCache cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(1, TimeValue.timeValueMinutes(1)), settingName); + cache.checkCompilationLimit(); // should pass + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(2, TimeValue.timeValueMinutes(1)), settingName); + 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)), settingName); + 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)), settingName); + expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit); + cache = new ScriptCache(size, expire, + new ScriptCache.CompilationRate(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)), settingName); + int largeLimit = randomIntBetween(1000, 10000); + for (int i = 0; i < largeLimit; i++) { + cache.checkCompilationLimit(); + } + } + public void testUnlimitedCompilationRate() { String context = randomFrom( ScriptModule.CORE_CONTEXTS.values().stream().filter( - c -> c.maxCompilationRateDefault.equals(ScriptCache.UNLIMITED_COMPILATION_RATE) == false + c -> c.compilationRateLimited ).collect(Collectors.toList()) ).name; final Integer size = ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY); @@ -73,4 +100,18 @@ public class ScriptCacheTests extends ESTestCase { 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(); + 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 + } + } } diff --git a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java index 1ea9780c762a..cdca6be6797b 100644 --- a/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java +++ b/server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java @@ -7,6 +7,7 @@ */ package org.elasticsearch.script; +import org.apache.logging.log4j.Level; import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.admin.cluster.storedscripts.GetStoredScriptRequest; import org.elasticsearch.cluster.ClusterName; @@ -16,6 +17,7 @@ import org.elasticsearch.common.breaker.CircuitBreakingException; 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; @@ -36,7 +38,12 @@ import java.util.stream.Collectors; import static org.elasticsearch.script.ScriptService.SCRIPT_CACHE_EXPIRE_SETTING; import static org.elasticsearch.script.ScriptService.SCRIPT_CACHE_SIZE_SETTING; +import static org.elasticsearch.script.ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING; +import static org.elasticsearch.script.ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING; +import static org.elasticsearch.script.ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING; import static org.elasticsearch.script.ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING; +import static org.elasticsearch.script.ScriptService.USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE; +import static org.elasticsearch.script.ScriptService.USE_CONTEXT_RATE_KEY; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -50,6 +57,7 @@ public class ScriptServiceTests extends ESTestCase { private ScriptService scriptService; private Settings baseSettings; private ClusterSettings clusterSettings; + private Map> rateLimitedContexts; @Before public void setup() throws IOException { @@ -68,6 +76,7 @@ public class ScriptServiceTests extends ESTestCase { 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 { @@ -204,7 +213,6 @@ public class ScriptServiceTests extends ESTestCase { scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(contexts.values())); assertEquals(1L, scriptService.stats().getCompilations()); } - public void testMultipleCompilationsCountedInCompilationStats() throws IOException { buildScriptService(Settings.EMPTY); int numberOfCompilations = randomIntBetween(1, 20); @@ -215,7 +223,7 @@ public class ScriptServiceTests extends ESTestCase { 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()); @@ -224,41 +232,106 @@ public class ScriptServiceTests extends ESTestCase { assertEquals(1L, scriptService.stats().getCompilations()); } - public void testIndexedScriptCountedInCompilationStats() throws IOException { + 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()); - assertEquals(1L, getByContext(scriptService.stats(), ctx.name).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(true, new DeprecationWarning(Level.WARN, 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"); + buildScriptService(builder.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()); + } + + 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(true, new DeprecationWarning(Level.WARN, 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_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context.name).getKey(), 1) + .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}, + new DeprecationWarning(Level.WARN, 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"); + buildScriptService(builder.build()); + scriptService.compile(new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap()), randomFrom(contexts.values())); + scriptService.compile(new Script(ScriptType.INLINE, "test", "2+2", Collections.emptyMap()), randomFrom(contexts.values())); + assertEquals(2L, scriptService.stats().getCompilations()); + assertEquals(2L, scriptService.cacheStats().getGeneralStats().getCompilations()); + assertEquals(1L, scriptService.stats().getCacheEvictions()); + assertEquals(1L, scriptService.cacheStats().getGeneralStats().getCacheEvictions()); } public void testContextCacheStats() throws IOException { - ScriptContext contextA = randomFrom(contexts.values()); + ScriptContext contextA = randomFrom(rateLimitedContexts.values()); String aRate = "2/10m"; - ScriptContext contextB = randomValueOtherThan(contextA, () -> randomFrom(contexts.values())); + ScriptContext contextB = randomValueOtherThan(contextA, () -> randomFrom(rateLimitedContexts.values())); String bRate = "3/10m"; BiFunction msg = (rate, ctx) -> ( "[script] Too many dynamic script compilations within, max: [" + rate + "]; please use indexed, or scripts with parameters instead; this limit can be changed by the [script.context." + ctx + ".max_compilations_rate] setting" ); + Setting cacheSizeA = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextA.name); + Setting compilationRateA = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextA.name); + + Setting cacheSizeB = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextB.name); + Setting compilationRateB = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextB.name); + buildScriptService(Settings.builder() - .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextA.name).getKey(), 1) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextA.name).getKey(), aRate) - .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextB.name).getKey(), 2) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextB.name).getKey(), bRate) + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(cacheSizeA.getKey(), 1) + .put(compilationRateA.getKey(), aRate) + .put(cacheSizeB.getKey(), 2) + .put(compilationRateB.getKey(), bRate) .build()); // Context A @@ -295,6 +368,9 @@ public class ScriptServiceTests extends ESTestCase { assertEquals(5L, scriptService.stats().getCompilations()); assertEquals(2L, scriptService.stats().getCacheEvictions()); assertEquals(3L, scriptService.stats().getCompilationLimitTriggered()); + + assertSettingDeprecationsAndWarnings(new Setting[]{cacheSizeA, compilationRateA, cacheSizeB, compilationRateB}, + new DeprecationWarning(Level.WARN, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE)); } private ScriptContextStats getByContext(ScriptStats stats, String context) { @@ -366,17 +442,126 @@ public class ScriptServiceTests extends ESTestCase { iae.getMessage()); } + public void testConflictContextSettings() throws IOException { + IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "10/1m") + .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("field").getKey(), 123).build()); + }); + assertEquals("Context cache settings [script.context.field.cache_max_size] requires " + + "[script.max_compilations_rate] to be [use-context]", + illegal.getMessage() + ); + + illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "10/1m") + .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("ingest").getKey(), "5m").build()); + }); + + assertEquals("Context cache settings [script.context.ingest.cache_expire] requires " + + "[script.max_compilations_rate] to be [use-context]", + illegal.getMessage() + ); + + illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "10/1m") + .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("score").getKey(), "50/5m").build()); + }); + + assertEquals("Context cache settings [script.context.score.max_compilations_rate] requires " + + "[script.max_compilations_rate] to be [use-context]", + illegal.getMessage() + ); + + 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(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}, + new DeprecationWarning(Level.WARN, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE)); + } + + public void testFallbackContextSettings() { + int cacheSizeBackup = randomIntBetween(0, 1024); + int cacheSizeFoo = randomValueOtherThan(cacheSizeBackup, () -> randomIntBetween(0, 1024)); + + String cacheExpireBackup = randomTimeValue(1, 1000, "h"); + TimeValue cacheExpireBackupParsed = TimeValue.parseTimeValue(cacheExpireBackup, ""); + String cacheExpireFoo = randomValueOtherThan(cacheExpireBackup, () -> randomTimeValue(1, 1000, "h")); + TimeValue cacheExpireFooParsed = TimeValue.parseTimeValue(cacheExpireFoo, ""); + + 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(cacheSizeSetting.getKey(), cacheSizeFoo) + .put(SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), cacheExpireBackup) + .put(cacheExpireSetting.getKey(), cacheExpireFoo) + .build(); + + assertEquals(cacheSizeFoo, ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo").get(s).intValue()); + assertEquals(cacheSizeBackup, ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("bar").get(s).intValue()); + + assertEquals(cacheExpireFooParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo").get(s)); + assertEquals(cacheExpireBackupParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("bar").get(s)); + 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(contextMaxCompilationRate.getKey(), + ScriptService.USE_CONTEXT_RATE_KEY) + .build(); + + assertEquals(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(s), ScriptService.USE_CONTEXT_RATE_VALUE); + + IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> { + ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getAsMap(s); + }); + + assertEquals("parameter must contain a positive integer and a timevalue, i.e. 10/1m, but was [use-context]", illegal.getMessage()); + assertSettingDeprecationsAndWarnings(new Setting[]{contextMaxCompilationRate}); + } + + public void testCacheHolderGeneralConstructor() throws IOException { + String compilationRate = "77/5m"; + buildScriptService( + Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate).build() + ); + + ScriptService.CacheHolder holder = scriptService.cacheHolder.get(); + + assertNotNull(holder.general); + assertNull(holder.contextCache); + assertEquals(holder.general.rate, new ScriptCache.CompilationRate(compilationRate)); + } + public void testCacheHolderContextConstructor() throws IOException { - String a = randomFrom(contexts.keySet()); - String b = randomValueOtherThan(a, () -> randomFrom(contexts.keySet())); + String a = randomFrom(rateLimitedContexts.keySet()); + String b = randomValueOtherThan(a, () -> randomFrom(rateLimitedContexts.keySet())); String aCompilationRate = "77/5m"; String bCompilationRate = "78/6m"; + Setting aSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a); + Setting bSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b); buildScriptService(Settings.builder() - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a).getKey(), aCompilationRate) - .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b).getKey(), bCompilationRate) + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY) + .put(aSetting.getKey(), aCompilationRate) + .put(bSetting.getKey(), bCompilationRate) .build()); + assertNull(scriptService.cacheHolder.get().general); assertNotNull(scriptService.cacheHolder.get().contextCache); assertEquals(contexts.keySet(), scriptService.cacheHolder.get().contextCache.keySet()); @@ -384,11 +569,35 @@ public class ScriptServiceTests extends ESTestCase { 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}, + new DeprecationWarning(Level.WARN, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE)); + } + + public void testCompilationRateUnlimitedContextOnly() throws IOException { + IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.UNLIMITED_COMPILATION_RATE_KEY) + .build()); + }); + 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(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}, + new DeprecationWarning(Level.WARN, 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) @@ -399,32 +608,61 @@ public class ScriptServiceTests extends ESTestCase { "[script.disable_max_compilations_rate]", illegal.getMessage()); + illegal = expectThrows(IllegalArgumentException.class, () -> { + buildScriptService(Settings.builder() + .put("script.disable_max_compilations_rate", true) + .put("script.max_compilations_rate", "76/10m") + .build()); + }); + assertEquals("Cannot set custom general compilation rates [script.max_compilations_rate] " + + "to [76/10m] if compile rates disabled via [script.disable_max_compilations_rate]", + illegal.getMessage()); + 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")}, + new DeprecationWarning(Level.WARN, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE)); } public void testCacheHolderChangeSettings() throws IOException { - Set contextNames = contexts.keySet(); + Set contextNames = rateLimitedContexts.keySet(); String a = randomFrom(contextNames); String aRate = "77/5m"; String b = randomValueOtherThan(a, () -> randomFrom(contextNames)); String bRate = "78/6m"; String c = randomValueOtherThanMany(s -> a.equals(s) || b.equals(s), () -> randomFrom(contextNames)); + String compilationRate = "77/5m"; + ScriptCache.CompilationRate generalRate = new ScriptCache.CompilationRate(compilationRate); - buildScriptService(Settings.EMPTY); - - Settings settings = 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) + Settings s = Settings.builder() + .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate) .build(); + buildScriptService(s); + + assertNotNull(scriptService.cacheHolder.get().general); + // Set should not throw when using general cache + scriptService.cacheHolder.get().set(c, scriptService.contextCache(s, contexts.get(c))); + 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_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() + ); + + assertNull(scriptService.cacheHolder.get().general); assertNotNull(scriptService.cacheHolder.get().contextCache); - scriptService.cacheHolder.get().set(a, scriptService.contextCache(settings, contexts.get(a))); - scriptService.cacheHolder.get().set(b, scriptService.contextCache(settings, contexts.get(b))); - scriptService.cacheHolder.get().set(c, scriptService.contextCache(settings, contexts.get(c))); // get of missing context should be null assertNull(scriptService.cacheHolder.get().get( randomValueOtherThanMany(contexts.keySet()::contains, () -> randomAlphaOfLength(8))) @@ -443,6 +681,27 @@ public class ScriptServiceTests extends ESTestCase { contexts.get(b))); assertEquals(new ScriptCache.CompilationRate(aRate), scriptService.cacheHolder.get().contextCache.get(b).get().rate); + + scriptService.setCacheHolder(s); + assertNotNull(scriptService.cacheHolder.get().general); + assertNull(scriptService.cacheHolder.get().contextCache); + assertEquals(generalRate, scriptService.cacheHolder.get().general.rate); + + scriptService.setCacheHolder( + Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), bRate).build() + ); + + assertNotNull(scriptService.cacheHolder.get().general); + assertNull(scriptService.cacheHolder.get().contextCache); + assertEquals(new ScriptCache.CompilationRate(bRate), scriptService.cacheHolder.get().general.rate); + + ScriptService.CacheHolder holder = scriptService.cacheHolder.get(); + scriptService.setCacheHolder( + 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 { @@ -451,20 +710,23 @@ public class ScriptServiceTests extends ESTestCase { int contextCacheSize = randomIntBetween(1, 1024); TimeValue contextExpire = TimeValue.timeValueMinutes(randomIntBetween(10, 200)); - buildScriptService(Settings.EMPTY); + buildScriptService( + Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "75/5m").build() + ); - String name = "ingest"; + String name = "score"; + Setting cacheSizeContextSetting = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(name); + Setting cacheExpireContextSetting = SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(name); + Setting compilationRateContextSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(name); // Use context specific - scriptService.cacheHolder.get().set( - name, - scriptService.contextCache(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) - .build(), - contexts.get(name) - )); + scriptService.setCacheHolder(Settings.builder() + .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() + ); ScriptService.CacheHolder holder = scriptService.cacheHolder.get(); assertNotNull(holder.contextCache); @@ -475,18 +737,31 @@ public class ScriptServiceTests extends ESTestCase { assertEquals(contextCacheSize, holder.contextCache.get(name).get().cacheSize); assertEquals(contextExpire, holder.contextCache.get(name).get().cacheExpire); - ScriptContext ingest = contexts.get(name); + ScriptContext score = contexts.get(name); // Fallback to context defaults - buildScriptService(Settings.EMPTY); + buildScriptService(Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY).build()); holder = scriptService.cacheHolder.get(); assertNotNull(holder.contextCache); assertNotNull(holder.contextCache.get(name)); assertNotNull(holder.contextCache.get(name).get()); - assertEquals(ingest.maxCompilationRateDefault, holder.contextCache.get(name).get().rate.asTuple()); - assertEquals(ingest.cacheSizeDefault, holder.contextCache.get(name).get().cacheSize); - assertEquals(ingest.cacheExpireDefault, holder.contextCache.get(name).get().cacheExpire); + assertEquals(ScriptContext.DEFAULT_COMPILATION_RATE_LIMIT, holder.contextCache.get(name).get().rate.asTuple()); + assertEquals(score.cacheSizeDefault, holder.contextCache.get(name).get().cacheSize); + assertEquals(score.cacheExpireDefault, holder.contextCache.get(name).get().cacheExpire); + + assertSettingDeprecationsAndWarnings(new Setting[]{cacheSizeContextSetting, cacheExpireContextSetting, + compilationRateContextSetting}, new DeprecationWarning(Level.WARN, USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE)); + } + + protected HashMap> compilationRateLimitedContexts() { + HashMap> rateLimited = new HashMap<>(); + for (Map.Entry> entry: contexts.entrySet()) { + if (entry.getValue().compilationRateLimited) { + rateLimited.put(entry.getKey(), entry.getValue()); + } + } + return rateLimited; } private void assertCompileRejected(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) { diff --git a/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java b/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java index 100b6611226e..fc71839b9320 100644 --- a/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java +++ b/test/framework/src/main/java/org/elasticsearch/cluster/MockInternalClusterInfoService.java @@ -74,7 +74,8 @@ public class MockInternalClusterInfoService extends InternalClusterInfoService { .map(fsInfoPath -> diskUsageFunction.apply(discoveryNode, fsInfoPath)) .toArray(FsInfo.Path[]::new)), nodeStats.getTransport(), nodeStats.getHttp(), nodeStats.getBreaker(), nodeStats.getScriptStats(), nodeStats.getDiscoveryStats(), - nodeStats.getIngestStats(), nodeStats.getAdaptiveSelectionStats(), nodeStats.getIndexingPressureStats()); + nodeStats.getIngestStats(), nodeStats.getAdaptiveSelectionStats(), nodeStats.getScriptCacheStats(), + nodeStats.getIndexingPressureStats()); }).collect(Collectors.toList()); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 6cfebd327581..4305f69924ce 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -93,7 +93,6 @@ import org.elasticsearch.node.Node; import org.elasticsearch.node.NodeService; import org.elasticsearch.node.NodeValidationException; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.SearchService; import org.elasticsearch.tasks.TaskManager; @@ -521,16 +520,16 @@ public final class InternalTestCluster extends TestCluster { builder.put(TransportSettings.PING_SCHEDULE.getKey(), RandomNumbers.randomIntBetween(random, 100, 2000) + "ms"); } + if (random.nextBoolean()) { - String ctx = randomFrom(random, ScriptModule.CORE_CONTEXTS.keySet()); - builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(ctx).getKey(), + builder.put(ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), RandomNumbers.randomIntBetween(random, 0, 2000)); } if (random.nextBoolean()) { - String ctx = randomFrom(random, ScriptModule.CORE_CONTEXTS.keySet()); - builder.put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(ctx).getKey(), + builder.put(ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), timeValueMillis(RandomNumbers.randomIntBetween(random, 750, 10000000)).getStringRep()); } + if (random.nextBoolean()) { int initialMillisBound = RandomNumbers.randomIntBetween(random,10, 100); builder.put(TransportReplicationAction.REPLICATION_INITIAL_RETRY_BACKOFF_BOUND.getKey(), timeValueMillis(initialMillisBound)); diff --git a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java index fb0a073c02e8..1c005095854b 100644 --- a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java +++ b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryInfoServiceTests.java @@ -367,6 +367,7 @@ public class AutoscalingMemoryInfoServiceTests extends AutoscalingTestCase { null, null, null, + null, null ); } 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 cece62ed0b85..c0274c85d635 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 @@ -79,7 +79,11 @@ public class DeprecationChecks { NodeDeprecationChecks::checkMonitoringSettingDecommissionAlerts, NodeDeprecationChecks::checkMonitoringSettingEsCollectionEnabled, NodeDeprecationChecks::checkMonitoringSettingCollectionEnabled, - NodeDeprecationChecks::checkMonitoringSettingCollectionInterval + NodeDeprecationChecks::checkMonitoringSettingCollectionInterval, + NodeDeprecationChecks::checkScriptContextCache, + NodeDeprecationChecks::checkScriptContextCompilationsRateLimitSetting, + NodeDeprecationChecks::checkScriptContextCacheSizeSetting, + NodeDeprecationChecks::checkScriptContextCacheExpirationSetting ); static List> INDEX_SETTINGS_CHECKS = List.of( 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 e3471d4df7a8..355fc21efd60 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 @@ -13,6 +13,7 @@ import org.elasticsearch.common.settings.SecureSetting; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.xpack.core.deprecation.DeprecationIssue; import org.elasticsearch.xpack.core.monitoring.MonitoringDeprecatedSettings; import org.elasticsearch.xpack.core.security.authc.RealmConfig; @@ -364,4 +365,80 @@ public class NodeDeprecationChecks { DeprecationIssue.Level.WARNING, settings); } + + static DeprecationIssue checkScriptContextCache(final Settings settings, + final PluginsAndModules pluginsAndModules) { + if (ScriptService.isUseContextCacheSet(settings)) { + return new DeprecationIssue(DeprecationIssue.Level.WARNING, + ScriptService.USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE, + "https://ela.st/es-deprecation-7-script-context-cache", + "found deprecated 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) { + 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.WARNING, + String.format(Locale.ROOT, + "Setting context-specific rate limits [%s] is deprecated." + + " Use [%s] to rate limit the compilation of user scripts." + + " Context-specific caches are no longer needed to prevent system scripts from triggering rate limits.", + maxSettings, ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey()), + "https://ela.st/es-deprecation-7-script-context-cache", + String.format(Locale.ROOT, "[%s] is deprecated and will be removed in a future release", maxSettings), + false, null); + } + return null; + } + + static DeprecationIssue checkScriptContextCacheSizeSetting(final Settings settings, + final PluginsAndModules pluginsAndModules) { + 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.WARNING, + String.format(Locale.ROOT, + "Setting a context-specific cache size [%s] is deprecated." + + " Use [%s] to configure the size of the general cache for scripts." + + " Context-specific caches are no longer needed to prevent system scripts from triggering rate limits.", + cacheSizeSettings, ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey()), + "https://ela.st/es-deprecation-7-script-context-cache", + String.format(Locale.ROOT, "[%s] is deprecated and will be removed in a future release", cacheSizeSettings), + false, null); + } + return null; + } + + static DeprecationIssue checkScriptContextCacheExpirationSetting(final Settings settings, + final PluginsAndModules pluginsAndModules) { + 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.WARNING, + String.format(Locale.ROOT, + "Setting a context-specific cache expiration [%s] is deprecated." + + " Use [%s] to configure the expiration of the general cache." + + " Context-specific caches are no longer needed to prevent system scripts from triggering rate limits.", + cacheExpireSettings, ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey()), + "https://ela.st/es-deprecation-7-script-context-cache", + String.format(Locale.ROOT, "[%s] is deprecated and will be removed in a future release", 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 7e490157966d..310470e2ae63 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 @@ -14,6 +14,7 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.TimeValue; import org.elasticsearch.env.Environment; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.deprecation.DeprecationIssue; @@ -349,4 +350,113 @@ public class NodeDeprecationChecksTests extends ESTestCase { "[xpack.monitoring.exporters.test.index.template.create_legacy_templates]", false, null))); } + + + public void testScriptContextCacheSetting() { + Settings settings = Settings.builder() + .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "use-context") + .build(); + + List issues = DeprecationChecks.filterChecks(NODE_SETTINGS_CHECKS, c -> c.apply(settings, null)); + + assertThat( + issues, + hasItem( + new DeprecationIssue(DeprecationIssue.Level.WARNING, + ScriptService.USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE, + "https://ela.st/es-deprecation-7-script-context-cache", + "found deprecated script context caches in use, change setting to compilation rate or remove " + + "setting to use the default", + false, null)) + ); + } + + public void testScriptContextCompilationsRateLimitSetting() { + List contexts = 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(); + + List issues = DeprecationChecks.filterChecks(NODE_SETTINGS_CHECKS, c -> c.apply(settings, null)); + + assertThat( + issues, + hasItem( + new DeprecationIssue(DeprecationIssue.Level.WARNING, + "Setting context-specific rate limits" + + " [script.context.field.max_compilations_rate,script.context.score.max_compilations_rate] is deprecated." + + " Use [script.max_compilations_rate] to rate limit the compilation of user scripts." + + " Context-specific caches are no longer needed to prevent system scripts from triggering rate limits.", + "https://ela.st/es-deprecation-7-script-context-cache", + "[script.context.field.max_compilations_rate,script.context.score.max_compilations_rate] is deprecated and" + + " will be removed in a future release", + 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 = 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(); + + List issues = DeprecationChecks.filterChecks(NODE_SETTINGS_CHECKS, c -> c.apply(settings, null)); + + assertThat( + issues, + hasItem( + new DeprecationIssue(DeprecationIssue.Level.WARNING, + "Setting a context-specific cache size" + + " [script.context.filter.cache_max_size,script.context.update.cache_max_size] is deprecated." + + " Use [script.cache.max_size] to configure the size of the general cache for scripts." + + " Context-specific caches are no longer needed to prevent system scripts from triggering rate limits.", + "https://ela.st/es-deprecation-7-script-context-cache", + "[script.context.filter.cache_max_size,script.context.update.cache_max_size] is deprecated and will be" + + " removed in a future release", + 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 = 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(); + + List issues = DeprecationChecks.filterChecks(NODE_SETTINGS_CHECKS, c -> c.apply(settings, null)); + + assertThat( + issues, + hasItem( + new DeprecationIssue(DeprecationIssue.Level.WARNING, + "Setting a context-specific cache expiration" + + " [script.context.interval.cache_expire,script.context.moving-function.cache_expire] is deprecated." + + " Use [script.cache.expire] to configure the expiration of the general cache." + + " Context-specific caches are no longer needed to prevent system scripts from triggering rate limits.", + "https://ela.st/es-deprecation-7-script-context-cache", + "[script.context.interval.cache_expire,script.context.moving-function.cache_expire] is deprecated and will be" + + " removed in a future release", + false, null))); + + + assertWarnings("[script.context.interval.cache_expire] setting was deprecated in Elasticsearch and will be" + + " removed in a future release! See the breaking changes documentation for the next major version.", + "[script.context.moving-function.cache_expire] setting was deprecated in Elasticsearch and will be removed in a future" + + " release! See the breaking changes documentation for the next major version."); + } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java index 9a283f31abe6..0402b32b5a8a 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningInfoTransportActionTests.java @@ -686,7 +686,7 @@ public class MachineLearningInfoTransportActionTests extends ESTestCase { IntStream.range(0, pipelineNames.size()).boxed().collect(Collectors.toMap(pipelineNames::get, processorStats::get))); return new NodeStats(mock(DiscoveryNode.class), Instant.now().toEpochMilli(), null, null, null, null, null, null, null, null, - null, null, null, ingestStats, null, null); + null, null, null, ingestStats, null, null, null); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportGetTrainedModelsStatsActionTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportGetTrainedModelsStatsActionTests.java index 46e018d8f0f1..d694fda60ec1 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportGetTrainedModelsStatsActionTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportGetTrainedModelsStatsActionTests.java @@ -299,7 +299,7 @@ public class TransportGetTrainedModelsStatsActionTests extends ESTestCase { IntStream.range(0, pipelineids.size()).boxed().collect(Collectors.toMap(pipelineids::get, processorStats::get))); return new NodeStats(mock(DiscoveryNode.class), Instant.now().toEpochMilli(), null, null, null, null, null, null, null, null, - null, null, null, ingestStats, null, null); + null, null, null, ingestStats, null, null, null); } diff --git a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java index ad4d99347c06..387513cda0bd 100644 --- a/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java +++ b/x-pack/plugin/monitoring/src/test/java/org/elasticsearch/xpack/monitoring/collector/node/NodeStatsMonitoringDocTests.java @@ -378,6 +378,7 @@ public class NodeStatsMonitoringDocTests extends BaseFilteredMonitoringDocTestCa emptySet(), Version.CURRENT); - return new NodeStats(discoveryNode, no, indices, os, process, jvm, threadPool, fs, null, null, null, null, null, null, null, null); + return new NodeStats(discoveryNode, no, indices, os, process, jvm, threadPool, fs, null, null, null, null, null, null, null, null, + null); } } 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 5d31bb245649..f682fa9e0bb1 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 @@ -51,7 +51,6 @@ import org.elasticsearch.plugins.SystemIndexPlugin; 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; @@ -233,7 +232,7 @@ public class Watcher extends Plugin implements SystemIndexPlugin, ScriptPlugin, public static final ScriptContext SCRIPT_TEMPLATE_CONTEXT = new ScriptContext<>("xpack_template", TemplateScript.Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); private static final Logger logger = LogManager.getLogger(Watcher.class); private WatcherIndexingListener listener; diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java index bc38efd0c657..04d4b97a8508 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/WatcherConditionScript.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.watcher.condition; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.script.ScriptCache; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.watcher.support.Variables; @@ -45,5 +44,5 @@ public abstract class WatcherConditionScript { } public static ScriptContext CONTEXT = new ScriptContext<>("watcher_condition", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); } diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java index 465f87cc26bd..3a48e25635f1 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transform/script/WatcherTransformScript.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.watcher.transform.script; import org.elasticsearch.core.TimeValue; -import org.elasticsearch.script.ScriptCache; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext; import org.elasticsearch.xpack.core.watcher.watch.Payload; @@ -46,5 +45,5 @@ public abstract class WatcherTransformScript { } public static ScriptContext CONTEXT = new ScriptContext<>("watcher_transform", Factory.class, - 200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true); + 200, TimeValue.timeValueMillis(0), false, true); }