Script: Restore the scripting general cache#79453
Merged
stu-elastic merged 14 commits intoelastic:masterfrom Oct 21, 2021
Merged
Script: Restore the scripting general cache#79453stu-elastic merged 14 commits intoelastic:masterfrom
stu-elastic merged 14 commits intoelastic:masterfrom
Conversation
Restore the scripting general cache in preparation for deprecation of the context cache, then removal of the context cache in master. Brings back the general cache settings: * `script.max_compilations_rate` * `script.cache.expire` * `script.cache.max_size` `script.cache.max_size` and `script.cache.expire` are now dynamic settings. The context cache is still default, the switch to general cache as default will happen on context cache deprecation. System script contexts can now opt-out of compilation rate limiting using a flag rather than a sentinel rate limit value. Duplicated defaults for system contexts have been coalesced. Other than that, this code is the same as what was in 7.x to make a `master`-first commit and backport strategy easy. Refs: elastic#62899
…estore-general-cache-pr
Collaborator
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Contributor
Author
|
@elasticmachine update branch |
3 tasks
Contributor
Author
Deprecate the script context cache in favor of the general cache. Users should use the following settings: `script.max_compilations_rate` to set the max compilation rate for user scripts such as filter scripts. Certain script contexts that submit scripts outside of the control of the user are exempted from this rate limit. Examples include runtime fields, ingest and watcher. `script.cache.max_size` to set the max size of the cache. `script.cache.expire` to set the expiration time for entries in the cache. Whats deprecated? `script.max_compilations_rate: use-context`. This special setting value was used to turn on the script context-specific caches. `script.context.$CONTEXT.cache_max_size`, use `script.cache.max_size` instead. `script.context.$CONTEXT.cache_expire`, use `script.cache.expire` instead. `script.context.$CONTEXT.max_compilations_rate`, use `script.max_compilations_rate` instead. The default cache size was increased from `100` to `3000`, which was approximately the max cache size when using context-specific caches. The default compilation rate limit was increased from `75/5m` to `150/5m` to account for increasing uses of scripts. System script contexts can now opt-out of compilation rate limiting using a flag rather than a sentinel rate limit value. 7.16: Script: Deprecate script context cache elastic#79508 Refs: elastic#62899 7.16: Script: Opt-out system contexts from script compilation rate limit elastic#79459 Refs: elastic#62899
Contributor
Author
Contributor
Author
|
@elasticmachine update branch |
Contributor
Author
|
@elasticmachine update branch |
rjernst
approved these changes
Oct 20, 2021
Member
rjernst
left a comment
There was a problem hiding this comment.
Looks good once it passes CI, a couple minor things
| cacheEvictions = in.readVLong(); | ||
| compilationLimitTriggered = in.readVLong(); | ||
| contextStats = in.readList(ScriptContextStats::new); | ||
| compilationLimitTriggered = in.getVersion().onOrAfter(Version.V_7_0_0) ? in.readVLong() : 0; |
Member
There was a problem hiding this comment.
I don't th ink these conditions are needed since this is master, which is only compatible with 7.16+
| out.writeVLong(cacheEvictions); | ||
| out.writeVLong(compilationLimitTriggered); | ||
| out.writeList(contextStats); | ||
| if (out.getVersion().onOrAfter(Version.V_7_0_0)) { |
Member
There was a problem hiding this comment.
Same here, this will always be true
| builder.field(Fields.COMPILATIONS, compilations); | ||
| builder.field(Fields.CACHE_EVICTIONS, cacheEvictions); | ||
| builder.field(Fields.COMPILATION_LIMIT_TRIGGERED, compilationLimitTriggered); | ||
| /* TODO(stu): master only |
…c/elasticsearch into pain_restore-general-cache-pr
Contributor
Author
|
@elasticmachine update branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restore the scripting general cache in preparation for deprecation of
the context cache, then removal of the context cache in master.
Brings back the general cache settings:
script.max_compilations_ratescript.cache.expirescript.cache.max_sizeThe context cache is still default, the switch to general cache as default will happen on context cache deprecation.
System script contexts can now opt-out of compilation rate limiting using a flag rather than a sentinel rate limit value. Duplicated defaults for system contexts have been coalesced.
Refs: #62899