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.

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

Script: Opt-out system contexts from script compilation rate limit #79459

Merged
merged 3 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ static <F> ScriptContext<F> newContext(String name, Class<F> factoryClass) {
* source of runaway script compilations. We think folks will
* mostly reuse scripts though.
*/
ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(),
false,
/*
* Disable runtime fields scripts from being allowed
* to be stored as part of the script meta data.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public abstract class IngestConditionalScript {

/** The context used to compile {@link IngestConditionalScript} factories. */
public static final ScriptContext<Factory> CONTEXT = new ScriptContext<>("processor_conditional", Factory.class,
200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true);
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 @@ -22,7 +22,7 @@ public abstract class IngestScript {

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

/** The generic runtime parameters for the script. */
private final Map<String, Object> params;
Expand Down
13 changes: 5 additions & 8 deletions server/src/main/java/org/elasticsearch/script/ScriptCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,7 @@ public class ScriptCache {
private final double compilesAllowedPerNano;
private final String contextRateSetting;

ScriptCache(
int cacheMaxSize,
TimeValue cacheExpire,
CompilationRate maxCompilationRate,
String contextRateSetting
) {
ScriptCache(int cacheMaxSize, TimeValue cacheExpire, CompilationRate maxCompilationRate, String contextRateSetting) {
this.cacheSize = cacheMaxSize;
this.cacheExpire = cacheExpire;
this.contextRateSetting = contextRateSetting;
Expand Down Expand Up @@ -94,8 +89,10 @@ <FactoryType> FactoryType compile(
logger.trace("context [{}]: compiling script, type: [{}], lang: [{}], options: [{}]", context.name, type,
lang, options);
}
// Check whether too many compilations have happened
checkCompilationLimit();
if (context.compilationRateLimited) {
// Check whether too many compilations have happened
checkCompilationLimit();
}
Object compiledScript = scriptEngine.compile(id, idOrCode, context, options);
// Since the cache key is the script content itself we don't need to
// invalidate/check the cache if an indexed script changes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.util.Objects;
import java.util.stream.Collectors;

// This class is deprecated in favor of ScriptStats and ScriptContextStats. It is removed in 8.
// This class is deprecated in favor of ScriptStats and ScriptContextStats
public class ScriptCacheStats implements Writeable, ToXContentFragment {
private final Map<String, ScriptStats> context;
private final ScriptStats general;
Expand Down
14 changes: 8 additions & 6 deletions server/src/main/java/org/elasticsearch/script/ScriptContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
* be {@code boolean needs_score()}.
*/
public final class ScriptContext<FactoryType> {
/** The default compilation rate limit for contexts with compilation rate limiting enabled */
public static final Tuple<Integer, TimeValue> DEFAULT_COMPILATION_RATE_LIMIT = new Tuple<>(150, TimeValue.timeValueMinutes(5));

/** A unique identifier for this context. */
public final String name;
Expand All @@ -66,15 +68,15 @@ public final class ScriptContext<FactoryType> {
/** 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;
/** Is compilation rate limiting enabled for this context? */
public final boolean compilationRateLimited;

/** Determines if the script can be stored as part of the cluster state. */
public final boolean allowStoredScript;

/** 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, boolean allowStoredScript) {
boolean compilationRateLimited, boolean allowStoredScript) {
this.name = name;
this.factoryClazz = factoryClazz;
Method newInstanceMethod = findMethod("FactoryType", factoryClazz, "newInstance");
Expand All @@ -98,15 +100,15 @@ public ScriptContext(String name, Class<FactoryType> factoryClazz, int cacheSize

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

/** 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), new Tuple<>(75, TimeValue.timeValueMinutes(5)), true);
this(name, factoryClazz, 100, TimeValue.timeValueMillis(0), true, true);
}

/** Returns a method with the given name, or throws an exception if multiple are found. */
Expand Down
15 changes: 9 additions & 6 deletions server/src/main/java/org/elasticsearch/script/ScriptService.java
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ void registerClusterSettingsListeners(ClusterSettings clusterSettings) {

// Handle all settings for context and general caches, this flips between general and context caches.
clusterSettings.addSettingsUpdateConsumer(
(settings) -> setCacheHolder(settings),
this::setCacheHolder,
Arrays.asList(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING,
SCRIPT_GENERAL_CACHE_EXPIRE_SETTING,
SCRIPT_GENERAL_CACHE_SIZE_SETTING,
Expand Down Expand Up @@ -565,8 +565,9 @@ void setCacheHolder(Settings settings) {
} else if (current.general == null) {
// Flipping to general
cacheHolder.set(generalCacheHolder(settings));
} else if (current.general.rate.equals(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings)) == false) {
// General compilation rate changed, that setting is the only dynamically updated general setting
} else if (current.general.rate.equals(SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings)) == false ||
current.general.cacheExpire.equals(SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(settings)) == false ||
current.general.cacheSize != SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(settings)) {
cacheHolder.set(generalCacheHolder(settings));
}
}
Expand All @@ -592,13 +593,15 @@ ScriptCache contextCache(Settings settings, ScriptContext<?> context) {

Setting<ScriptCache.CompilationRate> rateSetting =
SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context.name);
ScriptCache.CompilationRate rate = null;
if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings) || compilationLimitsEnabled() == false) {
ScriptCache.CompilationRate rate;
if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings)
|| compilationLimitsEnabled() == false
|| context.compilationRateLimited == false) {
rate = SCRIPT_COMPILATION_RATE_ZERO;
} else if (rateSetting.existsOrFallbackExists(settings)) {
rate = rateSetting.get(settings);
} else {
rate = new ScriptCache.CompilationRate(context.maxCompilationRateDefault);
rate = new ScriptCache.CompilationRate(ScriptContext.DEFAULT_COMPILATION_RATE_LIMIT);
}

return new ScriptCache(cacheSize, cacheExpire, rate, rateSetting.getKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ public interface Factory {
// 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,
200, TimeValue.timeValueMillis(0), ScriptCache.UNLIMITED_COMPILATION_RATE.asTuple(), true);
200, TimeValue.timeValueMillis(0), false, true);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,54 @@
package org.elasticsearch.script;

import org.elasticsearch.common.breaker.CircuitBreakingException;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.test.ESTestCase;

import java.util.stream.Collectors;

public class ScriptCacheTests extends ESTestCase {
// even though circuit breaking is allowed to be configured per minute, we actually weigh this over five minutes
// simply by multiplying by five, so even setting it to one, requires five compilations to break
public void testCompilationCircuitBreaking() throws Exception {
String context = randomFrom(
ScriptModule.CORE_CONTEXTS.values().stream().filter(
c -> c.compilationRateLimited
).collect(Collectors.toList())
).name;
final TimeValue expire = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY);
final Integer size = ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY);
Setting<ScriptCache.CompilationRate> rateSetting =
ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context);
ScriptCache.CompilationRate rate =
ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY);
String rateSettingName = rateSetting.getKey();
ScriptCache cache = new ScriptCache(size, expire,
new ScriptCache.CompilationRate(1, TimeValue.timeValueMinutes(1)), rateSettingName);
cache.checkCompilationLimit(); // should pass
expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit);
cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(2, TimeValue.timeValueMinutes(1)), rateSettingName);
cache.checkCompilationLimit(); // should pass
cache.checkCompilationLimit(); // should pass
expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit);
int count = randomIntBetween(5, 50);
cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(count, TimeValue.timeValueMinutes(1)), rateSettingName);
for (int i = 0; i < count; i++) {
cache.checkCompilationLimit(); // should pass
}
expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit);
cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(0, TimeValue.timeValueMinutes(1)), rateSettingName);
expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit);
cache = new ScriptCache(size, expire,
new ScriptCache.CompilationRate(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)), rateSettingName);
int largeLimit = randomIntBetween(1000, 10000);
for (int i = 0; i < largeLimit; i++) {
cache.checkCompilationLimit();
}
}

