Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scripting: Increase ingest script cache defaults #53765

Merged
merged 3 commits into from
Mar 20, 2020
Merged

Scripting: Increase ingest script cache defaults #53765

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a2bbfd
Scripting: Increase ingest script cache defaults
stu-elastic Mar 18, 2020
0e6b6e7
Add ingest conditional script
stu-elastic Mar 20, 2020
27c2e95
Merge branch 'master' of github.com:elastic/elasticsearch into fix/50…
stu-elastic Mar 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion server/src/main/java/org/elasticsearch/script/IngestScript.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

package org.elasticsearch.script;

import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.unit.TimeValue;

import java.util.Map;

/**
Expand All @@ -30,7 +33,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);
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("ingest", Factory.class,
200, TimeValue.timeValueMillis(0), new Tuple<>(375, TimeValue.timeValueMinutes(5)));
stu-elastic marked this conversation as resolved.
Show resolved Hide resolved

/** The generic runtime parameters for the script. */
private final Map<String, Object> params;
Expand Down
28 changes: 26 additions & 2 deletions server/src/main/java/org/elasticsearch/script/ScriptContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

package org.elasticsearch.script;

import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.unit.TimeValue;

import java.lang.reflect.Method;

/**
Expand Down Expand Up @@ -68,8 +71,18 @@ public final class ScriptContext<FactoryType> {
/** A class that is an instance of a script. */
public final Class<?> instanceClazz;

/** Construct a context with the related instance and compiled classes. */
public ScriptContext(String name, Class<FactoryType> factoryClazz) {
/** The default size of the cache for the context if not overridden */
public final int cacheSizeDefault;

/** The default expiration of a script in the cache for the context, if not overridden */
public final TimeValue cacheExpireDefault;

/** The default max compilation rate for scripts in this context. Script compilation is throttled if this is exceeded */
public final Tuple<Integer, TimeValue> maxCompilationRateDefault;

/** Construct a context with the related instance and compiled classes with caller provided cache defaults */
public ScriptContext(String name, Class<FactoryType> factoryClazz, int cacheSizeDefault, TimeValue cacheExpireDefault,
Tuple<Integer, TimeValue> maxCompilationRateDefault) {
this.name = name;
this.factoryClazz = factoryClazz;
Method newInstanceMethod = findMethod("FactoryType", factoryClazz, "newInstance");
Expand All @@ -90,6 +103,17 @@ public ScriptContext(String name, Class<FactoryType> factoryClazz) {
+ factoryClazz.getName() + "] for script context [" + name + "]");
}
instanceClazz = newInstanceMethod.getReturnType();

this.cacheSizeDefault = cacheSizeDefault;
this.cacheExpireDefault = cacheExpireDefault;
this.maxCompilationRateDefault = maxCompilationRateDefault;
}

/** Construct a context with the related instance and compiled classes with defaults for cacheSizeDefault, cacheExpireDefault and
* maxCompilationRateDefault */
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), new Tuple<>(75, TimeValue.timeValueMinutes(5)));
}

/** Returns a method with the given name, or throws an exception if multiple are found. */
Expand Down
54 changes: 34 additions & 20 deletions server/src/main/java/org/elasticsearch/script/ScriptService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
Expand Down Expand Up @@ -235,7 +236,7 @@ public ScriptService(Settings settings, Map<String, ScriptEngine> engines, Map<S

// Validation requires knowing which contexts exist.
this.validateCacheSettings(settings);
cacheHolder = new AtomicReference<>(new CacheHolder(settings, contexts.keySet(), compilationLimitsEnabled()));
cacheHolder = new AtomicReference<>(new CacheHolder(settings, contexts.values(), compilationLimitsEnabled()));
}

