From d1d29c22c0b9b72ee4ccbd0fd1a7ab76714ea252 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 17 Aug 2020 16:46:50 -0400 Subject: [PATCH 1/2] Opt date valued script fields out of rate limit This excuses script fields from the script compilation rate limiting. It'd be fairly sad to prevent mapping updates because of the rate limit. And we don't expect folks to add a zillion fields. On the other hand, once we allow these scripts on the `_search` request we might indeed want them to be considered in the limit. But we don't support that yet and we can deal with that when we get there. --- .../org/elasticsearch/script/ScriptCache.java | 2 +- .../AbstractScriptFieldScript.java | 22 ++++++++ .../runtimefields/DateScriptFieldScript.java | 2 +- .../DateScriptFieldScriptTests.java | 37 ++++++++++++++ .../xpack/runtimefields/TestScriptEngine.java | 50 +++++++++++++++++++ .../mapper/RuntimeScriptFieldMapperTests.java | 37 ++------------ 6 files changed, 116 insertions(+), 34 deletions(-) create mode 100644 x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScriptTests.java create mode 100644 x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/TestScriptEngine.java diff --git a/server/src/main/java/org/elasticsearch/script/ScriptCache.java b/server/src/main/java/org/elasticsearch/script/ScriptCache.java index e156cfa125b19..e7f1245d0afd9 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptCache.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptCache.java @@ -42,7 +42,7 @@ public class ScriptCache { private static final Logger logger = LogManager.getLogger(ScriptService.class); - static final CompilationRate UNLIMITED_COMPILATION_RATE = new CompilationRate(0, TimeValue.ZERO); + public static final CompilationRate UNLIMITED_COMPILATION_RATE = new CompilationRate(0, TimeValue.ZERO); private final Cache cache; private final ScriptMetrics scriptMetrics; diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java index ec49d38ef0a30..9db19ec19ac08 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java @@ -9,16 +9,38 @@ import org.apache.lucene.index.LeafReaderContext; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.script.AggregationScript; +import org.elasticsearch.script.ScriptCache; +import org.elasticsearch.script.ScriptContext; import org.elasticsearch.search.lookup.LeafSearchLookup; import org.elasticsearch.search.lookup.SearchLookup; import java.util.Map; +import static org.elasticsearch.common.unit.TimeValue.timeValueMillis; + /** * Abstract base for scripts to execute to build scripted fields. Inspired by * {@link AggregationScript} but hopefully with less historical baggage. */ public abstract class AbstractScriptFieldScript { + public static ScriptContext newContext(String name, Class factoryClass) { + return new ScriptContext(name + "_script_field", factoryClass, + /* + * In an ideal world we wouldn't need the script cache at all + * because we have a hard reference to the script. The trouble + * is that we compile the scripts a few times when performing + * a mapping update. This is unfortunate, but we rely on the + * cache to speed this up. + */ + 100, timeValueMillis(0), + /* + * Disable compilation rate limits for scripted fields so we + * don't prevent mapping updates because we've performed too + * many recently. That'd just be lame. + */ + ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple()); + } + private final Map params; private final LeafSearchLookup leafSearchLookup; diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScript.java index 8ce9ffe7059ed..bdb9e687fe1fa 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScript.java @@ -21,7 +21,7 @@ import java.util.Map; public abstract class DateScriptFieldScript extends AbstractLongScriptFieldScript { - public static final ScriptContext CONTEXT = new ScriptContext<>("date_script_field", Factory.class); + public static final ScriptContext CONTEXT = newContext("date", Factory.class); static List whitelist() { return List.of(WhitelistLoader.loadFromResourceFiles(RuntimeFieldsPainlessExtension.class, "date_whitelist.txt")); diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScriptTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScriptTests.java new file mode 100644 index 0000000000000..bf8e0b1685621 --- /dev/null +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DateScriptFieldScriptTests.java @@ -0,0 +1,37 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.runtimefields; + +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; +import java.util.Map; + +public class DateScriptFieldScriptTests extends ESTestCase { + public static final DateScriptFieldScript.Factory DUMMY = (params, lookup, formatter) -> ctx -> new DateScriptFieldScript( + params, + lookup, + formatter, + ctx + ) { + @Override + public void execute() { + new DateScriptFieldScript.Millis(this).millis(1595431354874L); + } + }; + + public void testRateLimitingDisabled() throws IOException { + try (ScriptService scriptService = TestScriptEngine.scriptService(DateScriptFieldScript.CONTEXT, DUMMY)) { + for (int i = 0; i < 1000; i++) { + scriptService.compile(new Script(ScriptType.INLINE, "test", "test_" + i, Map.of()), DateScriptFieldScript.CONTEXT); + } + } + } +} diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/TestScriptEngine.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/TestScriptEngine.java new file mode 100644 index 0000000000000..8fe7d7d565617 --- /dev/null +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/TestScriptEngine.java @@ -0,0 +1,50 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.runtimefields; + +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.script.ScriptContext; +import org.elasticsearch.script.ScriptEngine; +import org.elasticsearch.script.ScriptService; + +import java.util.Map; +import java.util.Set; + +public abstract class TestScriptEngine implements ScriptEngine { + public static ScriptService scriptService(ScriptContext context, F factory) { + return new ScriptService(Settings.EMPTY, Map.of("test", new TestScriptEngine() { + @Override + protected Object buildScriptFactory(ScriptContext context) { + return factory; + } + + @Override + public Set> getSupportedContexts() { + return Set.of(context); + } + }), Map.of(context.name, context)); + } + + @Override + public final String getType() { + return "test"; + } + + @Override + public final FactoryType compile( + String name, + String code, + ScriptContext context, + Map params + ) { + @SuppressWarnings("unchecked") + FactoryType castFactory = (FactoryType) buildScriptFactory(context); + return castFactory; + } + + protected abstract Object buildScriptFactory(ScriptContext context); +} diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeScriptFieldMapperTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeScriptFieldMapperTests.java index 07447123c5264..65b66774e2af9 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeScriptFieldMapperTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeScriptFieldMapperTests.java @@ -25,11 +25,13 @@ import org.elasticsearch.test.InternalSettingsPlugin; import org.elasticsearch.xpack.runtimefields.BooleanScriptFieldScript; import org.elasticsearch.xpack.runtimefields.DateScriptFieldScript; +import org.elasticsearch.xpack.runtimefields.DateScriptFieldScriptTests; import org.elasticsearch.xpack.runtimefields.DoubleScriptFieldScript; import org.elasticsearch.xpack.runtimefields.IpScriptFieldScript; import org.elasticsearch.xpack.runtimefields.LongScriptFieldScript; import org.elasticsearch.xpack.runtimefields.RuntimeFields; import org.elasticsearch.xpack.runtimefields.StringScriptFieldScript; +import org.elasticsearch.xpack.runtimefields.TestScriptEngine; import java.io.IOException; import java.util.Arrays; @@ -314,28 +316,9 @@ private XContentBuilder mapping(String type, CheckedConsumer> contexts) { - return new ScriptEngine() { + return new TestScriptEngine() { @Override - public String getType() { - return "test"; - } - - @Override - public FactoryType compile( - String name, - String code, - ScriptContext context, - Map paramsMap - ) { - if ("dummy_source".equals(code)) { - @SuppressWarnings("unchecked") - FactoryType castFactory = (FactoryType) dummyScriptFactory(context); - return castFactory; - } - throw new IllegalArgumentException("No test script for [" + code + "]"); - } - - private Object dummyScriptFactory(ScriptContext context) { + protected Object buildScriptFactory(ScriptContext context) { if (context == BooleanScriptFieldScript.CONTEXT) { return (BooleanScriptFieldScript.Factory) (params, lookup) -> ctx -> new BooleanScriptFieldScript( params, @@ -349,17 +332,7 @@ public void execute() { }; } if (context == DateScriptFieldScript.CONTEXT) { - return (DateScriptFieldScript.Factory) (params, lookup, formatter) -> ctx -> new DateScriptFieldScript( - params, - lookup, - formatter, - ctx - ) { - @Override - public void execute() { - new DateScriptFieldScript.Millis(this).millis(1595431354874L); - } - }; + return DateScriptFieldScriptTests.DUMMY; } if (context == DoubleScriptFieldScript.CONTEXT) { return (DoubleScriptFieldScript.Factory) (params, lookup) -> ctx -> new DoubleScriptFieldScript( From 52c77c87c9adc5c4e41b6fb2fc35bc99691f3edb Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 17 Aug 2020 16:57:05 -0400 Subject: [PATCH 2/2] Spotless --- .../xpack/runtimefields/AbstractScriptFieldScript.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java index 9db19ec19ac08..67693c4faea83 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java @@ -24,7 +24,9 @@ */ public abstract class AbstractScriptFieldScript { public static ScriptContext newContext(String name, Class factoryClass) { - return new ScriptContext(name + "_script_field", factoryClass, + return new ScriptContext( + name + "_script_field", + factoryClass, /* * In an ideal world we wouldn't need the script cache at all * because we have a hard reference to the script. The trouble @@ -32,13 +34,15 @@ public static ScriptContext newContext(String name, Class factoryClass * a mapping update. This is unfortunate, but we rely on the * cache to speed this up. */ - 100, timeValueMillis(0), + 100, + timeValueMillis(0), /* * Disable compilation rate limits for scripted fields so we * don't prevent mapping updates because we've performed too * many recently. That'd just be lame. */ - ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple()); + ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple() + ); } private final Map params;