public void testGeneralCompilationCircuitBreaking() throws Exception {
final TimeValue expire = ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(Settings.EMPTY);
final Integer size = ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(Settings.EMPTY);
String settingName = ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey();
Expand All @@ -35,14 +75,33 @@ public void testCompilationCircuitBreaking() throws Exception {
cache = new ScriptCache(size, expire, new ScriptCache.CompilationRate(0, TimeValue.timeValueMinutes(1)), settingName);
expectThrows(CircuitBreakingException.class, cache::checkCompilationLimit);
cache = new ScriptCache(size, expire,
new ScriptCache.CompilationRate(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)), settingName);
new ScriptCache.CompilationRate(Integer.MAX_VALUE, TimeValue.timeValueMinutes(1)), settingName);
int largeLimit = randomIntBetween(1000, 10000);
for (int i = 0; i < largeLimit; i++) {
cache.checkCompilationLimit();
}
}

public void testUnlimitedCompilationRate() {
String context = randomFrom(
ScriptModule.CORE_CONTEXTS.values().stream().filter(
c -> c.compilationRateLimited
).collect(Collectors.toList())
).name;
final Integer size = ScriptService.SCRIPT_CACHE_SIZE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY);
final TimeValue expire = ScriptService.SCRIPT_CACHE_EXPIRE_SETTING.getConcreteSettingForNamespace(context).get(Settings.EMPTY);
String settingName = ScriptService.SCRIPT_MAX_COMPILATIONS_RATE_SETTING.getConcreteSettingForNamespace(context).getKey();
ScriptCache cache = new ScriptCache(size, expire, ScriptCache.UNLIMITED_COMPILATION_RATE, settingName);
ScriptCache.TokenBucketState initialState = cache.tokenBucketState.get();
for(int i=0; i < 3000; i++) {
cache.checkCompilationLimit();
ScriptCache.TokenBucketState currentState = cache.tokenBucketState.get();
assertEquals(initialState.lastInlineCompileTime, currentState.lastInlineCompileTime);
assertEquals(initialState.availableTokens, currentState.availableTokens, 0.0); // delta of 0.0 because it should never change
}
}

public void testGeneralUnlimitedCompilationRate() {
final Integer size = ScriptService.SCRIPT_GENERAL_CACHE_SIZE_SETTING.get(Settings.EMPTY);
final TimeValue expire = ScriptService.SCRIPT_GENERAL_CACHE_EXPIRE_SETTING.get(Settings.EMPTY);
String settingName = ScriptService.SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey();
Expand Down
Loading