/**
Expand All @@ -249,12 +250,12 @@ void registerClusterSettingsListeners(ClusterSettings clusterSettings) {
clusterSettings.addSettingsUpdateConsumer(SCRIPT_MAX_SIZE_IN_BYTES, this::setMaxSizeInBytes);

// Handle all updatable per-context settings at once for each context.
for (String context: contexts.keySet()) {
for (ScriptContext<?> context: contexts.values()) {
clusterSettings.addSettingsUpdateConsumer(
(settings) -> cacheHolder.get().updateContextSettings(settings, context),
List.of(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context),
SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context),
SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context),
List.of(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context.name),
SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context.name),
SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context.name),
SCRIPT_GENERAL_CACHE_EXPIRE_SETTING, // general settings used for fallbacks
SCRIPT_GENERAL_CACHE_SIZE_SETTING
)
Expand Down Expand Up @@ -574,17 +575,18 @@ static class CacheHolder {
final ScriptCache general;
final Map<String, AtomicReference<ScriptCache>> contextCache;

final Set<String> contexts;
final Set<ScriptContext<?>> contexts;
final boolean compilationLimitsEnabled;

CacheHolder(Settings settings, Set<String> contexts, boolean compilationLimitsEnabled) {
CacheHolder(Settings settings, Collection<ScriptContext<?>> contexts, boolean compilationLimitsEnabled) {
this.compilationLimitsEnabled = compilationLimitsEnabled;
this.contexts = Set.copyOf(contexts);
if (SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE)) {
this.general = null;
Map<String, AtomicReference<ScriptCache>> contextCache = new HashMap<>(this.contexts.size());
for (String context : this.contexts) {
contextCache.put(context, new AtomicReference<>(contextFromSettings(settings, context, this.compilationLimitsEnabled)));
for (ScriptContext<?> context : this.contexts) {
contextCache.put(context.name,
new AtomicReference<>(contextFromSettings(settings, context, this.compilationLimitsEnabled)));
}
this.contextCache = Collections.unmodifiableMap(contextCache);
} else {
Expand All @@ -601,12 +603,24 @@ static class CacheHolder {
/**
* Create a ScriptCache for the given context.
*/
private static ScriptCache contextFromSettings(Settings settings, String context, boolean compilationLimitsEnabled) {
return new ScriptCache(SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context).get(settings),
SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context).get(settings),
compilationLimitsEnabled ?
SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context).get(settings) :
SCRIPT_COMPILATION_RATE_ZERO);
private static ScriptCache contextFromSettings(Settings settings, ScriptContext<?> context, boolean compilationLimitsEnabled) {
String name = context.name;
Tuple<Integer, TimeValue> compileRate;
Setting<Tuple<Integer, TimeValue>> rateSetting = SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(name);
if (compilationLimitsEnabled == false) {
compileRate = SCRIPT_COMPILATION_RATE_ZERO;
} else if (rateSetting.existsOrFallbackExists(settings)) {
compileRate = rateSetting.get(settings);
} else {
compileRate = context.maxCompilationRateDefault;
}

Setting<TimeValue> cacheExpire = SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(name);
Setting<Integer> cacheSize = SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(name);

return new ScriptCache(cacheSize.existsOrFallbackExists(settings) ? cacheSize.get(settings) : context.cacheSizeDefault,
cacheExpire.existsOrFallbackExists(settings) ? cacheExpire.get(settings) : context.cacheExpireDefault,
compileRate);
}

/**
Expand Down Expand Up @@ -660,16 +674,16 @@ ScriptStats stats() {
/**
* Update settings for the context cache, if we're in the context cache mode otherwise no-op.
*/
void updateContextSettings(Settings settings, String context) {
void updateContextSettings(Settings settings, ScriptContext<?> context) {
if (general != null) {
return;
}
AtomicReference<ScriptCache> ref = contextCache.get(context);
assert ref != null : "expected script cache to exist for context [" + context + "]";
AtomicReference<ScriptCache> ref = contextCache.get(context.name);
assert ref != null : "expected script cache to exist for context [" + context.name + "]";
ScriptCache cache = ref.get();
assert cache != null : "expected script cache to be non-null for context [" + context + "]";
assert cache != null : "expected script cache to be non-null for context [" + context.name + "]";
ref.set(contextFromSettings(settings, context, compilationLimitsEnabled));
logger.debug("Replaced context [" + context + "] with new settings");
logger.debug("Replaced context [" + context.name + "] with new settings");
}
}
}
Loading