Adding support for JDK17 and removing JDK8 #2025

martin-gaievski · 2022-02-01T01:39:07Z

Description

Making necessary adjustments to make build compatible with JDK17:

add add-exports arguments due to java.lang.IllegalAccessError when spotlessJavaCheck uses classes from com.sun.*
remove bad javadoc formatting as this generates warnings and build is running with -Werror option

Changing runtime java version from 8 to 11.

Issues Resolved

Remove support for Java 8, add support for Java 17

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Martin Gaievski <gaievski@amazon.com>

opensearch-ci-bot · 2022-02-01T01:40:04Z

Can one of the admins verify this patch?

opensearch-ci-bot · 2022-02-01T02:02:10Z

❌ Gradle Check failure 988b457
Log 2143

Reports 2143

dreamer-89 · 2022-02-01T02:08:46Z

Gradle check is failing testWithKeywordLongAndMissingBucket test which seems flaky. Its worth retyring Gradle check again.

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.testWithKeywordLongAndMissingBucket" -Dtests.seed=4E1342EC6B964D83 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=zh-Hans-CN -Dtests.timezone=Europe/Isle_of_Man -Druntime.java=17

org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests > testWithKeywordLongAndMissingBucket FAILED
    org.junit.ComparisonFailure: expected:<{keyword=null, long=[null]}> but was:<{keyword=null, long=[100]}>
        at __randomizedtesting.SeedInfo.seed([4E1342EC6B964D83:AF2CA28B0C4823CC]:0)
        at org.junit.Assert.assertEquals(Assert.java:117)
        at org.junit.Assert.assertEquals(Assert.java:146)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.lambda$testWithKeywordLongAndMissingBucket$85(CompositeAggregatorTests.java:1057)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.executeTestCase(CompositeAggregatorTests.java:2712)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.testSearchCase(CompositeAggregatorTests.java:2660)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.testWithKeywordLongAndMissingBucket(CompositeAggregatorTests.java:1045)

reta · 2022-02-01T02:18:28Z

gradle.properties

@@ -11,7 +11,12 @@

 org.gradle.warning.mode=none
 org.gradle.parallel=true
-org.gradle.jvmargs=-Xmx3g -XX:+HeapDumpOnOutOfMemoryError -Xss2m
+org.gradle.jvmargs=-Xmx3g -XX:+HeapDumpOnOutOfMemoryError -Xss2m \
+    --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \


I am wondering why these exports are needed? AFAIK OpenSearch does not use Java Compiler API, do you have an example at hand? Thank you.

hey Andriy,
Options are required to overcome the issue from spotlessJavaCheck task, it's called from OS formatting.gradle script.

example of stack-trace:

Caused by: java.lang.IllegalAccessError: class com.google.googlejavaformat.java.RemoveUnusedImports (in unnamed module @0x63239047) cannot access class com.sun.tools.javac.util.Context (in module jdk.compiler) because module jdk.compiler does not export com.sun.tools.javac.util to unnamed module @0x63239047 at com.google.googlejavaformat.java.RemoveUnusedImports.removeUnusedImports(RemoveUnusedImports.java:187) ... 139 more

some public discussions on the issue and possible workaround - google/google-java-format#612, https://stackoverflow.com/questions/58082298/gradle-run-with-add-exports, https://zenn.dev/yuta_saito/articles/8b185a3786088b.
^ @reta

Ah ... thank you @martin-gaievski , spotless again ...

martin-gaievski · 2022-02-01T17:03:30Z

Gradle check is failing testWithKeywordLongAndMissingBucket test which seems flaky. Its worth retyring Gradle check again.

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.testWithKeywordLongAndMissingBucket" -Dtests.seed=4E1342EC6B964D83 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=zh-Hans-CN -Dtests.timezone=Europe/Isle_of_Man -Druntime.java=17

org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests > testWithKeywordLongAndMissingBucket FAILED
    org.junit.ComparisonFailure: expected:<{keyword=null, long=[null]}> but was:<{keyword=null, long=[100]}>
        at __randomizedtesting.SeedInfo.seed([4E1342EC6B964D83:AF2CA28B0C4823CC]:0)
        at org.junit.Assert.assertEquals(Assert.java:117)
        at org.junit.Assert.assertEquals(Assert.java:146)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.lambda$testWithKeywordLongAndMissingBucket$85(CompositeAggregatorTests.java:1057)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.executeTestCase(CompositeAggregatorTests.java:2712)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.testSearchCase(CompositeAggregatorTests.java:2660)
        at org.opensearch.search.aggregations.bucket.composite.CompositeAggregatorTests.testWithKeywordLongAndMissingBucket(CompositeAggregatorTests.java:1045)

