1
0
Эх сурвалжийг харах

Scripting: Deprecate general cache settings (#55038)

* Scripting: Deprecate general cache settings

* Add script.disable_max_compilations_rate setting

* Move construction to ScriptCache

* Use ScriptService to do updates of CacheHolder

* Remove fallbacks

* Add SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING to ClusterSettings

* Node scope

* Use back compat

* 8.0 for bwc

* script.max_compilations_rate=2048/1m -> script.disable_max_compilations_rate=true in docker compose

* do not guard in esnode

* Doc update

* isSnapshotBuild() -> systemProperty 'es.script.disable_max_compilations_rate', 'true'

* Do not use snapshot in gradle to set max_compilations_rate

* Expose cacheHolder as package private

* monospace 75/5m in cbreaker docs, single space in using

* More detail in general compilation rate error

* Test: don't modify defaultConfig on upgrade
Stuart Tettemer 5 жил өмнө
parent
commit
bd64da0960

+ 5 - 1
buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

@@ -1069,7 +1069,11 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         baseConfig.put("cluster.routing.allocation.disk.watermark.low", "1b");
         baseConfig.put("cluster.routing.allocation.disk.watermark.high", "1b");
         // increase script compilation limit since tests can rapid-fire script compilations
-        baseConfig.put("script.max_compilations_rate", "2048/1m");
+        if (getVersion().getMajor() >= 8) {
+            baseConfig.put("script.disable_max_compilations_rate", "true");
+        } else {
+            baseConfig.put("script.max_compilations_rate", "2048/1m");
+        }
         if (getVersion().getMajor() >= 6) {
             baseConfig.put("cluster.routing.allocation.disk.watermark.flood_stage", "1b");
         }

+ 0 - 4
distribution/docker/docker-compose.yml

@@ -15,7 +15,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true
@@ -64,7 +63,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true
@@ -113,7 +111,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
     volumes:
        - ./build/oss-repo:/tmp/es-repo
@@ -147,7 +144,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
     volumes:
        - ./build/oss-repo:/tmp/es-repo

+ 3 - 3
docs/reference/modules/indices/circuit_breaker.asciidoc

@@ -111,8 +111,8 @@ within a period of time.
 See the "prefer-parameters" section of the <<modules-scripting-using,scripting>>
 documentation for more information.
 
-`script.max_compilations_rate`::
+`script.context.$CONTEXT.max_compilations_rate`::
 
     Limit for the number of unique dynamic scripts within a certain interval
-    that are allowed to be compiled. Defaults to 75/5m, meaning 75 every 5 
-    minutes.
+    that are allowed to be compiled for a given context. Defaults to `75/5m`,
+    meaning 75 every 5 minutes.

+ 5 - 3
docs/reference/scripting/using.asciidoc

@@ -98,9 +98,11 @@ second version is only compiled once.
 
 If you compile too many unique scripts within a small amount of time,
 Elasticsearch will reject the new dynamic scripts with a
-`circuit_breaking_exception` error. By default, up to 15 inline scripts per
-minute will be compiled. You can change this setting dynamically by setting
-`script.max_compilations_rate`.
+`circuit_breaking_exception` error. By default, up to 75 scripts per
+5 minutes will be compiled for most contexts and 375 scripts per 5 minutes
+for ingest contexts. You can change these settings dynamically by setting
+`script.context.$CONTEXT.max_compilations_rate` eg.
+`script.context.field.max_compilations_rate=100/10m`.
 
 ========================================
 

+ 0 - 2
qa/remote-clusters/docker-compose-oss.yml

@@ -15,7 +15,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
     volumes:
        - ./build/oss-repo:/tmp/es-repo
@@ -50,7 +49,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
     volumes:
        - ./build/oss-repo:/tmp/es-repo

+ 0 - 2
qa/remote-clusters/docker-compose.yml

@@ -15,7 +15,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true
@@ -65,7 +64,6 @@ services:
        - cluster.routing.allocation.disk.watermark.low=1b
        - cluster.routing.allocation.disk.watermark.high=1b
        - cluster.routing.allocation.disk.watermark.flood_stage=1b
-       - script.max_compilations_rate=2048/1m
        - node.store.allow_mmap=false
        - xpack.security.enabled=true
        - xpack.security.transport.ssl.enabled=true

+ 1 - 0
server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -363,6 +363,7 @@ public final class ClusterSettings extends AbstractScopedSettings {
             ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING,
             ScriptService.SCRIPT_CACHE_SIZE_SETTING,
             ScriptService.SCRIPT_CACHE_EXPIRE_SETTING,
+            ScriptService.SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING,
             ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING,
             ScriptService.SCRIPT_MAX_SIZE_IN_BYTES,
             ScriptService.TYPES_ALLOWED_SETTING,

+ 117 - 112
server/src/main/java/org/elasticsearch/script/ScriptService.java

@@ -46,7 +46,6 @@ import java.io.Closeable;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
@@ -103,15 +102,15 @@ public class ScriptService implements Closeable, ClusterStateApplier {
             };
 
     public static final Setting<Integer> SCRIPT_GENERAL_CACHE_SIZE_SETTING =
-        Setting.intSetting("script.cache.max_size", 100, 0, Property.NodeScope);
+        Setting.intSetting("script.cache.max_size", 100, 0, Property.NodeScope, Property.Deprecated);
     public static final Setting<TimeValue> SCRIPT_GENERAL_CACHE_EXPIRE_SETTING =
-        Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope);
+        Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope, Property.Deprecated);
     public static final Setting<Integer> SCRIPT_MAX_SIZE_IN_BYTES =
         Setting.intSetting("script.max_size_in_bytes", 65535, 0, Property.Dynamic, Property.NodeScope);
     public static final Setting<Tuple<Integer, TimeValue>> SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING =
