From 950dc735a8a2bd1140517180ba708df4ff4ae798 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Wed, 28 Mar 2018 12:39:46 +0200 Subject: [PATCH] Scripting: Scriptengine can tell if compilation is needed A script engine can now tell the ScriptService if compilation of a certain script is needed. For most of the cases this will always be the case, but the mustache templating engine can check, if a compilation is needed, and if not just return the original template. This also has the side effect that if no compilation is required, caching is not required either, as it makes no sense to put the original string into a cache. The goal of this commit is to prevent hitting the compilation limit, if a user is using a lot of templated fields (for example in a watch), but is not putting any template in there that requires compilation. --- .../script/mustache/MustacheScriptEngine.java | 44 ++++++++++++++++++- .../mustache/MustacheScriptEngineTests.java | 13 +++++- .../elasticsearch/script/ScriptEngine.java | 11 +++++ .../elasticsearch/script/ScriptService.java | 17 ++++--- 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java index 5a0b2e15460c5..7100ce9aff6b2 100644 --- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java +++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java @@ -20,8 +20,6 @@ import com.github.mustachejava.Mustache; import com.github.mustachejava.MustacheFactory; - -import java.io.StringReader; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; @@ -34,6 +32,7 @@ import org.elasticsearch.script.TemplateScript; import java.io.Reader; +import java.io.StringReader; import java.io.StringWriter; import java.security.AccessController; import java.security.PrivilegedAction; @@ -64,6 +63,11 @@ public T compile(String templateName, String templateSource, ScriptContext new NoRenderMustacheExecutableScript(templateSource, params); + return context.factoryClazz.cast(compiled); + } final MustacheFactory factory = createMustacheFactory(options); Reader reader = new StringReader(templateSource); Mustache template = factory.compile(reader, "query-template"); @@ -83,6 +87,22 @@ public String getType() { return NAME; } + @Override + public boolean requiresCompilation(String templateSource) { + return containsMustacheTemplates(templateSource); + } + + /** + * A simple check if this mustache templates requires compilation. If you pass JSON in here, this condition can + * be triggered by the JSON, even though no compilation would be needed otherwise + * + * @param template the mustache template source + * @return true if it contains mustache opening and closing tags, false otherwise + */ + private boolean containsMustacheTemplates(String template) { + return template.contains("{{") && template.contains("}}"); + } + /** * Used at query execution time by script service in order to execute a query template. * */ @@ -118,4 +138,24 @@ public String execute() { return writer.toString(); } } + + /** + * a small helper class, that returns the original template instead of trying to execute any compilation or merging of + * the supplied parameters + */ + // package private for testing + class NoRenderMustacheExecutableScript extends TemplateScript { + + private final String templateSource; + + NoRenderMustacheExecutableScript(String templateSource, Map params) { + super(params); + this.templateSource = templateSource; + } + + @Override + public String execute() { + return templateSource; + } + } } diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java index b3509046241ed..44bfc0a3917a9 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java @@ -20,6 +20,7 @@ import com.github.mustachejava.MustacheFactory; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.script.TemplateScript; @@ -34,6 +35,7 @@ import java.util.Map; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; /** * Mustache based templating test @@ -75,7 +77,7 @@ public void testSimpleParameterReplace() { public void testSimple() throws IOException { String templateString = - "{" + "{" + "\"source\":{\"match_{{template}}\": {}}," + "\"params\":{\"template\":\"all\"}" + "}"; @@ -101,6 +103,15 @@ public void testParseTemplateAsSingleStringWithConditionalClause() throws IOExce assertThat(TemplateScript.execute(), equalTo("{ \"match_all\":{} }")); } + public void testNoCompilation() throws Exception { + String templateString = "{ \"source\": \"no mustache templates here\" }"; + XContentParser parser = createParser(JsonXContent.jsonXContent, templateString); + Script script = Script.parse(parser); + TemplateScript.Factory compiled = qe.compile(null, script.getIdOrCode(), TemplateScript.CONTEXT, Collections.emptyMap()); + TemplateScript templateScript = compiled.newInstance(script.getParams()); + assertThat(templateScript, instanceOf(MustacheScriptEngine.NoRenderMustacheExecutableScript.class)); + } + public void testEscapeJson() throws IOException { { StringWriter writer = new StringWriter(); diff --git a/server/src/main/java/org/elasticsearch/script/ScriptEngine.java b/server/src/main/java/org/elasticsearch/script/ScriptEngine.java index bd32cce0b3781..e5d8d018e893c 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptEngine.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptEngine.java @@ -45,4 +45,15 @@ public interface ScriptEngine extends Closeable { @Override default void close() throws IOException {} + + /** + * Usually scripts require compilation. However there can be special cases where scripts do not require any compilation to happen. + * For example, when the input is equal to the output when rendering a string based template without parameters. + * + * @param scriptSource the script + * @return true if it makes sense to put the executed script in the cache, false otherwise + */ + default boolean requiresCompilation(String scriptSource) { + return true; + } } diff --git a/server/src/main/java/org/elasticsearch/script/ScriptService.java b/server/src/main/java/org/elasticsearch/script/ScriptService.java index ca79e3b80fc81..2c093d38aa25a 100644 --- a/server/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/server/src/main/java/org/elasticsearch/script/ScriptService.java @@ -19,7 +19,6 @@ package org.elasticsearch.script; -import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.ResourceNotFoundException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.cluster.storedscripts.DeleteStoredScriptRequest; @@ -46,6 +45,7 @@ import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.core.internal.io.IOUtils; import java.io.Closeable; import java.io.IOException; @@ -322,6 +322,7 @@ public FactoryType compile(Script script, ScriptContext FactoryType compile(Script script, ScriptContext FactoryType compile(Script script, ScriptContext