elastic · spinscale · Sep 1, 2017 · Aug 28, 2017 · Aug 29, 2017 · Aug 30, 2017
diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy
@@ -329,7 +329,11 @@ class ClusterFormationTasks {
             esConfig['cluster.routing.allocation.disk.watermark.flood_stage'] = '1b'
         }
         // increase script compilation limit since tests can rapid-fire script compilations
-        esConfig['script.max_compilations_per_minute'] = 2048
+        if (Version.fromString(node.nodeVersion).major > 6) {
+          esConfig['script.max_compilations_rate'] = 2048
+        } else {
+          esConfig['script.max_compilations_per_minute'] = 2048
+        }
         esConfig.putAll(node.config.settings)
 
         Task writeConfig = project.tasks.create(name: name, type: DefaultTask, dependsOn: setup)

diff --git a/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/core/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java
@@ -315,7 +315,7 @@ public void apply(Settings value, Settings current, Settings previous) {
                     ScriptService.SCRIPT_CACHE_SIZE_SETTING,
                     ScriptService.SCRIPT_CACHE_EXPIRE_SETTING,
                     ScriptService.SCRIPT_MAX_SIZE_IN_BYTES,
-                    ScriptService.SCRIPT_MAX_COMPILATIONS_PER_MINUTE,
+                    ScriptService.SCRIPT_MAX_COMPILATIONS_RATE,
                     ScriptService.TYPES_ALLOWED_SETTING,
                     ScriptService.CONTEXTS_ALLOWED_SETTING,
                     IndicesService.INDICES_CACHE_CLEAN_INTERVAL_SETTING,

diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java
@@ -66,8 +66,8 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
         Setting.positiveTimeSetting("script.cache.expire", TimeValue.timeValueMillis(0), Property.NodeScope);
     public static final Setting<Integer> SCRIPT_MAX_SIZE_IN_BYTES =
         Setting.intSetting("script.max_size_in_bytes", 65535, Property.NodeScope);
-    public static final Setting<Integer> SCRIPT_MAX_COMPILATIONS_PER_MINUTE =
-        Setting.intSetting("script.max_compilations_per_minute", 15, 0, Property.Dynamic, Property.NodeScope);
+    public static final Setting<Integer> SCRIPT_MAX_COMPILATIONS_RATE =
+        Setting.intSetting("script.max_compilations_rate", 75, 0, Property.Dynamic, Property.NodeScope);
 
     public static final String ALLOW_NONE = "none";
 
@@ -88,9 +88,9 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
 
     private ClusterState clusterState;
 
-    private int totalCompilesPerMinute;
+    private int totalCompilesPerFiveMinutes;
     private long lastInlineCompileTime;
-    private double scriptsPerMinCounter;
+    private double scriptsPerFiveMinsCounter;
     private double compilesAllowedPerNano;
 
     public ScriptService(Settings settings, Map<String, ScriptEngine> engines, Map<String, ScriptContext<?>> contexts) {
@@ -188,11 +188,11 @@ public ScriptService(Settings settings, Map<String, ScriptEngine> engines, Map<S
         this.cache = cacheBuilder.removalListener(new ScriptCacheRemovalListener()).build();
 
         this.lastInlineCompileTime = System.nanoTime();
-        this.setMaxCompilationsPerMinute(SCRIPT_MAX_COMPILATIONS_PER_MINUTE.get(settings));
+        this.setMaxCompilationRate(SCRIPT_MAX_COMPILATIONS_RATE.get(settings));
     }
 
     void registerClusterSettingsListeners(ClusterSettings clusterSettings) {
-        clusterSettings.addSettingsUpdateConsumer(SCRIPT_MAX_COMPILATIONS_PER_MINUTE, this::setMaxCompilationsPerMinute);
+        clusterSettings.addSettingsUpdateConsumer(SCRIPT_MAX_COMPILATIONS_RATE, this::setMaxCompilationRate);
     }
 
     @Override
@@ -208,11 +208,16 @@ private ScriptEngine getEngine(String lang) {
         return scriptEngine;
     }
 
-    void setMaxCompilationsPerMinute(Integer newMaxPerMinute) {
-        this.totalCompilesPerMinute = newMaxPerMinute;
+    /**
+     * This configures the maximum script compilations per five minute window.
+     *
+     * @param newRate the new expected maximum number of compilations per five minute window
+     */
+    void setMaxCompilationRate(Integer newRate) {
+        this.totalCompilesPerFiveMinutes = newRate;
         // Reset the counter to allow new compilations
-        this.scriptsPerMinCounter = totalCompilesPerMinute;
-        this.compilesAllowedPerNano = ((double) totalCompilesPerMinute) / TimeValue.timeValueMinutes(1).nanos();
+        this.scriptsPerFiveMinsCounter = totalCompilesPerFiveMinutes;
+        this.compilesAllowedPerNano = ((double) totalCompilesPerFiveMinutes) / TimeValue.timeValueMinutes(5).nanos();
     }
 
     /**
@@ -325,22 +330,22 @@ void checkCompilationLimit() {
         long timePassed = now - lastInlineCompileTime;
         lastInlineCompileTime = now;
 
-        scriptsPerMinCounter += (timePassed) * compilesAllowedPerNano;
+        scriptsPerFiveMinsCounter += (timePassed) * compilesAllowedPerNano;
 
         // It's been over the time limit anyway, readjust the bucket to be level
-        if (scriptsPerMinCounter > totalCompilesPerMinute) {
-            scriptsPerMinCounter = totalCompilesPerMinute;
+        if (scriptsPerFiveMinsCounter > totalCompilesPerFiveMinutes) {
+            scriptsPerFiveMinsCounter = totalCompilesPerFiveMinutes;
         }
 
         // If there is enough tokens in the bucket, allow the request and decrease the tokens by 1
-        if (scriptsPerMinCounter >= 1) {
-            scriptsPerMinCounter -= 1.0;
+        if (scriptsPerFiveMinsCounter >= 1) {
+            scriptsPerFiveMinsCounter -= 1.0;
         } else {
             scriptMetrics.onCompilationLimit();
             // Otherwise reject the request
-            throw new CircuitBreakingException("[script] Too many dynamic script compilations within one minute, max: [" +
-                            totalCompilesPerMinute + "/min]; please use on-disk, indexed, or scripts with parameters instead; " +
-                            "this limit can be changed by the [" + SCRIPT_MAX_COMPILATIONS_PER_MINUTE.getKey() + "] setting");
+            throw new CircuitBreakingException("[script] Too many dynamic script compilations within five minute window, max: [" +
+                    totalCompilesPerFiveMinutes + "/min]; please use indexed, or scripts with parameters instead; " +
+                            "this limit can be changed by the [" + SCRIPT_MAX_COMPILATIONS_RATE.getKey() + "] setting");
         }
     }
 

diff --git a/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java b/core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java
@@ -55,7 +55,7 @@ public class ScriptServiceTests extends ESTestCase {
     public void setup() throws IOException {
         baseSettings = Settings.builder()
                 .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
-                .put(ScriptService.SCRIPT_MAX_COMPILATIONS_PER_MINUTE.getKey(), 10000)
+                .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE.getKey(), 10000)
                 .build();
         Map<String, Function<Map<String, Object>, Object>> scripts = new HashMap<>();
         for (int i = 0; i < 20; ++i) {
@@ -82,24 +82,26 @@ StoredScriptSource getScriptFromClusterState(String id) {
         };
     }
 
+    // even though circuit breaking is allowed to be configured per minute, we actually weigh this over five minutes
+    // simply by multiplying by five, so even setting it to one, requires five compilations to break
     public void testCompilationCircuitBreaking() throws Exception {
         buildScriptService(Settings.EMPTY);
-        scriptService.setMaxCompilationsPerMinute(1);
+        scriptService.setMaxCompilationRate(1);
         scriptService.checkCompilationLimit(); // should pass
         expectThrows(CircuitBreakingException.class, () -> scriptService.checkCompilationLimit());
-        scriptService.setMaxCompilationsPerMinute(2);
+        scriptService.setMaxCompilationRate(2);
         scriptService.checkCompilationLimit(); // should pass
         scriptService.checkCompilationLimit(); // should pass
         expectThrows(CircuitBreakingException.class, () -> scriptService.checkCompilationLimit());
         int count = randomIntBetween(5, 50);
-        scriptService.setMaxCompilationsPerMinute(count);
+        scriptService.setMaxCompilationRate(count);
         for (int i = 0; i < count; i++) {
             scriptService.checkCompilationLimit(); // should pass
         }
         expectThrows(CircuitBreakingException.class, () -> scriptService.checkCompilationLimit());
-        scriptService.setMaxCompilationsPerMinute(0);
+        scriptService.setMaxCompilationRate(0);
         expectThrows(CircuitBreakingException.class, () -> scriptService.checkCompilationLimit());
-        scriptService.setMaxCompilationsPerMinute(Integer.MAX_VALUE);
+        scriptService.setMaxCompilationRate(Integer.MAX_VALUE);
         int largeLimit = randomIntBetween(1000, 10000);
         for (int i = 0; i < largeLimit; i++) {
             scriptService.checkCompilationLimit();

diff --git a/docs/build.gradle b/docs/build.gradle
@@ -44,7 +44,7 @@ buildRestTests.expectedUnconvertedCandidates = [
 ]
 
 integTestCluster {
-  setting 'script.max_compilations_per_minute', '1000'
+  setting 'script.max_compilations_rate', '1000'
   /* Enable regexes in painless so our tests don't complain about example
    * snippets that use them. */
   setting 'script.painless.regex.enabled', 'true'

diff --git a/docs/reference/modules/indices/circuit_breaker.asciidoc b/docs/reference/modules/indices/circuit_breaker.asciidoc
@@ -83,7 +83,7 @@ within a period of time.
 See the "prefer-parameters" section of the <<modules-scripting-using,scripting>>
 documentation for more information.
 
-`script.max_compilations_per_minute`::
+`script.max_compilations_rate`::
 
-    Limit for the number of unique dynamic scripts within a minute that are
-    allowed to be compiled. Defaults to 15.
+    Limit for the number of unique dynamic scripts within a five minute interval
+    that are allowed to be compiled. Defaults to 75.
diff --git a/docs/reference/modules/scripting/using.asciidoc b/docs/reference/modules/scripting/using.asciidoc
@@ -104,7 +104,7 @@ 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_per_minute`.
+`script.max_compilations_rate`.
 
 ========================================
 

diff --git a/modules/lang-expression/build.gradle b/modules/lang-expression/build.gradle
@@ -36,5 +36,5 @@ dependencyLicenses {
 }
 
 integTestCluster {
-  setting 'script.max_compilations_per_minute', '1000'
+  setting 'script.max_compilations_rate', '1000'
 }
diff --git a/modules/lang-mustache/build.gradle b/modules/lang-mustache/build.gradle
@@ -28,5 +28,5 @@ dependencies {
 }
 
 integTestCluster {
-  setting 'script.max_compilations_per_minute', '1000'
+  setting 'script.max_compilations_rate', '1000'
 }
diff --git a/modules/lang-painless/build.gradle b/modules/lang-painless/build.gradle
@@ -38,7 +38,7 @@ test {
 }
 
 integTestCluster {
-  setting 'script.max_compilations_per_minute', '1000'
+  setting 'script.max_compilations_rate', '1000'
 }
 
 /* Build Javadoc for the Java classes in Painless's public API that are in the

diff --git a/qa/smoke-test-ingest-with-all-dependencies/build.gradle b/qa/smoke-test-ingest-with-all-dependencies/build.gradle
@@ -30,5 +30,5 @@ dependencies {
 
 integTestCluster {
     plugin ':plugins:ingest-geoip'
-    setting 'script.max_compilations_per_minute', '1000'
+    setting 'script.max_compilations_rate', '1000'
 }
diff --git a/qa/smoke-test-reindex-with-all-modules/build.gradle b/qa/smoke-test-reindex-with-all-modules/build.gradle
@@ -21,7 +21,7 @@ apply plugin: 'elasticsearch.standalone-rest-test'
 apply plugin: 'elasticsearch.rest-test'
 
 integTestCluster {
-  setting 'script.max_compilations_per_minute', '1000'
+  setting 'script.max_compilations_rate', '1000'
   // Whitelist reindexing from the local node so we can test it.
   setting 'reindex.remote.whitelist', '127.0.0.1:*'
-}
+}
diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java
@@ -1694,7 +1694,7 @@ protected Settings nodeSettings(int nodeOrdinal) {
             .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "1b")
             .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_HIGH_DISK_WATERMARK_SETTING.getKey(), "1b")
             .put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_DISK_FLOOD_STAGE_WATERMARK_SETTING.getKey(), "1b")
-            .put(ScriptService.SCRIPT_MAX_COMPILATIONS_PER_MINUTE.getKey(), 2048)
+            .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE.getKey(), 2048)
             // by default we never cache below 10k docs in a segment,
             // bypass this limit so that caching gets some testing in
             // integration tests that usually create few documents

diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java
@@ -170,7 +170,7 @@ private Node newNode() {
             // This needs to tie into the ESIntegTestCase#indexSettings() method
             .put(Environment.PATH_SHARED_DATA_SETTING.getKey(), createTempDir().getParent())
             .put("node.name", "node_s_0")
-            .put(ScriptService.SCRIPT_MAX_COMPILATIONS_PER_MINUTE.getKey(), 1000)
+            .put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE.getKey(), 1000)
             .put(EsExecutors.PROCESSORS_SETTING.getKey(), 1) // limit the number of threads created
             .put(NetworkModule.HTTP_ENABLED.getKey(), false)
             .put("transport.type", getTestTransportType())

diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
@@ -337,7 +337,7 @@ public InternalTestCluster(long clusterSeed, Path baseDir,
         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");
         // Some tests make use of scripting quite a bit, so increase the limit for integration tests
-        builder.put(ScriptService.SCRIPT_MAX_COMPILATIONS_PER_MINUTE.getKey(), 1000);
+        builder.put(ScriptService.SCRIPT_MAX_COMPILATIONS_RATE.getKey(), 1000);
         if (TEST_NIGHTLY) {
             builder.put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_INCOMING_RECOVERIES_SETTING.getKey(), RandomNumbers.randomIntBetween(random, 5, 10));
             builder.put(ThrottlingAllocationDecider.CLUSTER_ROUTING_ALLOCATION_NODE_CONCURRENT_OUTGOING_RECOVERIES_SETTING.getKey(), RandomNumbers.randomIntBetween(random, 5, 10));