-        new Setting<>("script.max_compilations_rate", "75/5m",
+        new Setting<>("script.max_compilations_rate", USE_CONTEXT_RATE_KEY,
             (String value) -> value.equals(USE_CONTEXT_RATE_KEY) ? USE_CONTEXT_RATE_VALUE: MAX_COMPILATION_RATE_FUNCTION.apply(value),
-            Property.Dynamic, Property.NodeScope);
+            Property.Dynamic, Property.NodeScope, Property.Deprecated);
 
     // Per-context settings
     static final String CONTEXT_PREFIX = "script.context.";
@@ -120,14 +119,11 @@ public class ScriptService implements Closeable, ClusterStateApplier {
 
     public static final Setting.AffixSetting<Integer> SCRIPT_CACHE_SIZE_SETTING =
         Setting.affixKeySetting(CONTEXT_PREFIX,
-            "cache_max_size",
-            key -> Setting.intSetting(key, SCRIPT_GENERAL_CACHE_SIZE_SETTING, 0, Property.NodeScope, Property.Dynamic));
+            "cache_max_size", key -> Setting.intSetting(key, 0, Property.NodeScope, Property.Dynamic));
 
     public static final Setting.AffixSetting<TimeValue> SCRIPT_CACHE_EXPIRE_SETTING =
         Setting.affixKeySetting(CONTEXT_PREFIX,
-            "cache_expire",
-            key -> Setting.positiveTimeSetting(key, SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, TimeValue.timeValueMillis(0),
-                                               Property.NodeScope, Property.Dynamic));
+            "cache_expire", key -> Setting.positiveTimeSetting(key, TimeValue.timeValueMillis(0), Property.NodeScope, Property.Dynamic));
 
     // Unlimited compilation rate for context-specific script caches
     static final String UNLIMITED_COMPILATION_RATE_KEY = "unlimited";
