Skip to content

Commit

Permalink
Scripting: Increase ingest script cache defaults
Browse files Browse the repository at this point in the history 
* Adds ability for contexts to specify their own defaults.
* Context defaults are applied if no context-specific or
  general setting exists.
* See 070ea7e for settings keys.

* Increases the per-context default for the `ingest` context.
* Cache size is doubled, 200 compared to default of 100
* Cache expiration is unchanged at no expiration
* Cache max compilation is quintupled, 375/5m instead of 75/5m

Refs: elastic#50152
  • Loading branch information
@stu-elastic
stu-elastic committed Mar 18, 2020
1 parent 73d2add commit 6a2bbfd
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 33 deletions.
6 changes: 5 additions & 1 deletion server/src/main/java/org/elasticsearch/script/IngestScript.java
View file
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)));

/** 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
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
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

0 comments on commit 6a2bbfd

Please sign in to comment.