Script: Implicit context cache #81552
Merged
Conversation
This file contains 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
The script context cache is deprecated. In 7.16 the default value of `script.max_compilation_rate` was switched from `"use-context"`, to `"150/5m". That means uses would have to explicitly set "use-context" to use any of the context cache families of settings: `script.context.*.max_compilations_rate` `script.context.*.cache_max_size` `script.context.*.cache_expire` On upgrades to 7.16, if the customer was using the default and had set any of those settings without setting `script.max_compilation_rate: "use-context"`, the upgrade would be rejected. To avoid an unintentional breaking change, the script service will now **implicitly** use the script context cache if `script.max_compilation_rate` is **unset** and any of the context cache family of settings is set. The context cache will also be used if `script.max_compilation_rate: "use-context"`, as before. Fixes: elastic#81486
stu-elastic
added
>bug
:Core/Infra/Scripting
|
Hi @stu-elastic, I've created a changelog YAML for you. |
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
jdconrad
approved these changes
Dec 8, 2021
There was a problem hiding this comment.
Thanks for the quick work on this! @stu-elastic walked me through this, and the logic seems sound.
| + "]" | ||
| ); | ||
| if (contextSettings.incompatibleSettings()) { | ||
| throw new IllegalArgumentException(contextSettings.incompatibleSettingsMessage()); | ||
| } | ||
| if (SCRIPT_DISABLE_MAX_COMPILATIONS_RATE_SETTING.get(settings)) { |
There was a problem hiding this comment.
@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?
There was a problem hiding this comment.
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.
💔 Backport failed
You can use sqren/backport to manually backport by running |
stu-elastic
added a commit
to stu-elastic/elasticsearch
that referenced
this pull request
Dec 8, 2021
The script context cache is deprecated. In 7.16 the default value of `script.max_compilation_rate` was switched from `"use-context"`, to `"150/5m"`. That means uses would have to explicitly set `"use-context"` to use any of the context cache families of settings: `script.context.*.max_compilations_rate` `script.context.*.cache_max_size` `script.context.*.cache_expire` On upgrades to 7.16, if the customer was using the default and had set any of those settings without setting `script.max_compilation_rate: "use-context"`, the upgrade would be fail. To avoid an unintentional breaking change, the script service will now **implicitly** use the script context cache if `script.max_compilation_rate` is **unset** and any of the context cache family of settings is set. The context cache will also be used if `script.max_compilation_rate: "use-context"`, as before. Fixes: elastic#81486
stu-elastic
added a commit
to stu-elastic/elasticsearch
that referenced
this pull request
Dec 8, 2021
The script context cache is deprecated. In 7.16 the default value of `script.max_compilation_rate` was switched from `"use-context"`, to `"150/5m". That means uses would have to explicitly set "use-context" to use any of the context cache families of settings: `script.context.*.max_compilations_rate` `script.context.*.cache_max_size` `script.context.*.cache_expire` On upgrades to 7.16, if the customer was using the default and had set any of those settings without setting `script.max_compilation_rate: "use-context"`, the upgrade would be rejected. To avoid an unintentional breaking change, the script service will now **implicitly** use the script context cache if `script.max_compilation_rate` is **unset** and any of the context cache family of settings is set. The context cache will also be used if `script.max_compilation_rate: "use-context"`, as before. Fixes: elastic#81486 Backport: ba6c17d
stu-elastic
added a commit
that referenced
this pull request
Dec 8, 2021
The script context cache is deprecated. In 7.16 the default value of `script.max_compilation_rate` was switched from `"use-context"`, to `"150/5m"`. That means uses would have to explicitly set `"use-context"` to use any of the context cache families of settings: `script.context.*.max_compilations_rate` `script.context.*.cache_max_size` `script.context.*.cache_expire` On upgrades to 7.16, if the customer was using the default and had set any of those settings without setting `script.max_compilation_rate: "use-context"`, the upgrade would be fail. To avoid an unintentional breaking change, the script service will now **implicitly** use the script context cache if `script.max_compilation_rate` is **unset** and any of the context cache family of settings is set. The context cache will also be used if `script.max_compilation_rate: "use-context"`, as before. Fixes: #81486 Backport: ba6c17d
stu-elastic
added a commit
that referenced
this pull request
Dec 8, 2021
* [7.16] Script: Implicit context cache (#81552) The script context cache is deprecated. In 7.16 the default value of `script.max_compilation_rate` was switched from `"use-context"`, to `"150/5m". That means uses would have to explicitly set "use-context" to use any of the context cache families of settings: `script.context.*.max_compilations_rate` `script.context.*.cache_max_size` `script.context.*.cache_expire` On upgrades to 7.16, if the customer was using the default and had set any of those settings without setting `script.max_compilation_rate: "use-context"`, the upgrade would be rejected. To avoid an unintentional breaking change, the script service will now **implicitly** use the script context cache if `script.max_compilation_rate` is **unset** and any of the context cache family of settings is set. The context cache will also be used if `script.max_compilation_rate: "use-context"`, as before. Fixes: #81486 Backport: ba6c17d
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.
The script context cache is deprecated.
In 7.16 the default value of
script.max_compilation_ratewas switched from"use-context",to
"150/5m".That means uses would have to explicitly set
"use-context"to use any of the context cache families of settings:script.context.*.max_compilations_ratescript.context.*.cache_max_sizescript.context.*.cache_expireOn upgrades to 7.16, if the customer was using the default and had set any of those settings without setting
script.max_compilation_rate: "use-context", the upgrade would be fail.To avoid an unintentional breaking change, the script service will now implicitly use the script context cache if
script.max_compilation_rateis unset and any of the context cache family of settings is set.The context cache will also be used if
script.max_compilation_rate: "use-context", as before.Fixes: #81486