@@ -142,6 +138,9 @@ public class ScriptService implements Closeable, ClusterStateApplier {
 
     private static final Tuple<Integer, TimeValue> SCRIPT_COMPILATION_RATE_ZERO = new Tuple<>(0, TimeValue.ZERO);
 
+    public static final Setting<Boolean> SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING =
+        Setting.boolSetting("script.disable_max_compilations_rate", false, Property.NodeScope);
+
     public static final String ALLOW_NONE = "none";
 
     public static final Setting<List<String>> TYPES_ALLOWED_SETTING =
@@ -159,7 +158,8 @@ public class ScriptService implements Closeable, ClusterStateApplier {
 
     private int maxSizeInBytes;
 
-    private final AtomicReference<CacheHolder> cacheHolder;
+    // package private for tests
+    final AtomicReference<CacheHolder> cacheHolder = new AtomicReference<>();
 
     public ScriptService(Settings settings, Map<String, ScriptEngine> engines, Map<String, ScriptContext<?>> contexts) {
         this.engines = Objects.requireNonNull(engines);
@@ -242,7 +242,7 @@ public class ScriptService implements Closeable, ClusterStateApplier {
 
         // Validation requires knowing which contexts exist.
         this.validateCacheSettings(settings);
-        cacheHolder = new AtomicReference<>(new CacheHolder(settings, contexts.values(), compilationLimitsEnabled()));
+        this.setCacheHolder(settings);
     }
 
     /**
@@ -258,7 +258,7 @@ public class ScriptService implements Closeable, ClusterStateApplier {
         // Handle all updatable per-context settings at once for each context.
         for (ScriptContext<?> context: contexts.values()) {
             clusterSettings.addSettingsUpdateConsumer(
-                (settings) -> cacheHolder.get().updateContextSettings(settings, context),
+                (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),
@@ -270,11 +270,12 @@ public class ScriptService implements Closeable, ClusterStateApplier {
 
         // Handle all settings for context and general caches, this flips between general and context caches.
         clusterSettings.addSettingsUpdateConsumer(
-            (settings) -> cacheHolder.set(cacheHolder.get().withUpdatedCacheSettings(settings)),
+            (settings) -> setCacheHolder(settings),
             List.of(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING,
                     SCRIPT_GENERAL_CACHE_EXPIRE_SETTING,
                     SCRIPT_GENERAL_CACHE_SIZE_SETTING,
                     SCRIPT_MAX_COMPILATIONS_RATE_SETTING,
+                    SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING,
                     SCRIPT_CACHE_EXPIRE_SETTING,
                     SCRIPT_CACHE_SIZE_SETTING),
             this::validateCacheSettings
@@ -289,31 +290,39 @@ public class ScriptService implements Closeable, ClusterStateApplier {
         boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE);
         List<Setting.AffixSetting<?>> affixes = List.of(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, SCRIPT_CACHE_EXPIRE_SETTING,
                                                         SCRIPT_CACHE_SIZE_SETTING);
+        List<String> customRates = new ArrayList<>();
         List<String> keys = new ArrayList<>();
         for (Setting.AffixSetting<?> affix: affixes) {
-            keys.addAll(getConcreteSettingKeys(affix, settings));
+            for (String context: affix.getAsMap(settings).keySet()) {
+                String s = affix.getConcreteSettingForNamespace(context).getKey();
+                if (contexts.containsKey(context) == false) {
+                    throw new IllegalArgumentException("Context [" + context + "] doesn't exist for setting [" + s + "]");
+                }
+                keys.add(s);
+                if (affix.equals(SCRIPT_MAX_COMPILATIONS_RATE_SETTING)) {
+                    customRates.add(s);
+                }
+            }
         }
         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 + "]");
         }
-    }
-
-    /**
-     * Get concrete settings keys from affix settings for given Settings.  Throws an IllegalArgumentException if the namespace of matching
-     * affix settings do not match any context name.
-     */
-    List<String> getConcreteSettingKeys(Setting.AffixSetting<?> setting, Settings settings) {
-        List<String> concreteKeys = new ArrayList<>();
-        for (String context: setting.getAsMap(settings).keySet()) {
-            String s = setting.getConcreteSettingForNamespace(context).getKey();
-            if (contexts.containsKey(context) == false) {
-                throw new IllegalArgumentException("Context [" + context + "] doesn't exist for setting [" + s + "]");
+        if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings)) {
+            if (customRates.size() > 0) {
+                customRates.sort(Comparator.naturalOrder());
+                throw new IllegalArgumentException("Cannot set custom context compilation rates [" +
+                    String.join(", ", customRates) + "] if compile rates disabled via [" +
+                    SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.getKey() + "]");
+            }
+            if (useContext == false) {
+                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() + "]");
             }
-            concreteKeys.add(s);
         }
-        concreteKeys.sort(Comparator.naturalOrder());
-        return concreteKeys;
     }
 
     @Override
@@ -576,89 +585,85 @@ public class ScriptService implements Closeable, ClusterStateApplier {
         clusterState = event.state();
     }
 
-    /**
-     * Container for the ScriptCache(s).  This class operates in two modes:
-     * 1) general mode, if the general script cache is configured.  There are no context caches in this case.
-     * 2) context mode, if the context script cache is configured.  There is no general cache in this case.
-     */
-    static class CacheHolder {
-        final ScriptCache general;
-        final Map<String, AtomicReference<ScriptCache>> contextCache;
+    void setCacheHolder(Settings settings) {
+        CacheHolder current = cacheHolder.get();
+        boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE);
 
-        final Set<ScriptContext<?>> contexts;
-        final boolean compilationLimitsEnabled;
-
-        CacheHolder(Settings settings, Collection<ScriptContext<?>> contexts, boolean compilationLimitsEnabled) {
-            this.compilationLimitsEnabled = compilationLimitsEnabled;
-            this.contexts = Set.copyOf(contexts);
-            if (SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE)) {
-                this.general = null;
-                Map<String, AtomicReference<ScriptCache>> contextCache = new HashMap<>(this.contexts.size());
-                for (ScriptContext<?> context : this.contexts) {
-                    contextCache.put(context.name,
-                                     new AtomicReference<>(contextFromSettings(settings, context, this.compilationLimitsEnabled)));
-                }
-                this.contextCache = Collections.unmodifiableMap(contextCache);
+        if (current == null) {
+            if (useContext) {
+                cacheHolder.set(contextCacheHolder(settings));
             } else {
-                this.contextCache = null;
-                this.general = new ScriptCache(
-                    SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(settings),
-                    SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(settings),
-                    compilationLimitsEnabled ?
-                        SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings) :
-                        SCRIPT_COMPILATION_RATE_ZERO,
-                    SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey());
+                cacheHolder.set(generalCacheHolder(settings));
             }
+            return;
         }
 
-        /**
-         * Create a ScriptCache for the given context.
-         */
-        private static ScriptCache contextFromSettings(Settings settings, ScriptContext<?> context, boolean compilationLimitsEnabled) {
-            String name = context.name;
-            Tuple<Integer, TimeValue> compileRate;
-            Setting<Tuple<Integer, TimeValue>> rateSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(name);
-            if (compilationLimitsEnabled == false) {
-                compileRate = SCRIPT_COMPILATION_RATE_ZERO;
-            } else if (rateSetting.existsOrFallbackExists(settings)) {
-                compileRate = rateSetting.get(settings);
-            } else {
-                compileRate = context.maxCompilationRateDefault;
+        // 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) {
+            // General compilation rate changed, that setting is the only dynamically updated general setting
+            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<String, ScriptCache> contextCache = new HashMap<>(contexts.size());
+        contexts.forEach((k, v) -> contextCache.put(k, contextCache(settings, v)));
+        return new CacheHolder(contextCache);
+    }
+
+    ScriptCache contextCache(Settings settings, ScriptContext<?> context) {
+        Setting<Integer> cacheSizeSetting = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context.name);
+        int cacheSize = cacheSizeSetting.existsOrFallbackExists(settings) ? cacheSizeSetting.get(settings) : context.cacheSizeDefault;
 
-            Setting<TimeValue> cacheExpire = SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(name);
-            Setting<Integer> cacheSize = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(name);
+        Setting<TimeValue> cacheExpireSetting = SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context.name);
+        TimeValue cacheExpire = cacheExpireSetting.existsOrFallbackExists(settings) ?
+            cacheExpireSetting.get(settings) : context.cacheExpireDefault;
 
-            return new ScriptCache(cacheSize.existsOrFallbackExists(settings) ? cacheSize.get(settings) : context.cacheSizeDefault,
-                                   cacheExpire.existsOrFallbackExists(settings) ? cacheExpire.get(settings) : context.cacheExpireDefault,
-                                   compileRate,
-                                   SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context.name).getKey());
+        Setting<Tuple<Integer, TimeValue>> rateSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context.name);
+        Tuple<Integer, TimeValue> rate = null;
+        if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings) || compilationLimitsEnabled() == false) {
+            rate = SCRIPT_COMPILATION_RATE_ZERO;
+        } else if (rateSetting.existsOrFallbackExists(settings)) {
+            rate = rateSetting.get(settings);
+        } else {
+            rate = context.maxCompilationRateDefault;
         }
 
