From ca324d78b0ae1b86b244b5f4525ea5961decd687 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Wed, 10 Jun 2020 15:47:47 -0700 Subject: [PATCH] Pre-compile inline scripts in Ingest Script processors This commit introduces an optimization for inline scripts. It keeps the compiled ingest script that the ScriptProcessor.Factory has been creating for validation purposes. Previously, the Script Service's cache was leveraged because it was the best way to handle caching of both stored and inline scripts. Since inline scripts are so widely used in Ingest Node, it is probably best to ensure we are using the pre-compiled version from the beginning. --- .../ingest/common/ScriptProcessor.java | 33 ++++++++--- .../common/ScriptProcessorFactoryTests.java | 57 +++++++++++++++++++ .../ingest/common/ScriptProcessorTests.java | 48 +++++++++++----- 3 files changed, 117 insertions(+), 21 deletions(-) diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java index e6ac9e71839ec..d71cc0a64aeae 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java @@ -19,6 +19,7 @@ package org.elasticsearch.ingest.common; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; @@ -34,6 +35,7 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptException; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; import java.io.InputStream; import java.util.Arrays; @@ -50,17 +52,19 @@ public final class ScriptProcessor extends AbstractProcessor { private final Script script; private final ScriptService scriptService; + private final IngestScript precompiledIngestScript; /** * Processor that evaluates a script with an ingest document in its context - * - * @param tag The processor's tag. + * @param tag The processor's tag. * @param script The {@link Script} to execute. + * @param precompiledIngestScript The {@link Script} precompiled * @param scriptService The {@link ScriptService} used to execute the script. */ - ScriptProcessor(String tag, Script script, ScriptService scriptService) { + ScriptProcessor(String tag, Script script, @Nullable IngestScript precompiledIngestScript, ScriptService scriptService) { super(tag); this.script = script; + this.precompiledIngestScript = precompiledIngestScript; this.scriptService = scriptService; } @@ -71,8 +75,14 @@ public final class ScriptProcessor extends AbstractProcessor { */ @Override public IngestDocument execute(IngestDocument document) { - IngestScript.Factory factory = scriptService.compile(script, IngestScript.CONTEXT); - factory.newInstance(script.getParams()).execute(document.getSourceAndMetadata()); + final IngestScript ingestScript; + if (precompiledIngestScript == null) { + IngestScript.Factory factory = scriptService.compile(script, IngestScript.CONTEXT); + ingestScript = factory.newInstance(script.getParams()); + } else { + ingestScript = precompiledIngestScript; + } + ingestScript.execute(document.getSourceAndMetadata()); CollectionUtils.ensureNoSelfReferences(document.getSourceAndMetadata(), "ingest script"); return document; } @@ -86,6 +96,10 @@ Script getScript() { return script; } + IngestScript getPrecompiledIngestScript() { + return precompiledIngestScript; + } + public static final class Factory implements Processor.Factory { private final ScriptService scriptService; @@ -105,13 +119,16 @@ public ScriptProcessor create(Map registry, String pr Arrays.asList("id", "source", "inline", "lang", "params", "options").forEach(config::remove); // verify script is able to be compiled before successfully creating processor. + IngestScript ingestScript = null; try { - scriptService.compile(script, IngestScript.CONTEXT); + final IngestScript.Factory factory = scriptService.compile(script, IngestScript.CONTEXT); + if (ScriptType.INLINE.equals(script.getType())) { + ingestScript = factory.newInstance(script.getParams()); + } } catch (ScriptException e) { throw newConfigurationException(TYPE, processorTag, null, e); } - - return new ScriptProcessor(processorTag, script, scriptService); + return new ScriptProcessor(processorTag, script, ingestScript, scriptService); } } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java index 9559c150df68a..0e4cd5e705625 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java @@ -20,10 +20,15 @@ package org.elasticsearch.ingest.common; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParseException; +import org.elasticsearch.script.IngestScript; +import org.elasticsearch.script.MockScriptEngine; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptException; +import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; import org.elasticsearch.test.ESTestCase; import org.junit.Before; @@ -51,6 +56,10 @@ public void init() { } public void testFactoryValidationWithDefaultLang() throws Exception { + ScriptService mockedScriptService = mock(ScriptService.class); + when(mockedScriptService.compile(any(), any())).thenReturn(mock(IngestScript.Factory.class)); + factory = new ScriptProcessor.Factory(mockedScriptService); + Map configMap = new HashMap<>(); String randomType = randomFrom("id", "source"); configMap.put(randomType, "foo"); @@ -61,6 +70,10 @@ public void testFactoryValidationWithDefaultLang() throws Exception { } public void testFactoryValidationWithParams() throws Exception { + ScriptService mockedScriptService = mock(ScriptService.class); + when(mockedScriptService.compile(any(), any())).thenReturn(mock(IngestScript.Factory.class)); + factory = new ScriptProcessor.Factory(mockedScriptService); + Map configMap = new HashMap<>(); String randomType = randomFrom("id", "source"); Map randomParams = Collections.singletonMap(randomAlphaOfLength(10), randomAlphaOfLength(10)); @@ -94,6 +107,10 @@ public void testFactoryValidationAtLeastOneScriptingType() throws Exception { } public void testInlineBackcompat() throws Exception { + ScriptService mockedScriptService = mock(ScriptService.class); + when(mockedScriptService.compile(any(), any())).thenReturn(mock(IngestScript.Factory.class)); + factory = new ScriptProcessor.Factory(mockedScriptService); + Map configMap = new HashMap<>(); configMap.put("inline", "code"); @@ -117,4 +134,44 @@ public void testFactoryInvalidateWithInvalidCompiledScript() throws Exception { assertThat(exception.getMessage(), is("compile-time exception")); } + + public void testInlineIsCompiled() throws Exception { + String scriptName = "foo"; + ScriptService scriptService = new ScriptService(Settings.builder().build(), + Collections.singletonMap( + Script.DEFAULT_SCRIPT_LANG, new MockScriptEngine( + Script.DEFAULT_SCRIPT_LANG, + Collections.singletonMap(scriptName, ctx -> { + ctx.put("foo", "bar"); + return null; + }), + Collections.emptyMap() + ) + ), new HashMap<>(ScriptModule.CORE_CONTEXTS)); + factory = new ScriptProcessor.Factory(scriptService); + + Map configMap = new HashMap<>(); + configMap.put("source", scriptName); + ScriptProcessor processor = factory.create(null, randomAlphaOfLength(10), configMap); + assertThat(processor.getScript().getLang(), equalTo(Script.DEFAULT_SCRIPT_LANG)); + assertThat(processor.getScript().getType(), equalTo(ScriptType.INLINE)); + assertThat(processor.getScript().getParams(), equalTo(Collections.emptyMap())); + assertNotNull(processor.getPrecompiledIngestScript()); + Map ctx = new HashMap<>(); + processor.getPrecompiledIngestScript().execute(ctx); + assertThat(ctx.get("foo"), equalTo("bar")); + } + + public void testStoredIsNotCompiled() throws Exception { + ScriptService mockedScriptService = mock(ScriptService.class); + when(mockedScriptService.compile(any(), any())).thenReturn(mock(IngestScript.Factory.class)); + factory = new ScriptProcessor.Factory(mockedScriptService); + Map configMap = new HashMap<>(); + configMap.put("id", "script_name"); + ScriptProcessor processor = factory.create(null, randomAlphaOfLength(10), configMap); + assertNull(processor.getScript().getLang()); + assertThat(processor.getScript().getType(), equalTo(ScriptType.STORED)); + assertThat(processor.getScript().getParams(), equalTo(Collections.emptyMap())); + assertNull(processor.getPrecompiledIngestScript()); + } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java index 10fcf5fe602ad..54fd262b32ba8 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorTests.java @@ -22,12 +22,14 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.ingest.IngestDocument; import org.elasticsearch.ingest.RandomDocumentPicks; +import org.elasticsearch.script.IngestScript; import org.elasticsearch.script.MockScriptEngine; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; import org.elasticsearch.test.ESTestCase; +import org.junit.Before; import java.util.Collections; import java.util.HashMap; @@ -38,18 +40,22 @@ public class ScriptProcessorTests extends ESTestCase { - public void testScripting() throws Exception { - int randomBytesIn = randomInt(); - int randomBytesOut = randomInt(); - int randomBytesTotal = randomBytesIn + randomBytesOut; + private ScriptService scriptService; + private Script script; + private IngestScript ingestScript; + + @Before + public void setupScripting() { String scriptName = "script"; - ScriptService scriptService = new ScriptService(Settings.builder().build(), + scriptService = new ScriptService(Settings.builder().build(), Collections.singletonMap( Script.DEFAULT_SCRIPT_LANG, new MockScriptEngine( Script.DEFAULT_SCRIPT_LANG, Collections.singletonMap( scriptName, ctx -> { - ctx.put("bytes_total", randomBytesTotal); + Integer bytesIn = (Integer) ctx.get("bytes_in"); + Integer bytesOut = (Integer) ctx.get("bytes_out"); + ctx.put("bytes_total", bytesIn + bytesOut); return null; } ), @@ -58,20 +64,36 @@ Script.DEFAULT_SCRIPT_LANG, new MockScriptEngine( ), new HashMap<>(ScriptModule.CORE_CONTEXTS) ); - Script script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, scriptName, Collections.emptyMap()); + script = new Script(ScriptType.INLINE, Script.DEFAULT_SCRIPT_LANG, scriptName, Collections.emptyMap()); + ingestScript = scriptService.compile(script, IngestScript.CONTEXT).newInstance(script.getParams()); + } + + public void testScriptingWithoutPrecompiledScriptFactory() throws Exception { + ScriptProcessor processor = new ScriptProcessor(randomAlphaOfLength(10), script, null, scriptService); + IngestDocument ingestDocument = randomDocument(); + processor.execute(ingestDocument); + assertIngestDocument(ingestDocument); + } + + public void testScriptingWithPrecompiledIngestScript() { + ScriptProcessor processor = new ScriptProcessor(randomAlphaOfLength(10), script, ingestScript, scriptService); + IngestDocument ingestDocument = randomDocument(); + processor.execute(ingestDocument); + assertIngestDocument(ingestDocument); + } + private IngestDocument randomDocument() { Map document = new HashMap<>(); document.put("bytes_in", randomInt()); document.put("bytes_out", randomInt()); - IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); - - ScriptProcessor processor = new ScriptProcessor(randomAlphaOfLength(10), script, scriptService); - - processor.execute(ingestDocument); + return RandomDocumentPicks.randomIngestDocument(random(), document); + } + private void assertIngestDocument(IngestDocument ingestDocument) { assertThat(ingestDocument.getSourceAndMetadata(), hasKey("bytes_in")); assertThat(ingestDocument.getSourceAndMetadata(), hasKey("bytes_out")); assertThat(ingestDocument.getSourceAndMetadata(), hasKey("bytes_total")); - assertThat(ingestDocument.getSourceAndMetadata().get("bytes_total"), is(randomBytesTotal)); + int bytesTotal = ingestDocument.getFieldValue("bytes_in", Integer.class) + ingestDocument.getFieldValue("bytes_out", Integer.class); + assertThat(ingestDocument.getSourceAndMetadata().get("bytes_total"), is(bytesTotal)); } }