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

Jump to bottom

Script settings validation breaks mixed version cluster (7.15 + 7.16) #81486

Closed
tvernum opened this issue Dec 8, 2021 · 4 comments · Fixed by #81552
Closed

Script settings validation breaks mixed version cluster (7.15 + 7.16) #81486

tvernum opened this issue Dec 8, 2021 · 4 comments · Fixed by #81552
Labels
blocker >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache Team:Core/Infra Meta label for core/infra team

Comments

@tvernum
Copy link
Contributor

tvernum commented Dec 8, 2021 •
edited
Loading

In #79508 the default value for script.max_compilations_rate was changed from "use-context" to "150/5m".

That means, that validateCacheSetting now treats useContext as false, and rejects any cluster settings on a context.
https://github.com/elastic/elasticsearch/blob/v7.16.0/server/src/main/java/org/elasticsearch/script/ScriptService.java#L349-L360

When in a mixed version cluster, the 7.15 node will accept cluster settings such as

PUT /_cluster/settings
{
  "persistent": {
    "script.context.template.max_compilations_rate": "5000/5m"
  }
}

But the 7.16 version will reject them, and will fail to apply the cluster state on the local node.

Consequently, rolling upgrades with context settings will fail unless script.max_compilations_rate is explicitly set to use-context on the 7.15 node.
@tvernum tvernum added >bug blocker :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache labels Dec 8, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Dec 8, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@stu-elastic
Copy link
Contributor

To resolve this, we'll change the interpretation of script.max_compilation_rate.

Users who do not set the script.max_compilation_rate setting and have context settings will implicitly use the context cache. Implicitly using the context cache this way is deprecated and will result in a deprecation warning with the same key as the warning for setting above to "use-context".

We will reject setting script.max_compilation_rate to any value other than "use-context" if the user has context settings.

cc: @colings86.

stu-elastic added a commit to stu-elastic/elasticsearch that referenced this issue Dec 8, 2021
@stu-elastic
Script: Implicit context cache
c661479 
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 stu-elastic mentioned this issue Dec 8, 2021
Merged
stu-elastic added a commit that referenced this issue Dec 8, 2021
@stu-elastic
Script: Implicit context cache (#81552)
ba6c17d 
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
stu-elastic added a commit to stu-elastic/elasticsearch that referenced this issue Dec 8, 2021
@stu-elastic
Script: Implicit context cache (elastic#81552)
e02f624 
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 issue Dec 8, 2021
@stu-elastic
[7.16] Script: Implicit context cache (elastic#81552)
2595b18 
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 issue Dec 8, 2021
@stu-elastic
Script: Implicit context cache (#81552) (#81559)
c4b1e8c 
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 stu-elastic reopened this Dec 8, 2021
@stu-elastic
Copy link
Contributor

I'm leaving this open until the fix is fully backported.

stu-elastic added a commit that referenced this issue Dec 8, 2021
@stu-elastic
[7.16] Script: Implicit context cache (#81552) (#81560)
7d8f0b3 
* [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
@stu-elastic
Copy link
Contributor

Backport complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
blocker >bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache Team:Core/Infra Meta label for core/infra team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Script: Implicit context cache stu-elastic/elasticsearch
3 participants
@tvernum @elasticmachine @stu-elastic