-        /**
-         * Returns a CacheHolder with the given settings.  Flips between general and context caches if necessary.  Creates new general
-         * cache if in general cache mode and {@code script.max_compilations_rate} has changed to any value other than {@code use-context}.
-         */
-        CacheHolder withUpdatedCacheSettings(Settings settings) {
-            if (SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE)) {
-                if (general != null) {
-                    // Flipping to context specific
-                    logger.debug("Switching to context cache from general cache");
-                    return new CacheHolder(settings, contexts, compilationLimitsEnabled);
-                }
-            } else if (general == null) {
-                // Flipping to general
-                logger.debug("Switching from context cache to general cache");
-                return new CacheHolder(settings, contexts, compilationLimitsEnabled);
-            } else if (general.rate.equals(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings)) == false) {
-                // General compilation rate changed, that setting is the only dynamically updated general setting
-                logger.debug("General compilation rate changed from [" + general.rate + "] to [" +
-                    SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings) + "], creating new general cache");
-                return new CacheHolder(settings, contexts, compilationLimitsEnabled);
-            }
+        return new ScriptCache(cacheSize, cacheExpire, rate, rateSetting.getKey());
+    }
+
+    /**
+     * Container for the ScriptCache(s).  This class operates in two modes:
+     * 1) general mode, if the general script cache is configured.  There are no context caches in this case.
+     * 2) context mode, if the context script cache is configured.  There is no general cache in this case.
+     */
+    static class CacheHolder {
+        final ScriptCache general;
+        final Map<String, AtomicReference<ScriptCache>> contextCache;
+
+        CacheHolder(int cacheMaxSize, TimeValue cacheExpire, Tuple<Integer, TimeValue> maxCompilationRate, String contextRateSetting) {
+            contextCache = null;
+            general = new ScriptCache(cacheMaxSize, cacheExpire, maxCompilationRate, contextRateSetting);
+        }
 
-            // no-op change, this is possible when context settings change while in context mode
-            return this;
+        CacheHolder(Map<String, ScriptCache> context) {
+            Map<String, AtomicReference<ScriptCache>> refs = new HashMap<>(context.size());
+            context.forEach((k, v) -> refs.put(k, new AtomicReference<>(v)));
+            contextCache = Collections.unmodifiableMap(refs);
+            general = null;
         }
 
         /**
@@ -688,25 +693,25 @@ public class ScriptService implements Closeable, ClusterStateApplier {
                 return new ScriptCacheStats(general.stats());
             }
             Map<String, ScriptStats> context = new HashMap<>(contextCache.size());
-            for (ScriptContext<?> ctx: contexts) {
-                context.put(ctx.name, contextCache.get(ctx.name).get().stats());
+            for (String name: contextCache.keySet()) {
+                context.put(name, contextCache.get(name).get().stats());
             }
             return new ScriptCacheStats(context);
         }
 
         /**
-         * Update settings for the context cache, if we're in the context cache mode otherwise no-op.
+         * Update a single context cache if we're in the context cache mode otherwise no-op.
          */
-        void updateContextSettings(Settings settings, ScriptContext<?> context) {
+        void set(String name, ScriptCache cache) {
             if (general != null) {
                 return;
             }
-            AtomicReference<ScriptCache> ref = contextCache.get(context.name);
-            assert ref != null : "expected script cache to exist for context [" + context.name + "]";
-            ScriptCache cache = ref.get();
-            assert cache != null : "expected script cache to be non-null for context [" + context.name + "]";
-            ref.set(contextFromSettings(settings, context, compilationLimitsEnabled));
-            logger.debug("Replaced context [" + context.name + "] with new settings");
+            AtomicReference<ScriptCache> ref = contextCache.get(name);
+            assert ref != null : "expected script cache to exist for context [" + name + "]";
+            ScriptCache oldCache = ref.get();
+            assert oldCache != null : "expected script cache to be non-null for context [" + name + "]";
+            ref.set(cache);
+            logger.debug("Replaced context [" + name + "] with new settings");
         }
     }
 }

+ 0 - 2
server/src/test/java/org/elasticsearch/indices/IndicesServiceCloseTests.java

@@ -39,7 +39,6 @@ import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService;
 import org.elasticsearch.node.MockNode;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.node.NodeValidationException;
-import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.InternalSettingsPlugin;
 import org.elasticsearch.test.InternalTestCluster;
@@ -69,7 +68,6 @@ public class IndicesServiceCloseTests extends ESTestCase {
             .put(Environment.PATH_REPO_SETTING.getKey(), tempDir.resolve("repo"))
             .put(Environment.PATH_SHARED_DATA_SETTING.getKey(), createTempDir().getParent())
             .put(Node.NODE_NAME_SETTING.getKey(), nodeName)
-            .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "1000/1m")
             .put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 1) // limit the number of threads created
             .put("transport.type", getTestTransportType())
             .put(Node.NODE_DATA_SETTING.getKey(), true)

+ 161 - 203
server/src/test/java/org/elasticsearch/script/ScriptServiceTests.java

@@ -28,6 +28,7 @@ import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -37,25 +38,20 @@ import org.elasticsearch.test.ESTestCase;
 import org.junit.Before;
 
 import java.io.IOException;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.BiFunction;
 import java.util.function.Function;
-import java.util.stream.Collectors;
 
