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

Script: Implicit context cache #81552

Merged
merged 4 commits into from
Dec 8, 2021
+283 −57
Merged

Script: Implicit context cache #81552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c661479
Script: Implicit context cache
stu-elastic Dec 8, 2021
e7e8eae
Tests
stu-elastic Dec 8, 2021
82b2d2e
Sort keys and update tests
stu-elastic Dec 8, 2021
d246290
Update docs/changelog/81552.yaml
stu-elastic Dec 8, 2021
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
5 changes: 5 additions & 0 deletions docs/changelog/81552.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 81552
summary: "Script: Implicit context cache"
area: Infra/Scripting
type: bug
issues: []
197 changes: 158 additions & 39 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 @@ -299,6 +299,14 @@ public static boolean isUseContextCacheSet(Settings settings) {
return SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE);
}

public static boolean isImplicitContextCacheSet(Settings settings) {
return new ScriptService.ContextSettings(settings).implicitContextCache();
}

public static String contextDeprecationMessage(Settings settings) {
return new ScriptService.ContextSettings(settings).deprecationMessage();
}

/**
* This is overridden in tests to disable compilation rate limiting.
*/
Expand Down Expand Up @@ -345,53 +353,26 @@ void registerClusterSettingsListeners(ClusterSettings clusterSettings) {
* when using the general cache.
*/
void validateCacheSettings(Settings settings) {
boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE);
if (useContext) {
ContextSettings contextSettings = new ContextSettings(settings, contexts.keySet());
if (contextSettings.useContextSet) {
deprecationLogger.warn(DeprecationCategory.SCRIPTING, "scripting-context-cache", USE_CONTEXT_RATE_KEY_DEPRECATION_MESSAGE);
} else if (contextSettings.hasContextSettings()) {
deprecationLogger.warn(DeprecationCategory.SCRIPTING, "scripting-context-cache", contextSettings.deprecationMessage());
}
List<Setting.AffixSetting<?>> affixes = Arrays.asList(
SCRIPT_MAX_COMPILATIONS_RATE_SETTING,
SCRIPT_CACHE_EXPIRE_SETTING,
SCRIPT_CACHE_SIZE_SETTING
);
List<String> customRates = new ArrayList<>();
List<String> keys = new ArrayList<>();
for (Setting.AffixSetting<?> affix : affixes) {
for (String context : affix.getAsMap(settings).keySet()) {
String s = affix.getConcreteSettingForNamespace(context).getKey();
if (contexts.containsKey(context) == false) {
throw new IllegalArgumentException("Context [" + context + "] doesn't exist for setting [" + s + "]");
}
keys.add(s);
if (affix.equals(SCRIPT_MAX_COMPILATIONS_RATE_SETTING)) {
customRates.add(s);
}
}
}
if (useContext == false && keys.isEmpty() == false) {
keys.sort(Comparator.naturalOrder());
throw new IllegalArgumentException(
"Context cache settings ["
+ String.join(", ", keys)
+ "] requires ["
+ SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey()
+ "] to be ["
+ USE_CONTEXT_RATE_KEY
+ "]"
);
if (contextSettings.incompatibleSettings()) {
throw new IllegalArgumentException(contextSettings.incompatibleSettingsMessage());
}
if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stu-elastic You mentioned this setting is only used as part of testing. If a user were to set this are there any other problems that may occur?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an internal, undocumented setting for tests. If they set this they'll never have rate limits, updating other settings with respect to this one is unsupported.

if (customRates.size() > 0) {
customRates.sort(Comparator.naturalOrder());
if (contextSettings.compilationContexts.size() > 0) {
throw new IllegalArgumentException(
"Cannot set custom context compilation rates ["
+ String.join(", ", customRates)
+ String.join(", ", contextSettings.contextCompilationKeys())
+ "] if compile rates disabled via ["
+ SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.getKey()
+ "]"
);
}
if (useContext == false && SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.exists(settings)) {
if (contextSettings.useContextSet == false && contextSettings.isGeneralCompilationRateSet) {
throw new IllegalArgumentException(
"Cannot set custom general compilation rates ["
+ SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey()
Expand All @@ -405,6 +386,144 @@ void validateCacheSettings(Settings settings) {
}
}

/**
* Collect settings related to script context and general caches.
*
* The general cache is used by default.
* The context cache is used if {@code script.max_compilations_rate} is {@code "use-context"}, a deprecated value.
* The context cache is used implicitly if {@code script.max_compilations_rate} is unset and any of the context
* cache family of settings is used:
* {@code script.context.*.max_compilations_rate}, {@link ScriptService#SCRIPT_MAX_COMPILATIONS_RATE_SETTING}
* {@code script.context.*.cache_max_size}, {@link ScriptService#SCRIPT_CACHE_SIZE_SETTING}
* {@code script.context.*.cache_expire}, {@link ScriptService#SCRIPT_CACHE_EXPIRE_SETTING}
*/
public static class ContextSettings {
public final Settings settings;
public final boolean useContextSet;
public final boolean isGeneralCompilationRateSet;
public final ScriptCache.CompilationRate generalCompilationRate;
public final List<String> compilationContexts;
public final List<String> sizeContexts;
public final List<String> expireContexts;

public ContextSettings(Settings settings, Set<String> contexts) {
this.settings = settings;
generalCompilationRate = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings);
useContextSet = generalCompilationRate.equals(USE_CONTEXT_RATE_VALUE);
isGeneralCompilationRateSet = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.exists(settings);
compilationContexts = getContexts(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, settings, contexts);
sizeContexts = getContexts(SCRIPT_CACHE_SIZE_SETTING, settings, contexts);
expireContexts = getContexts(SCRIPT_CACHE_EXPIRE_SETTING, settings, contexts);
}

public ContextSettings(Settings settings) {
this(settings, Collections.emptySet());
}

protected static List<String> getContexts(Setting.AffixSetting<?> setting, Settings settings, Set<String> contexts) {
List<String> contextSettings = new ArrayList<>();
for (String context : setting.getAsMap(settings).keySet()) {
if (contexts.isEmpty() == false && contexts.contains(context) == false) {
String settingKey = setting.getConcreteSettingForNamespace(context).getKey();
throw new IllegalArgumentException("Context [" + context + "] doesn't exist for setting [" + settingKey + "]");
}
contextSettings.add(context);
}
contextSettings.sort(Comparator.naturalOrder());
return contextSettings;
}

/** Are there any context specific settings */
public boolean hasContextSettings() {
return compilationContexts.isEmpty() == false || expireContexts.isEmpty() == false || sizeContexts.isEmpty() == false;
}

/** deprecation message for implicitly using the context cache */
public String deprecationMessage() {
// Implicitly using the script context cache is deprecated, remove the following deprecated settings to use the script general
// cache.
if (hasContextSettings() == false) {
return "";
}
List<String> settingsKeys = new ArrayList<>();
settingsKeys.addAll(fullKeys(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, compilationContexts));
settingsKeys.addAll(fullKeys(SCRIPT_CACHE_SIZE_SETTING, sizeContexts));
settingsKeys.addAll(fullKeys(SCRIPT_CACHE_EXPIRE_SETTING, expireContexts));
settingsKeys.sort(Comparator.naturalOrder());
return "Implicitly using the script context cache is deprecated, remove settings "
+ "["
+ String.join(", ", settingsKeys)
+ "] to use the script general cache.";
}

/** the context specific max compilation keys */
public List<String> contextCompilationKeys() {
return fullKeys(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, compilationContexts);
}

/** the full keys for the contexts in the context affix setting */
protected static List<String> fullKeys(Setting.AffixSetting<?> affix, List<String> contexts) {
return contexts.stream().map(ctx -> affix.getConcreteSettingForNamespace(ctx).getKey()).collect(Collectors.toList());
}

/**
* Should the context cache be used? This is true if "use-context" is set explicitly or implicitly, see above for implicit
* definition.
*/
public boolean useContextCache() {
return useContextSet || implicitContextCache();
}

/**
* Implicitly use the script context cache. False if context cache is explicitly used as well as context cache is unused.
*/
public boolean implicitContextCache() {
return useContextSet == false && hasContextSettings() && isGeneralCompilationRateSet == false;
}

/**
* Is the set of settings incompatible? This is the case if:
* 1) {@code script.max_compilations_rate}, {@link ScriptService#SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING} is set but not
* set to "use-context".
* 2) Any of the context cache family of settings is set.
*/
public boolean incompatibleSettings() {
return useContextSet == false && hasContextSettings() && isGeneralCompilationRateSet;
}

/**
* All context specific settings
*/
public List<String> contextSettings() {
List<String> contextSettings = fullKeys(SCRIPT_MAX_COMPILATIONS_RATE_SETTING, compilationContexts);
contextSettings.addAll(fullKeys(SCRIPT_CACHE_SIZE_SETTING, sizeContexts));
contextSettings.addAll(fullKeys(SCRIPT_CACHE_EXPIRE_SETTING, expireContexts));
return contextSettings;
}

/**
* Error message if there are incompatible settings.
*/
public String incompatibleSettingsMessage() {
if (incompatibleSettings() == false) {
return "";
}
List<String> incompatible = contextSettings();
return "Context cache settings ["
+ String.join(",", incompatible)
+ "] are incompatible with ["
+ SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey()
+ "] set to non-default value ["
+ generalCompilationRate
+ "]."
+ " Either remove the incompatible settings (recommended) or set ["
+ SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.getKey()
+ "] to ["
+ USE_CONTEXT_RATE_KEY
+ "] to use per-context settings";
}
}

@Override
public void close() throws IOException {
IOUtils.close(engines.values());
Expand Down Expand Up @@ -692,10 +811,10 @@ public void applyClusterState(ClusterChangedEvent event) {

void setCacheHolder(Settings settings) {
CacheHolder current = cacheHolder.get();
boolean useContext = SCRIPT_GENERAL_MAX_COMPILATIONS_RATE_SETTING.get(settings).equals(USE_CONTEXT_RATE_VALUE);
ContextSettings contextSettings = new ContextSettings(settings, contexts.keySet());

if (current == null) {
if (useContext) {
if (contextSettings.useContextCache()) {
cacheHolder.set(contextCacheHolder(settings));
} else {
cacheHolder.set(generalCacheHolder(settings));
Expand All @@ -704,7 +823,7 @@ void setCacheHolder(Settings settings) {
}

// Update
if (useContext) {
if (contextSettings.useContextCache()) {
if (current.general != null) {
// Flipping to context specific
cacheHolder.set(contextCacheHolder(settings));
Expand Down
Loading