Add JIT compiler excludes for computeCommonPrefixLengthAndBuildHistogram#103112
Add JIT compiler excludes for computeCommonPrefixLengthAndBuildHistogram#103112ChrisHegarty merged 2 commits intoelastic:mainfrom
Conversation
|
Pinging @elastic/es-search (Team:Search) |
|
Hi @ChrisHegarty, I've created a changelog YAML for you. |
|
It's great that we have a fix for Elasticsearch and get unblocked quickly, yet it would be good to protect Lucene from this, given that other Lucene users are exposed to it until the fix is released with the jdk? |
Indeed. This PR is intended as a temporary measure so that we can avoid the backout of the Lucene 9.9.0 upgrade in ES, and restore stability to ES. I am continuing to investigate a possible workaround in Lucene code, which if possible would require a new Lucene release, Maybe 9.9.1 ? (but that not something that we can decide here) |
|
@elasticmachine retest this please Clean CI run, but lets do it again. |
2 similar comments
|
@elasticmachine retest this please Clean CI run, but lets do it again. |
|
@elasticmachine retest this please Clean CI run, but lets do it again. |
…ram (elastic#103112) This commit adds a temporary JIT compile command that excludes the compilation of a couple of Lucene methods which, if compiled, crash the JVM.
💚 Backport successful
|
| "--add-opens=java.management/java.lang.management=ALL-UNNAMED", | ||
| "-XX:+HeapDumpOnOutOfMemoryError" | ||
| "-XX:+HeapDumpOnOutOfMemoryError", | ||
| // REMOVE once bumped to a JDK greater than 21.0.1, https://github.com/elastic/elasticsearch/issues/103004 |
There was a problem hiding this comment.
I'm not sure we'll be able to remove this just because of a JDK upgrade since we still test on older JDKs, unless this fix is backported to JDK 17.
There was a problem hiding this comment.
That's a fair comment @mark-vieira. We're currently in the process of investigating a possible "fix" in Lucene. If this were to happen then these options can be removed. If not, then I'll amend the comment to something more appropriate for its longer term presence.
| @@ -0,0 +1,5 @@ | |||
| pr: 103112 | |||
There was a problem hiding this comment.
FWIW, I'm not sure we need to include a changelog entry given this "bug" never existed in a released version.
This commit upgrades to Lucene 9.9.1. With the upgrade to 9.9.1 we can now remove the compiler excludes added by #103112.
This commit adds a temporary JIT compile command that excludes the compilation of a couple of Lucene methods which, if compiled, crash the JVM.
The excludes are added to:
The Gradle test plugin, so that tests run with the Gradle runner will not compile these methods - this the most impactful change as we see
RangeAggregatorTestsfailing very frequently when run withgradle :server:test.The distribution
jvm.options, so that the distribution is protected from the potential crash. A consequence of this is that the log output will now emit a notice of these excludes - this is benign, e.g.One consequence of no.1 is that
RangeAggregatorTestsmay still crash when run directly in the IDE, but this is of a lesser concern. And since the workaround is temporary (until JDK 21.0.2 is released), then this should be ok.More specifics can be found in #103004, which I will not repeat here.