+import static org.elasticsearch.script.ScriptService.CacheHolder;
 import static org.elasticsearch.script.ScriptService.MAX_COMPILATION_RATE_FUNCTION;
 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;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.notNullValue;
@@ -234,26 +230,31 @@ public class ScriptServiceTests extends ESTestCase {
     }
 
     public void testCompilationStatsOnCacheHit() throws IOException {
-        Settings.Builder builder = Settings.builder();
-        builder.put(SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), 1);
+        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());
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_CACHE_SIZE_SETTING,
+            SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
     }
 
     public void testIndexedScriptCountedInCompilationStats() throws IOException {
         buildScriptService(Settings.EMPTY);
-        scriptService.compile(new Script(ScriptType.STORED, null, "script", Collections.emptyMap()), randomFrom(contexts.values()));
+        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().getGeneralStats().getCompilations());
+        assertEquals(1L, scriptService.cacheStats().getContextStats().get(ctx.name).getCompilations());
     }
 
     public void testCacheEvictionCountedInCacheEvictionsStats() 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()));
@@ -261,6 +262,8 @@ public class ScriptServiceTests extends ESTestCase {
         assertEquals(2L, scriptService.cacheStats().getGeneralStats().getCompilations());
         assertEquals(1L, scriptService.stats().getCacheEvictions());
         assertEquals(1L, scriptService.cacheStats().getGeneralStats().getCacheEvictions());
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_CACHE_SIZE_SETTING,
+            SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
     }
 
     public void testContextCacheStats() throws IOException {
@@ -274,7 +277,6 @@ public class ScriptServiceTests extends ESTestCase {
             ".max_compilations_rate] setting"
         );
         buildScriptService(Settings.builder()
-            .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY)
             .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextA.name).getKey(), 1)
             .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(contextA.name).getKey(), aRate)
             .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(contextB.name).getKey(), 2)
@@ -383,6 +385,7 @@ public class ScriptServiceTests extends ESTestCase {
     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 " +
@@ -392,6 +395,7 @@ public class ScriptServiceTests extends ESTestCase {
 
         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());
         });
 
@@ -402,6 +406,7 @@ public class ScriptServiceTests extends ESTestCase {
 
         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());
         });
 
@@ -415,30 +420,8 @@ public class ScriptServiceTests extends ESTestCase {
                 .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("ingest").getKey(), "5m")
                 .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("field").getKey(), 123)
                 .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("score").getKey(), "50/5m")
-                .put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY).build());
-    }
-
-    public void testFallbackContextSettings() {
-        int cacheSizeBackup = randomIntBetween(0, 1024);
-        int cacheSizeFoo = randomValueOtherThan(cacheSizeBackup, () -> randomIntBetween(0, 1024));
-
-        String cacheExpireBackup = randomTimeValue(1, 1000, "h");
-        TimeValue cacheExpireBackupParsed = TimeValue.parseTimeValue(cacheExpireBackup, "");
-        String cacheExpireFoo = randomValueOtherThan(cacheExpireBackup, () -> randomTimeValue(1, 1000, "h"));
-        TimeValue cacheExpireFooParsed = TimeValue.parseTimeValue(cacheExpireFoo, "");
-
-        Settings s = Settings.builder()
-            .put(SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), cacheSizeBackup)
-            .put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo").getKey(), cacheSizeFoo)
-            .put(ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), cacheExpireBackup)
-            .put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo").getKey(), cacheExpireFoo)
-            .build();
-
-        assertEquals(cacheSizeFoo, ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("foo").get(s).intValue());
-        assertEquals(cacheSizeBackup, ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace("bar").get(s).intValue());
-
-        assertEquals(cacheExpireFooParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("foo").get(s));
-        assertEquals(cacheExpireBackupParsed, ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace("bar").get(s));
+                .build());
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
     }
 
     public void testUseContextSettingValue() {
@@ -455,66 +438,44 @@ public class ScriptServiceTests extends ESTestCase {
         });
 
         assertEquals("parameter must contain a positive integer and a timevalue, i.e. 10/1m, but was [use-context]", illegal.getMessage());
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
     }
 
-    public void testCacheHolderGeneralConstructor() {
+    public void testCacheHolderGeneralConstructor() throws IOException {
         String compilationRate = "77/5m";
         Tuple<Integer, TimeValue> rate = ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(compilationRate);
-        boolean compilationLimitsEnabled = true;
-        ScriptService.CacheHolder holder = new ScriptService.CacheHolder(
-            Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate).build(),
-            Set.of(newContext("foo")),
-            compilationLimitsEnabled
+        buildScriptService(
+            Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate).build()
         );
 
-        assertNotNull(holder.general);
-        assertNull(holder.contextCache);
-        assertEquals(holder.general.rate, rate);
-
-        compilationLimitsEnabled = false;
-        holder = new ScriptService.CacheHolder(
-            Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate).build(),
-            new HashSet<>(Collections.singleton(newContext("foo"))),
-            compilationLimitsEnabled
-        );
+        CacheHolder holder = scriptService.cacheHolder.get();
 
         assertNotNull(holder.general);
         assertNull(holder.contextCache);
-        assertEquals(holder.general.rate, new Tuple<>(0, TimeValue.ZERO));
+        assertEquals(holder.general.rate, rate);
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
     }
 
