Skip to content

Commit

Permalink
Use full ScriptContext constructor, remove comments
Browse files Browse the repository at this point in the history
  • Loading branch information
stu-elastic committed Oct 19, 2021
1 parent 291da10 commit bc9e5ad
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.util.Map;
import java.util.function.Function;

import static org.elasticsearch.core.TimeValue.timeValueMillis;

/**
* Abstract base for scripts to execute to build scripted fields. Inspired by
* {@link AggregationScript} but hopefully with less historical baggage.
Expand All @@ -30,21 +32,33 @@ public abstract class AbstractFieldScript extends DocBasedScript {
public static final int MAX_VALUES = 100;

static <F> ScriptContext<F> newContext(String name, Class<F> factoryClass) {
/*
* We rely on the script cache in two ways:
* 1. It caches the "heavy" part of mappings generated at runtime.
* 2. Mapping updates tend to try to compile the script twice. Not
* for any good reason. They just do.
* Thus we use the default cache size.
*
* We also disable compilation rate limits for runtime fields so we
* don't prevent mapping updates because we've performed too
* many recently. That'd just be lame. We also compile these
* scripts during search requests so this could totally be a
* source of runaway script compilations. We think folks will
* mostly reuse scripts though.
*/
return new ScriptContext<>(name, factoryClass, false);
return new ScriptContext<>(
name,
factoryClass,
/*
* We rely on the script cache in two ways:
* 1. It caches the "heavy" part of mappings generated at runtime.
* 2. Mapping updates tend to try to compile the script twice. Not
* for any good reason. They just do.
* Thus we use the default 100.
*/
100,
timeValueMillis(0),
/*
* Disable compilation rate limits for runtime fields so we
* don't prevent mapping updates because we've performed too
* many recently. That'd just be lame. We also compile these
* scripts during search requests so this could totally be a
* source of runaway script compilations. We think folks will
* mostly reuse scripts though.
*/
false,
/*
* Disable runtime fields scripts from being allowed
* to be stored as part of the script meta data.
*/
false
);
}

