Skip to content

Commit 0942be0

Browse files
committed
Watcher: Reduce script cache churn by checking for mustache tags (#33978)
Watcher is using a lot of so called TextTemplate fields in a watch definition, which can use mustache to insert the watch id for example. For the user it is non-obvious which field is just a string field or which field is a text template. This also means, that for every such field, we currently do a script compilation, even if the field does not contain any mustache syntax. This will lead to an increased script cache churn, because those compiled scripts (that only contain a string), will evict other scripts. On top of that, this also means that an unneeded compilation has happened, instead of returning that string immediately. The usages of mustache templating are in all of the actions (most of the time far more than one compilation) as well as most of the inputs. Especially when running a lot of watches in parallel, this will reduce execution times and help reuse of real scripts.
1 parent 7154255 commit 0942be0

File tree

2 files changed

+45
-3
lines changed

2 files changed

+45
-3
lines changed

x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateEngine.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ public String render(TextTemplate textTemplate, Map<String, Object> model) {
3535
String mediaType = compileParams(detectContentType(template));
3636
template = trimContentType(textTemplate);
3737

38+
int indexStartMustacheExpression = template.indexOf("{{");
39+
if (indexStartMustacheExpression == -1) {
40+
return template;
41+
}
42+
3843
Map<String, Object> mergedModel = new HashMap<>();
3944
if (textTemplate.getParams() != null) {
4045
mergedModel.putAll(textTemplate.getParams());

x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/common/text/TextTemplateTests.java

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import java.util.Collections;
2323
import java.util.HashMap;
24+
import java.util.Locale;
2425
import java.util.Map;
2526

2627
import static java.util.Collections.singletonMap;
@@ -31,7 +32,10 @@
3132
import static org.hamcrest.Matchers.is;
3233
import static org.hamcrest.Matchers.notNullValue;
3334
import static org.hamcrest.Matchers.nullValue;
35+
import static org.mockito.Matchers.any;
3436
import static org.mockito.Mockito.mock;
37+
import static org.mockito.Mockito.verify;
38+
import static org.mockito.Mockito.verifyZeroInteractions;
3539
import static org.mockito.Mockito.when;
3640

3741
public class TextTemplateTests extends ESTestCase {
@@ -47,7 +51,7 @@ public void init() throws Exception {
4751
}
4852

4953
public void testRender() throws Exception {
50-
String templateText = "_template";
54+
String templateText = "{{_template}}";
5155
Map<String, Object> params = singletonMap("param_key", "param_val");
5256
Map<String, Object> model = singletonMap("model_key", "model_val");
5357
Map<String, Object> merged = new HashMap<>(params);
@@ -72,7 +76,7 @@ public String execute() {
7276
}
7377

7478
public void testRenderOverridingModel() throws Exception {
75-
String templateText = "_template";
79+
String templateText = "{{_template}}";
7680
Map<String, Object> params = singletonMap("key", "param_val");
7781
Map<String, Object> model = singletonMap("key", "model_val");
7882
ScriptType type = randomFrom(ScriptType.values());
@@ -94,7 +98,7 @@ public String execute() {
9498
}
9599

96100
public void testRenderDefaults() throws Exception {
97-
String templateText = "_template";
101+
String templateText = "{{_template}}";
98102
Map<String, Object> model = singletonMap("key", "model_val");
99103

100104
TemplateScript.Factory compiledTemplate = templateParams ->
@@ -113,6 +117,39 @@ public String execute() {
113117
assertThat(engine.render(template, model), is("rendered_text"));
114118
}
115119

120+
public void testDontInvokeScriptServiceOnNonMustacheText() {
121+
assertNoCompilation("this is my text");
122+
assertScriptServiceInvoked("}}{{");
123+
assertScriptServiceInvoked("}}{{ctx.payload}}");
124+
}
125+
126+
private void assertNoCompilation(String input) {
127+
String output = engine.render(new TextTemplate(input), Collections.emptyMap());
128+
assertThat(input, is(output));
129+
verifyZeroInteractions(service);
130+
}
131+
132+
private void assertScriptServiceInvoked(final String input) {
133+
ScriptService scriptService = mock(ScriptService.class);
134+
TextTemplateEngine e = new TextTemplateEngine(Settings.EMPTY, scriptService);
135+
136+
TemplateScript.Factory compiledTemplate = templateParams ->
137+
new TemplateScript(templateParams) {
138+
@Override
139+
public String execute() {
140+
return input.toUpperCase(Locale.ROOT);
141+
}
142+
};
143+
144+
when(scriptService.compile(new Script(ScriptType.INLINE, lang, input,
145+
Collections.singletonMap("content_type", "text/plain"), Collections.emptyMap()), Watcher.SCRIPT_TEMPLATE_CONTEXT))
146+
.thenReturn(compiledTemplate);
147+
148+
String output = e.render(new TextTemplate(input), Collections.emptyMap());
149+
verify(scriptService).compile(any(), any());
150+
assertThat(output, is(input.toUpperCase(Locale.ROOT)));
151+
}
152+
116153
public void testParser() throws Exception {
117154
ScriptType type = randomScriptType();
118155
TextTemplate template =

0 commit comments

Comments
 (0)