-    public void testCacheHolderContextConstructor() {
-        String fooCompilationRate = "77/5m";
-        String barCompilationRate = "78/6m";
-        boolean compilationLimitsEnabled = true;
+    public void testCacheHolderContextConstructor() throws IOException {
+        String a = randomFrom(contexts.keySet());
+        String b = randomValueOtherThan(a, () -> randomFrom(contexts.keySet()));
+        String c = randomValueOtherThanMany(Set.of(a, b)::contains, () -> randomFrom(contexts.keySet()));
+        String aCompilationRate = "77/5m";
+        String bCompilationRate = "78/6m";
 
-        Settings s = Settings.builder()
-            .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY)
-            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("foo").getKey(), fooCompilationRate)
-            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("bar").getKey(), barCompilationRate)
-            .build();
-        Collection<ScriptContext<?>> contexts = Set.of(newContext("foo"), newContext("bar"), newContext("baz"));
-        ScriptService.CacheHolder holder = new ScriptService.CacheHolder(s, contexts, compilationLimitsEnabled);
-
-        assertNull(holder.general);
-        assertNotNull(holder.contextCache);
-        assertEquals(3, holder.contextCache.size());
-        assertEquals(contexts.stream().map(c -> c.name).collect(Collectors.toSet()), holder.contextCache.keySet());
-
-        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(fooCompilationRate), holder.contextCache.get("foo").get().rate);
-        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(barCompilationRate), holder.contextCache.get("bar").get().rate);
-        assertEquals(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getDefault(Settings.EMPTY),
-                     holder.contextCache.get("baz").get().rate);
+        buildScriptService(Settings.builder()
+            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a).getKey(), aCompilationRate)
+            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b).getKey(), bCompilationRate)
+            .build());
 
-        Tuple<Integer, TimeValue> zero = new Tuple<>(0, TimeValue.ZERO);
-        compilationLimitsEnabled = false;
-        holder = new ScriptService.CacheHolder(s, contexts, compilationLimitsEnabled);
+        assertNull(scriptService.cacheHolder.get().general);
+        assertNotNull(scriptService.cacheHolder.get().contextCache);
+        assertEquals(contexts.keySet(), scriptService.cacheHolder.get().contextCache.keySet());
 
-        assertNotNull(holder.contextCache);
-        assertEquals(3, holder.contextCache.size());
-        assertEquals(zero, holder.contextCache.get("foo").get().rate);
-        assertEquals(zero, holder.contextCache.get("bar").get().rate);
-        assertEquals(zero, holder.contextCache.get("baz").get().rate);
+        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(aCompilationRate),
+                     scriptService.cacheHolder.get().contextCache.get(a).get().rate);
+        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(bCompilationRate),
+                     scriptService.cacheHolder.get().contextCache.get(b).get().rate);
     }
 
     public void testCompilationRateUnlimitedContextOnly() throws IOException {
@@ -533,155 +494,156 @@ public class ScriptServiceTests extends ESTestCase {
                  ScriptService.UNLIMITED_COMPILATION_RATE_KEY)
             .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY)
             .build());
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
     }
 
-    public void testCacheHolderChangeSettings() {
-        String fooCompilationRate = "77/5m";
-        String barCompilationRate = "78/6m";
+    public void testDisableCompilationRateSetting() throws IOException {
+        IllegalArgumentException illegal = expectThrows(IllegalArgumentException.class, () -> {
+            buildScriptService(Settings.builder()
+                .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)
+                .build());
+        });
+        assertEquals("Cannot set custom context compilation rates [script.context.field.max_compilations_rate, " +
+                "script.context.ingest.max_compilations_rate] if compile rates disabled via " +
+                "[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 [Tuple [v1=76, v2=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_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
+    }
+
+    public void testCacheHolderChangeSettings() throws IOException {
+        Set<String> contextNames = contexts.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";
         Tuple<Integer, TimeValue> generalRate = MAX_COMPILATION_RATE_FUNCTION.apply(compilationRate);
 
         Settings s = Settings.builder()
             .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), compilationRate)
             .build();
-        Collection<ScriptContext<?>> contexts = Set.of(newContext("foo"), newContext("bar"), newContext("baz"),
-                                                       newContext("qux"));
-        ScriptService.CacheHolder holder = new ScriptService.CacheHolder(s, contexts, true);
 
-        assertNotNull(holder.general);
-        assertNull(holder.contextCache);
-        assertEquals(generalRate, holder.general.rate);
+        buildScriptService(s);
 
-        holder = holder.withUpdatedCacheSettings(Settings.builder()
-            .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), ScriptService.USE_CONTEXT_RATE_KEY)
-            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("foo").getKey(), fooCompilationRate)
-            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("bar").getKey(), barCompilationRate)
-            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("qux").getKey(),
-                 ScriptService.UNLIMITED_COMPILATION_RATE_KEY)
-            .build()
-        );
+        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);
 
-        assertNull(holder.general);
-        assertNotNull(holder.contextCache);
-        assertEquals(4, holder.contextCache.size());
-        assertEquals(contexts.stream().map(c -> c.name).collect(Collectors.toSet()), holder.contextCache.keySet());
-
-        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(fooCompilationRate), holder.contextCache.get("foo").get().rate);
-        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(barCompilationRate), holder.contextCache.get("bar").get().rate);
-        assertEquals(ScriptCache.UNLIMITED_COMPILATION_RATE, holder.contextCache.get("qux").get().rate);
-        assertEquals(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getDefault(Settings.EMPTY),
-            holder.contextCache.get("baz").get().rate);
-
-        holder.updateContextSettings(Settings.builder()
-                .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace("bar").getKey(), fooCompilationRate).build(),
-                newContext("bar")
+        scriptService.setCacheHolder(Settings.builder()
+                .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(a).getKey(), aRate)
+                .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b).getKey(), bRate)
+                .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(c).getKey(),
+                     ScriptService.UNLIMITED_COMPILATION_RATE_KEY)
+                .build()
         );