private static final Map<String, Function<Object, Object>> PARAMS_FUNCTIONS = org.elasticsearch.core.Map.of(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.elasticsearch.script;

import org.elasticsearch.core.TimeValue;

import java.util.Map;

/**
Expand All @@ -18,7 +20,8 @@ public abstract class IngestConditionalScript {
public static final String[] PARAMETERS = { "ctx" };

/** The context used to compile {@link IngestConditionalScript} factories. */
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("processor_conditional", Factory.class, true);
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("processor_conditional", Factory.class,
200, TimeValue.timeValueMillis(0), false, true);

/** The generic runtime parameters for the script. */
private final Map<String, Object> params;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

package org.elasticsearch.script;

import org.elasticsearch.core.TimeValue;

import java.util.Map;

/**
Expand All @@ -19,7 +21,8 @@ public abstract class IngestScript {
public static final String[] PARAMETERS = { "ctx" };

/** The context used to compile {@link IngestScript} factories. */
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("ingest", Factory.class, true);
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("ingest", Factory.class,
200, TimeValue.timeValueMillis(0), false, true);

/** The generic runtime parameters for the script. */
private final Map<String, Object> params;
Expand Down
16 changes: 3 additions & 13 deletions server/src/main/java/org/elasticsearch/script/ScriptContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public final class ScriptContext<FactoryType> {
public final boolean allowStoredScript;

/** Construct a context with the related instance and compiled classes with caller provided cache defaults */
ScriptContext(String name, Class<FactoryType> factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault,
boolean compilationRateLimited, boolean allowStoredScript) {
public ScriptContext(String name, Class<FactoryType> factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault,
boolean compilationRateLimited, boolean allowStoredScript) {
this.name = name;
this.factoryClazz = factoryClazz;
Method newInstanceMethod = findMethod("FactoryType", factoryClazz, "newInstance");
Expand Down Expand Up @@ -105,22 +105,12 @@ public final class ScriptContext<FactoryType> {
}

/** Construct a context with the related instance and compiled classes with defaults for cacheSizeDefault, cacheExpireDefault and
* maxCompilationRateDefault and allow scripts of this context to be stored scripts */
* compilationRateLimited and allow scripts of this context to be stored scripts */
public ScriptContext(String name, Class<FactoryType> factoryClazz) {
// cache size default, cache expire default, max compilation rate are defaults from ScriptService.
this(name, factoryClazz, 100, TimeValue.timeValueMillis(0), true, true);
}

/**
* Construct a context for a system script usage, using the:
* - default cache size (only relevant when context caching enabled): 200
* - default cache expiration: no expiration
* - unlimited compilation rate
*/
public ScriptContext(String name, Class<FactoryType> factoryClazz, boolean allowStoredScript) {
this(name, factoryClazz, 200, TimeValue.timeValueMillis(0), false, allowStoredScript);
}

/** Returns a method with the given name, or throws an exception if multiple are found. */
private Method findMethod(String type, Class<?> clazz, String methodName) {
Method foundMethod = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(Fields.COMPILATIONS, compilations);
builder.field(Fields.CACHE_EVICTIONS, cacheEvictions);
builder.field(Fields.COMPILATION_LIMIT_TRIGGERED, compilationLimitTriggered);
/* TODO(stu): master only
builder.startArray(Fields.CONTEXTS);
for (ScriptContextStats contextStats: contextStats) {
contextStats.toXContent(builder, params);
}
builder.endArray();
*/
builder.endObject();
return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.elasticsearch.script;

import org.elasticsearch.core.TimeValue;

import java.util.Map;

/**
Expand Down Expand Up @@ -39,5 +41,6 @@ public interface Factory {
// Remove compilation rate limit for ingest. Ingest pipelines may use many mustache templates, triggering compilation
// rate limiting. MustacheScriptEngine explicitly checks for TemplateScript. Rather than complicating the implementation there by
// creating a new Script class (as would be customary), this context is used to avoid the default rate limit.
public static final ScriptContext<Factory> INGEST_CONTEXT = new ScriptContext<>("ingest_template", Factory.class, true);
public static final ScriptContext<Factory> INGEST_CONTEXT = new ScriptContext<>("ingest_template", Factory.class,
200, TimeValue.timeValueMillis(0), false, true);
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,6 @@ public void testXContent() throws IOException {
stats.toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();

/* TODO(stu): master only
String expected = "{\n" +
" \"script\" : {\n" +
" \"compilations\" : 1100,\n" +
" \"cache_evictions\" : 2211,\n" +
" \"compilation_limit_triggered\" : 3322\n" +
" }\n" +
"}";
*/
String expected = "{\n" +
" \"script\" : {\n" +
" \"compilations\" : 1100,\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ public class Watcher extends Plugin implements SystemIndexPlugin, ScriptPlugin,
new ByteSizeValue(1, ByteSizeUnit.MB), new ByteSizeValue(10, ByteSizeUnit.MB), NodeScope);

public static final ScriptContext<TemplateScript.Factory> SCRIPT_TEMPLATE_CONTEXT
= new ScriptContext<>("xpack_template", TemplateScript.Factory.class, true);
= new ScriptContext<>("xpack_template", TemplateScript.Factory.class,
200, TimeValue.timeValueMillis(0), false, true);

private static final Logger logger = LogManager.getLogger(Watcher.class);
private WatcherIndexingListener listener;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.watcher.condition;

import org.elasticsearch.core.TimeValue;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.watcher.support.Variables;
Expand Down Expand Up @@ -42,5 +43,6 @@ public interface Factory {
WatcherConditionScript newInstance(Map<String, Object> params, WatchExecutionContext watcherContext);
}

public static ScriptContext<Factory> CONTEXT = new ScriptContext<>("watcher_condition", Factory.class, true);
public static ScriptContext<Factory> CONTEXT = new ScriptContext<>("watcher_condition", Factory.class,
200, TimeValue.timeValueMillis(0), false, true);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/
package org.elasticsearch.xpack.watcher.transform.script;

import org.elasticsearch.core.TimeValue;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.xpack.core.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.core.watcher.watch.Payload;
Expand Down Expand Up @@ -43,5 +44,6 @@ public interface Factory {
WatcherTransformScript newInstance(Map<String, Object> params, WatchExecutionContext watcherContext, Payload payload);
}

public static ScriptContext<Factory> CONTEXT = new ScriptContext<>("watcher_transform", Factory.class, true);
public static ScriptContext<Factory> CONTEXT = new ScriptContext<>("watcher_transform", Factory.class,
200, TimeValue.timeValueMillis(0), false, true);
}

0 comments on commit bc9e5ad

Please sign in to comment.