hey Suraj,
How can I restart the check (Gradle check that you've mentioned)? Checks that are listed in PR's check tab are all green.
^ @dreamer-89

owaiskazi19 · 2022-02-01T17:42:14Z

start gradle check

opensearch-ci-bot · 2022-02-01T18:11:17Z

❌ Gradle Check failure 988b457
Log 2150

Reports 2150

reta · 2022-02-01T18:12:57Z

x Gradle Check failure 988b457 Log 2150

Reports 2150

Hehe ... @owaiskazi19 seems like without our #2024 PR merged, not much hope for builds to succeed :(

* What went wrong:
Execution failed for task ':distribution:docker:buildDockerImage'.
> A failure occurred while executing org.opensearch.gradle.docker.DockerBuildTask$DockerBuildAction
   > Process 'command 'docker'' finished with non-zero exit value 1

owaiskazi19 · 2022-02-01T18:53:51Z

x Gradle Check failure 988b457 Log 2150
Reports 2150

Hehe ... @owaiskazi19 seems like without our #2024 PR merged, not much hope for builds to succeed :(
* What went wrong:
Execution failed for task ':distribution:docker:buildDockerImage'.
> A failure occurred while executing org.opensearch.gradle.docker.DockerBuildTask$DockerBuildAction
   > Process 'command 'docker'' finished with non-zero exit value 1

Yeah. Looks like all the build failed for the same reason. PR is merged. Starting gradle check again.

owaiskazi19 · 2022-02-01T18:54:19Z

start gradle check

opensearch-ci-bot · 2022-02-01T19:50:23Z

❌ Gradle Check failure 988b457
Log 2153

Reports 2153

dblock · 2022-02-02T18:59:45Z

Tests with failures:
 - org.opensearch.cluster.routing.MovePrimaryFirstTests.testClusterGreenAfterPartialRelocation

#1957

dblock · 2022-02-02T18:59:53Z

start gradle check

opensearch-ci-bot · 2022-02-02T19:38:04Z

✅ Gradle Check success 988b457
Log 2182

Reports 2182

dblock · 2022-02-02T20:59:18Z

This overlaps with #1977.

@martin-gaievski want to merge these two and update docs?

Signed-off-by: Martin Gaievski <gaievski@amazon.com>

martin-gaievski · 2022-02-02T21:41:52Z

This overlaps with #1977.

@martin-gaievski want to merge these two and update docs?

sure, added changes from #1977 so now this PR consolidates all changes
^ @dblock

andrross · 2022-02-02T21:44:27Z

The developer guide needs to be updated as well.

Signed-off-by: Martin Gaievski <gaievski@amazon.com>

martin-gaievski · 2022-02-02T22:01:01Z

Ack, updated in next commit

reta · 2022-02-02T22:20:55Z

DEVELOPER_GUIDE.md


-To run the full suite of tests, download and install [JDK 8](https://adoptium.net/releases.html?variant=openjdk8) and [JDK 14](https://jdk.java.net/archive/) and set `JAVA8_HOME`, `JAVA11_HOME`, and `JAVA14_HOME`. They are required by the [backwards compatibility test](./TESTING.md#testing-backwards-compatibility).
+To run the full suite of tests, download and install [JDK 14](https://jdk.java.net/archive/) and set `JAVA11_HOME`, and `JAVA14_HOME`. They are required by the [backwards compatibility test](./TESTING.md#testing-backwards-compatibility).


@martin-gaievski this is interesting, we are not going to backport this change to 1.x, right? In this case, I am not sure we have to drop the JDK-8 requirements for bwc tests, did you have a chance to check it?

@reta As to my knowledge this is not going to be back-ported to 1.x. It's meant to be for 2.0 only as per original requirements from 110.
I've run bwcTest locally for this branch with only JAVA11_HOME and JAVA14_HOME set and it passed without errors.

All set, thank you

opensearch-ci-bot · 2022-02-02T22:23:26Z

✅ Gradle Check success 9660a8c
Log 2196

Reports 2196

opensearch-ci-bot · 2022-02-02T22:44:00Z

✅ Gradle Check success 9d825f2
Log 2197

Reports 2197

dblock

Nice work @martin-gaievski, thanks!

Adding support for JDK17 and removing JDK8

988b457

Signed-off-by: Martin Gaievski <gaievski@amazon.com>

reta reviewed Feb 1, 2022

View reviewed changes

martin-gaievski marked this pull request as ready for review February 1, 2022 18:19

martin-gaievski requested a review from a team as a code owner February 1, 2022 18:19

martin-gaievski marked this pull request as draft February 2, 2022 20:14

martin-gaievski marked this pull request as ready for review February 2, 2022 20:15

Merge overlaping PR, bumping min java version to 11

9660a8c

Signed-off-by: Martin Gaievski <gaievski@amazon.com>

Removing references to JDK8 from dev guide

9d825f2

Signed-off-by: Martin Gaievski <gaievski@amazon.com>

reta reviewed Feb 2, 2022

View reviewed changes

dblock approved these changes Feb 3, 2022

View reviewed changes

dblock merged commit 3093975 into opensearch-project:main Feb 3, 2022

dblock mentioned this pull request Feb 3, 2022

Bump min java version to 11 #1977

Closed

martin-gaievski mentioned this pull request Feb 3, 2022

Default CI Java Version to Java 11, run tests on 8, 14 and 17 opensearch-project/k-NN#220

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for JDK17 and removing JDK8 #2025

Adding support for JDK17 and removing JDK8 #2025

martin-gaievski commented Feb 1, 2022

opensearch-ci-bot commented Feb 1, 2022

opensearch-ci-bot commented Feb 1, 2022

dreamer-89 commented Feb 1, 2022

reta Feb 1, 2022

martin-gaievski Feb 1, 2022

reta Feb 1, 2022

martin-gaievski commented Feb 1, 2022

owaiskazi19 commented Feb 1, 2022

opensearch-ci-bot commented Feb 1, 2022

reta commented Feb 1, 2022

owaiskazi19 commented Feb 1, 2022 •

edited

owaiskazi19 commented Feb 1, 2022

opensearch-ci-bot commented Feb 1, 2022

dblock commented Feb 2, 2022

dblock commented Feb 2, 2022

opensearch-ci-bot commented Feb 2, 2022

dblock commented Feb 2, 2022

martin-gaievski commented Feb 2, 2022

andrross commented Feb 2, 2022

martin-gaievski commented Feb 2, 2022

reta Feb 2, 2022

martin-gaievski Feb 3, 2022

reta Feb 3, 2022

opensearch-ci-bot commented Feb 2, 2022

opensearch-ci-bot commented Feb 2, 2022

dblock left a comment


		To run the full suite of tests, download and install [JDK 8](https://adoptium.net/releases.html?variant=openjdk8) and [JDK 14](https://jdk.java.net/archive/) and set `JAVA8_HOME`, `JAVA11_HOME`, and `JAVA14_HOME`. They are required by the [backwards compatibility test](./TESTING.md#testing-backwards-compatibility).
		To run the full suite of tests, download and install [JDK 14](https://jdk.java.net/archive/) and set `JAVA11_HOME`, and `JAVA14_HOME`. They are required by the [backwards compatibility test](./TESTING.md#testing-backwards-compatibility).

Adding support for JDK17 and removing JDK8 #2025

Adding support for JDK17 and removing JDK8 #2025

Conversation

martin-gaievski commented Feb 1, 2022

Description

Issues Resolved

Check List

opensearch-ci-bot commented Feb 1, 2022

opensearch-ci-bot commented Feb 1, 2022

dreamer-89 commented Feb 1, 2022

reta Feb 1, 2022

Choose a reason for hiding this comment

martin-gaievski Feb 1, 2022

Choose a reason for hiding this comment

reta Feb 1, 2022

Choose a reason for hiding this comment

martin-gaievski commented Feb 1, 2022

owaiskazi19 commented Feb 1, 2022

opensearch-ci-bot commented Feb 1, 2022

reta commented Feb 1, 2022

owaiskazi19 commented Feb 1, 2022 • edited

owaiskazi19 commented Feb 1, 2022

opensearch-ci-bot commented Feb 1, 2022

dblock commented Feb 2, 2022

dblock commented Feb 2, 2022

opensearch-ci-bot commented Feb 2, 2022

dblock commented Feb 2, 2022

martin-gaievski commented Feb 2, 2022

andrross commented Feb 2, 2022

martin-gaievski commented Feb 2, 2022

reta Feb 2, 2022

Choose a reason for hiding this comment

martin-gaievski Feb 3, 2022

Choose a reason for hiding this comment

reta Feb 3, 2022

Choose a reason for hiding this comment

opensearch-ci-bot commented Feb 2, 2022

opensearch-ci-bot commented Feb 2, 2022

dblock left a comment

Choose a reason for hiding this comment

owaiskazi19 commented Feb 1, 2022 •

edited