-        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(fooCompilationRate), holder.contextCache.get("bar").get().rate);
-
-        holder = holder.withUpdatedCacheSettings(s);
-        assertNotNull(holder.general);
-        assertNull(holder.contextCache);
-        assertEquals(generalRate, holder.general.rate);
 
-        holder = holder.withUpdatedCacheSettings(
-            Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), barCompilationRate).build()
+        assertNull(scriptService.cacheHolder.get().general);
+        assertNotNull(scriptService.cacheHolder.get().contextCache);
+        // get of missing context should be null
+        assertNull(scriptService.cacheHolder.get().get(
+            randomValueOtherThanMany(contexts.keySet()::contains, () -> randomAlphaOfLength(8)))
         );
-
-        assertNotNull(holder.general);
-        assertNull(holder.contextCache);
-        assertEquals(MAX_COMPILATION_RATE_FUNCTION.apply(barCompilationRate), holder.general.rate);
-
-        ScriptService.CacheHolder update = holder.withUpdatedCacheSettings(
-            Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), barCompilationRate).build()
+        assertEquals(contexts.keySet(), scriptService.cacheHolder.get().contextCache.keySet());
+
+        String d = randomValueOtherThanMany(Set.of(a, b, c)::contains, () -> randomFrom(contextNames));
+        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(aRate),
+                     scriptService.cacheHolder.get().contextCache.get(a).get().rate);
+        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(bRate),
+                     scriptService.cacheHolder.get().contextCache.get(b).get().rate);
+        assertEquals(ScriptCache.UNLIMITED_COMPILATION_RATE,
+                     scriptService.cacheHolder.get().contextCache.get(c).get().rate);
+
+        scriptService.cacheHolder.get().set(b, scriptService.contextCache(Settings.builder()
+                .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(b).getKey(), aRate).build(),
+            contexts.get(b)));
+        assertEquals(ScriptService.MAX_COMPILATION_RATE_FUNCTION.apply(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()
         );
-        assertSame(holder, update);
-    }
 
-    public void testFallbackToContextDefaults() {
-        Tuple<Integer, TimeValue> contextDefaultRate = new Tuple<>(randomIntBetween(10, 1024),
-                                                                   TimeValue.timeValueMinutes(randomIntBetween(10, 200)));
-        String name = "foo";
-        ScriptContext<?> foo = new ScriptContext<>(name,
-                                                   ScriptContextTests.DummyScript.Factory.class,
-                                                   randomIntBetween(1, 1024),
-                                                   TimeValue.timeValueMinutes(randomIntBetween(10, 200)),
-                                                   contextDefaultRate);
+        assertNotNull(scriptService.cacheHolder.get().general);
+        assertNull(scriptService.cacheHolder.get().contextCache);
+        assertEquals(MAX_COMPILATION_RATE_FUNCTION.apply(bRate), scriptService.cacheHolder.get().general.rate);
 
+        CacheHolder holder = scriptService.cacheHolder.get();
+        scriptService.setCacheHolder(
+            Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), bRate).build()
+        );
+        assertEquals(holder, scriptService.cacheHolder.get());
 
-        int generalCacheSize = randomIntBetween(1, 1024);
-        TimeValue generalExpire = TimeValue.timeValueMinutes(randomIntBetween(10, 200));
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
+    }
 
+    public void testFallbackToContextDefaults() throws IOException {
         String contextRateStr = randomIntBetween(10, 1024) + "/" +  randomIntBetween(10, 200) + "m";
-        Tuple<Integer, TimeValue>  contextRate = MAX_COMPILATION_RATE_FUNCTION.apply(contextRateStr);
+        Tuple<Integer, TimeValue> contextRate = MAX_COMPILATION_RATE_FUNCTION.apply(contextRateStr);
         int contextCacheSize = randomIntBetween(1, 1024);
         TimeValue contextExpire = TimeValue.timeValueMinutes(randomIntBetween(10, 200));
 
+        buildScriptService(
+            Settings.builder().put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "75/5m").build()
+        );
+
+        String name = "ingest";
+
         // Use context specific
-        ScriptService.CacheHolder contextCache = new ScriptService.CacheHolder(
-            Settings.builder()
-            .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextCacheSize)
-            .put(SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextExpire)
-            .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextRateStr)
-            .put(SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), generalCacheSize)
-            .put(SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), generalExpire)
-            .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY)
-            .build(),
-            List.of(foo),
-            true);
-
-        assertNotNull(contextCache.contextCache);
-        assertNotNull(contextCache.contextCache.get(name));
-        assertNotNull(contextCache.contextCache.get(name).get());
-
-        assertEquals(contextRate, contextCache.contextCache.get(name).get().rate);
-        assertEquals(contextCacheSize, contextCache.contextCache.get(name).get().cacheSize);
-        assertEquals(contextExpire, contextCache.contextCache.get(name).get().cacheExpire);
-
-        // Fallback to general
-        contextCache = new ScriptService.CacheHolder(
-            Settings.builder()
-            .put(SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), generalCacheSize)
-            .put(SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(), generalExpire)
-            .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY)
-            .build(),
-            List.of(foo),
-            true);
+        scriptService.setCacheHolder(Settings.builder()
+                .put(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextCacheSize)
+                .put(SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextExpire)
+                .put(SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(name).getKey(), contextRateStr)
+                .build()
+        );
 
