Skip to content

[ES docker] Fixing script compilation settings to resolve CI errors#551

Merged
P1llus merged 1 commit intoelastic:masterfrom
P1llus:fix_docker_script_compilation
Oct 25, 2021
Merged

[ES docker] Fixing script compilation settings to resolve CI errors#551
P1llus merged 1 commit intoelastic:masterfrom
P1llus:fix_docker_script_compilation

Conversation

@P1llus
Copy link
Copy Markdown
Member

@P1llus P1llus commented Oct 25, 2021 •
edited
Loading

This PR resolves errors like this:

�[36melasticsearch_1              |�[0m java.lang.IllegalArgumentException: Context cache settings [script.context.ingest.cache_max_size, script.context.processor_conditional.cache_max_size, script.context.template.cache_max_size, script.context.template.max_compilations_rate] requires [script.max_compilations_rate] to be [use-context]
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.script.ScriptService.validateCacheSettings(ScriptService.java:289)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.script.ScriptService.<init>(ScriptService.java:214)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.node.Node.newScriptService(Node.java:1296)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.node.Node.<init>(Node.java:444)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.node.Node.<init>(Node.java:291)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.bootstrap.Bootstrap$5.<init>(Bootstrap.java:221)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.bootstrap.Bootstrap.setup(Bootstrap.java:221)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.bootstrap.Bootstrap.init(Bootstrap.java:401)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:167)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.bootstrap.Elasticsearch.execute(Elasticsearch.java:158)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:75)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:114)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.cli.Command.main(Command.java:79)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:123)
�[36melasticsearch_1              |�[0m 	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:81)
�[36melasticsearch_1              |�[0m For complete error details, refer to the log at /usr/share/elasticsearch/logs/docker-cluster.log

However this is a short term resolution. From 7.16+ it is deprecated to set context specific settings for compilation rates etc, all of these should be removed:

    - "script.max_compilations_rate=use-context"
    - "script.context.template.max_compilations_rate=unlimited"
    - "script.context.ingest.cache_max_size=2000"
    - "script.context.processor_conditional.cache_max_size=2000"
    - "script.context.template.cache_max_size=2000"

To make this PR work with both older and newer versions however, I have simply added the setting that was missing, so that build can pass on both new and old.

Deprecation notice: https://www.elastic.co/guide/en/elasticsearch/reference/7.16/migrating-7.16.html#breaking_716_cluster_deprecations

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Oct 25, 2021

Pinging @elastic/integrations (Team:Integrations)

@P1llus P1llus requested a review from mtojek October 25, 2021 09:07
mtojek
mtojek approved these changes Oct 25, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

LGTM if it helps with problems with starting the 7.16.0 stack.

@P1llus
Copy link
Copy Markdown
Member Author

P1llus commented Oct 25, 2021 •
edited
Loading

Just waiting for a confirmation that it will still work in 8.0. Or if the defaults in 7.14 is already good enough so these could be removed, because setting these limits is usually a way to resolve painless script compilation rates in older versions.

@mtojek mtojek mentioned this pull request Oct 25, 2021
Closed
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Oct 25, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-10-25T09:05:24.623+0000

  • Duration: 35 min 41 sec

  • Commit: 0d44049

Test stats 🧪

Test Results
Failed 0
Passed 438
Skipped 4
Total 442

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@P1llus P1llus mentioned this pull request Oct 25, 2021
Merged
2 tasks
@P1llus
Copy link
Copy Markdown
Member Author

P1llus commented Oct 25, 2021

I will merge this for now, to fix CI for any package with 7.16 version constraints. We will discuss 8.0 separately.

@P1llus P1llus merged commit a0c6664 into elastic:master Oct 25, 2021
@mdelapenya mdelapenya mentioned this pull request Oct 26, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mtojek mtojek mtojek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Team:Integrations Label for the Integrations team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@P1llus @elasticmachine @mtojek