-        assertNotNull(contextCache.contextCache);
-        assertNotNull(contextCache.contextCache.get(name));
-        assertNotNull(contextCache.contextCache.get(name).get());
+        ScriptService.CacheHolder holder = scriptService.cacheHolder.get();
+        assertNotNull(holder.contextCache);
+        assertNotNull(holder.contextCache.get(name));
+        assertNotNull(holder.contextCache.get(name).get());
 
-        assertEquals(contextDefaultRate, contextCache.contextCache.get(name).get().rate);
-        assertEquals(generalCacheSize, contextCache.contextCache.get(name).get().cacheSize);
-        assertEquals(generalExpire, contextCache.contextCache.get(name).get().cacheExpire);
+        assertEquals(contextRate, holder.contextCache.get(name).get().rate);
+        assertEquals(contextCacheSize, holder.contextCache.get(name).get().cacheSize);
+        assertEquals(contextExpire, holder.contextCache.get(name).get().cacheExpire);
 
+        ScriptContext<?> ingest = contexts.get(name);
         // Fallback to context defaults
-        contextCache = new ScriptService.CacheHolder(
-            Settings.builder()
-            .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY)
-            .build(),
-            List.of(foo),
-            true);
+        buildScriptService(Settings.EMPTY);
 
-        assertNotNull(contextCache.contextCache);
-        assertNotNull(contextCache.contextCache.get(name));
-        assertNotNull(contextCache.contextCache.get(name).get());
+        holder = scriptService.cacheHolder.get();
+        assertNotNull(holder.contextCache);
+        assertNotNull(holder.contextCache.get(name));
+        assertNotNull(holder.contextCache.get(name).get());
 
-        assertEquals(contextDefaultRate, contextCache.contextCache.get(name).get().rate);
-        assertEquals(foo.cacheSizeDefault, contextCache.contextCache.get(name).get().cacheSize);
-        assertEquals(foo.cacheExpireDefault, contextCache.contextCache.get(name).get().cacheExpire);
+        assertEquals(ingest.maxCompilationRateDefault, holder.contextCache.get(name).get().rate);
+        assertEquals(ingest.cacheSizeDefault, holder.contextCache.get(name).get().cacheSize);
+        assertEquals(ingest.cacheExpireDefault, holder.contextCache.get(name).get().cacheExpire);
 
-        // Use context specific for ingest
-        contextCache = new ScriptService.CacheHolder(
-            Settings.builder()
-                .put(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), USE_CONTEXT_RATE_KEY)
-                .build(),
-            List.of(foo, IngestScript.CONTEXT, IngestConditionalScript.CONTEXT),
-            true);
-        assertEquals(new Tuple<>(375, TimeValue.timeValueMinutes(5)),
-                     contextCache.contextCache.get("ingest").get().rate);
-        assertEquals(200, contextCache.contextCache.get("ingest").get().cacheSize);
-        assertEquals(TimeValue.timeValueMillis(0), contextCache.contextCache.get("ingest").get().cacheExpire);
+        assertSettingDeprecationsAndWarnings(new Setting<?>[]{SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING});
     }
 
     private void assertCompileRejected(String lang, String script, ScriptType scriptType, ScriptContext scriptContext) {
@@ -700,8 +662,4 @@ public class ScriptServiceTests extends ESTestCase {
                 notNullValue()
         );
     }
-
-    ScriptContext<ScriptContextTests.DummyScript.Factory> newContext(String name) {
-        return new ScriptContext<>(name, ScriptContextTests.DummyScript.Factory.class);
-    }
 }

+ 7 - 3
test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java

@@ -98,6 +98,7 @@ 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.test.disruption.ServiceDisruptionScheme;
@@ -495,11 +496,14 @@ public final class InternalTestCluster extends TestCluster {
         }
 
         if (random.nextBoolean()) {
-            builder.put(ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.getKey(), RandomNumbers.randomIntBetween(random, 0, 2000));
+            String ctx = randomFrom(random, ScriptModule.CORE_CONTEXTS.keySet());
+            builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(ctx).getKey(),
+                        RandomNumbers.randomIntBetween(random, 0, 2000));
         }
         if (random.nextBoolean()) {
-            builder.put(ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.getKey(),
-                    timeValueMillis(RandomNumbers.randomIntBetween(random, 750, 10000000)).getStringRep());
+            String ctx = randomFrom(random, ScriptModule.CORE_CONTEXTS.keySet());
+            builder.put(ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(ctx).getKey(),
+                        timeValueMillis(RandomNumbers.randomIntBetween(random, 750, 10000000)).getStringRep());
         }
 
         return builder.build();

+ 0 - 2
x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/CcrIntegTestCase.java

@@ -61,7 +61,6 @@ import org.elasticsearch.license.LicenseService;
 import org.elasticsearch.license.LicensesMetadata;
 import org.elasticsearch.persistent.PersistentTasksCustomMetadata;
 import org.elasticsearch.plugins.Plugin;
-import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.snapshots.RestoreInfo;
 import org.elasticsearch.snapshots.RestoreService;
@@ -220,7 +219,6 @@ public abstract class CcrIntegTestCase extends ESTestCase {
         builder.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "1b");
         builder.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "1b");
         builder.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "1b");
-        builder.put(ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey(), "2048/1m");
         // wait short time for other active shards before actually deleting, default 30s not needed in tests
         builder.put(IndicesStore.INDICES_STORE_DELETE_SHARD_TIMEOUT.getKey(), new TimeValue(1, TimeUnit.SECONDS));
         builder.putList(DISCOVERY_SEED_HOSTS_SETTING.getKey()); // empty list disables a port scan for other nodes