Modernize and consolidate JDKs usage across all stages of the build. Update JDK-14 requirement, switch to JDK-17 instead #1368

reta · 2021-10-14T16:15:20Z

Signed-off-by: Andriy Redko andriy.redko@aiven.io

Description

Update DEVELOPER_GUIDE.md and java-versions.properties.

Issues Resolved

Part of #1351

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
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.

reta · 2021-10-14T16:15:45Z

DEVELOPER_GUIDE.md

@@ -70,8 +70,8 @@ Start by running the test suite with `gradlew check`. This should complete witho
 OpenSearch Build Hamster says Hello!
  Gradle Version        : 6.6.1
  OS Info               : Linux 5.4.0-1037-aws (amd64)
-  JDK Version           : 14 (JDK)
-  JAVA_HOME             : /usr/lib/jvm/java-14-openjdk-amd64
+  JDK Version           : 11 (JDK)


Since we lowered it to 11

opensearch-ci-bot · 2021-10-14T16:16:03Z

Can one of the admins verify this patch?

reta · 2021-10-14T16:16:18Z

DEVELOPER_GUIDE.md

@@ -49,7 +49,7 @@ Fork [opensearch-project/OpenSearch](https://github.com/opensearch-project/OpenS

 OpenSearch builds using Java 11 at a minimum. This means you must have a JDK 11 installed with the environment variable `JAVA_HOME` referencing the path to Java home for your JDK 11 installation, e.g. `JAVA_HOME=/usr/lib/jvm/jdk-11`.

-By default, tests use the same runtime as `JAVA_HOME`. However, since OpenSearch also supports JDK 8 as the runtime, the build supports compiling with JDK 11 and testing on a different version of JDK runtime. To do this, set `RUNTIME_JAVA_HOME` pointing to the Java home of another JDK installation, e.g. `RUNTIME_JAVA_HOME=/usr/lib/jvm/jdk-8`.
+By default, the test tasks use bundled JDK runtime, configured in `buildSrc/version.properties` and set to JDK 17 (LTS). Other kind of test tasks (integration, cluster, ... ) use the same runtime as `JAVA_HOME`. However, since OpenSearch supports JDK 8/17 as the runtime, the build supports compiling with JDK 11 and testing on a different version of JDK runtime. To do this, set `RUNTIME_JAVA_HOME` pointing to the Java home of another JDK installation, e.g. `RUNTIME_JAVA_HOME=/usr/lib/jvm/jdk-8`. Alernatively, the runtime JDK version could be provided as the command line argument, using combination of `runtime.java=<major JDK version>` property and `JAVA<major JDK version>_HOME` environment variable, for exampe `./gradlew -Druntime.java=17 ...` (in this case, the tooling expects `JAVA17_HOME` enviroment variable to be set).

 To run the full suite of tests you will also need `JAVA8_HOME`, `JAVA11_HOME`, and `JAVA14_HOME`. They are required by the [backwards compatibility test](./TESTING.md#testing-backwards-compatibility).


The JAVA14_HOME is still needed, we checkout 1.x/1.0 branches and they do reference JDK-14

Yeah, JAVA14_HOME is only used for building OpenSearch 1.0, it's a compromise for the smooth transition with Elasticsearch 7.10.2 🙂.

reta · 2021-10-14T16:16:45Z

...egTest/resources/org/opensearch/gradle/internal/fake_git/remote/.ci/java-versions.properties

@@ -27,6 +27,6 @@
 # specific language governing permissions and limitations
 # under the License.
 #
-OPENSEARCH_BUILD_JAVA=openjdk14
-OPENSEARCH_RUNTIME_JAVA=openjdk14
+OPENSEARCH_BUILD_JAVA=openjdk17


@dblock should we lower it to openjdk11 and runtime to 8 like .ci/java-versions.properties?

OPENSEARCH_BUILD_JAVA=openjdk11 OPENSEARCH_RUNTIME_JAVA=java8

Yes, we should!

opensearch-ci-bot · 2021-10-14T16:16:47Z

✅ DCO Check Passed 4420f2c1f2313c6ba4666d936f4c1826d120bf16

opensearch-ci-bot · 2021-10-14T16:18:06Z

✅ Gradle Wrapper Validation success 4420f2c1f2313c6ba4666d936f4c1826d120bf16

opensearch-ci-bot · 2021-10-14T16:25:49Z

✅ Gradle Precommit success 4420f2c1f2313c6ba4666d936f4c1826d120bf16

…Update JDK-14 requirement, switch to JDK-17 instead Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

opensearch-ci-bot · 2021-10-14T16:35:57Z

✅ Gradle Wrapper Validation success e18dbd8

opensearch-ci-bot · 2021-10-14T16:36:47Z

✅ DCO Check Passed e18dbd8

opensearch-ci-bot · 2021-10-14T16:41:52Z

✅ Gradle Precommit success e18dbd8

tlfeng · 2021-10-14T16:43:38Z

DEVELOPER_GUIDE.md

@@ -49,7 +49,7 @@ Fork [opensearch-project/OpenSearch](https://github.com/opensearch-project/OpenS

 OpenSearch builds using Java 11 at a minimum. This means you must have a JDK 11 installed with the environment variable `JAVA_HOME` referencing the path to Java home for your JDK 11 installation, e.g. `JAVA_HOME=/usr/lib/jvm/jdk-11`.

-By default, tests use the same runtime as `JAVA_HOME`. However, since OpenSearch also supports JDK 8 as the runtime, the build supports compiling with JDK 11 and testing on a different version of JDK runtime. To do this, set `RUNTIME_JAVA_HOME` pointing to the Java home of another JDK installation, e.g. `RUNTIME_JAVA_HOME=/usr/lib/jvm/jdk-8`.
+By default, the test tasks use bundled JDK runtime, configured in `buildSrc/version.properties` and set to JDK 17 (LTS). Other kind of test tasks (integration, cluster, ... ) use the same runtime as `JAVA_HOME`. However, since OpenSearch supports JDK 8/17 as the runtime, the build supports compiling with JDK 11 and testing on a different version of JDK runtime. To do this, set `RUNTIME_JAVA_HOME` pointing to the Java home of another JDK installation, e.g. `RUNTIME_JAVA_HOME=/usr/lib/jvm/jdk-8`. Alernatively, the runtime JDK version could be provided as the command line argument, using combination of `runtime.java=<major JDK version>` property and `JAVA<major JDK version>_HOME` environment variable, for exampe `./gradlew -Druntime.java=17 ...` (in this case, the tooling expects `JAVA17_HOME` environment variable to be set).


Hi Andriy, a little concern regarding the expression "OpenSearch supports JDK 8/17 as the runtime". 😄
My concern is if listing 2 JDK versions, will it mislead the readers that only the 2 versions are supported? Actually other versions are able to use as runtime.
In my opinion, the original expression "However, since OpenSearch supports JDK 8 as the runtime" aims to emphasize the lowest JDK version for the runtime.
It's fine if there is actually no ambiguity on this expression.

Absolutely agree, changing the phrasing, thank you

Thanks! While the original expression might also have a little confusion - whether only JDK 8 is supported as the runtime. 😂 Haha, anyway, a good solution might be to put the supporting matrix of OpenSearch and JVM in somewhere in user's documentation, but it's out of scope.
Glad to see you improving the developer guide!

Thanks @tlfeng , I think the build matrix is the goal, hopefully we will get there soon, discussion is ongoing in opensearch-project/opensearch-build#732

Thank you for providing the link 😄 (seems there are many GitHub issues regarding the JDK versions). I will keep an eye on the discussion.

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

opensearch-ci-bot · 2021-10-14T16:49:57Z

✅ Gradle Wrapper Validation success f55b0ea

opensearch-ci-bot · 2021-10-14T16:50:49Z

✅ DCO Check Passed f55b0ea

opensearch-ci-bot · 2021-10-14T17:00:32Z

✅ Gradle Precommit success f55b0ea

opensearch-ci-bot · 2021-10-14T18:36:23Z

✅ Gradle Wrapper Validation success d3ab87c17fe74c8002ee109214f060c5d61e0694

opensearch-ci-bot · 2021-10-14T18:36:47Z

✅ DCO Check Passed d3ab87c17fe74c8002ee109214f060c5d61e0694

opensearch-ci-bot · 2021-10-14T18:40:27Z

❌ Gradle Precommit failure d3ab87c17fe74c8002ee109214f060c5d61e0694
Log 1323

reta · 2021-10-14T18:46:57Z

buildSrc/src/main/java/org/opensearch/gradle/info/GlobalBuildInfoPlugin.java

@@ -101,8 +101,9 @@ public void apply(Project project) {
        JavaVersion minimumCompilerVersion = JavaVersion.toVersion(Util.getResourceContents("/minimumCompilerVersion"));
        JavaVersion minimumRuntimeVersion = JavaVersion.toVersion(Util.getResourceContents("/minimumRuntimeVersion"));

-        File runtimeJavaHome = findRuntimeJavaHome();
-
+        Optional<File> runtimeJavaHomeOpt = findRuntimeJavaHome();


@dblock @tlfeng found this interesting issue (and hopefully fixed it): when RUNTIME_JAVA_HOME is set but is the same as JAVA_HOME, the build ignores RUNTIME_JAVA_HOME altogether and basically uses the whatever is set in buildSrc/version.properties as bundled JDK. Practically, it means that we cannot run build + all tests using the same JDK version. It is very easy to verify:

gradle ':server:test' --tests "org.opensearch.snapshots.SnapshotResiliencyTests.testConcurrentSnapshotDeleteAndDeleteIndex" -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1" -Dtests.locale=th -Dtests.timezone=Asia/Saigon -Druntime.java=11

Will span JDK-11 for Gradle and JDK-17 for tests:

Probably could backport it to 1.x as well.

Nice one. Agreed.

Impressed by your enquiring mind! 🏆

…_JAVA_HOME == JAVA_HOME Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

dblock · 2021-10-14T18:50:42Z

This makes me so happy @reta, thank you, we don't pay you enough to work on OSS and I hope someone does :)
We've had this whole "lower to JDK11" issue open for months and were dragging to address it.

opensearch-ci-bot · 2021-10-14T18:50:47Z

✅ DCO Check Passed fe6157f

opensearch-ci-bot · 2021-10-14T18:51:59Z

✅ Gradle Wrapper Validation success 4349dbf

opensearch-ci-bot · 2021-10-14T18:52:48Z

✅ DCO Check Passed 4349dbf

reta · 2021-10-14T18:53:15Z

Thanks a lot @dblock , happy to help and help to drive it to the completion (with your enormous help), thanks a mill!

opensearch-ci-bot · 2021-10-14T18:55:56Z

✅ Gradle Precommit success 4349dbf

tlfeng · 2021-10-14T22:13:29Z

start gradle check

opensearch-ci-bot · 2021-10-14T22:39:59Z

❌ Gradle Check failure 4349dbf
Log 693

Reports 693

tlfeng · 2021-10-14T22:57:47Z

In Log 693:

Tests with failures:
 - org.opensearch.repositories.azure.AzureBlobStoreRepositoryTests.testIndicesDeletedFromRepository

---

> Task :plugins:repository-azure:internalClusterTest

REPRODUCE WITH: ./gradlew ':plugins:repository-azure:internalClusterTest' --tests "org.opensearch.repositories.azure.AzureBlobStoreRepositoryTests.testIndicesDeletedFromRepository" -Dtests.seed=6D3A4C2E37516B12 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1" -Dtests.locale=vi-VN -Dtests.timezone=Asia/Phnom_Penh -Druntime.java=17

org.opensearch.repositories.azure.AzureBlobStoreRepositoryTests > testIndicesDeletedFromRepository FAILED
    java.lang.AssertionError: AcknowledgedResponse failed - not acked
    Expected: <true>
         but: was <false>

reta · 2021-10-14T23:15:26Z

Thanks a lot @tlfeng, was checking the tests output, this is unexpected:

[2021-10-15T05:27:50,780][WARN ][o.o.t.n.MockNioTransport ] [transport_client_node_s1] Slow execution on network thread [400 milliseconds]
java.lang.RuntimeException: Slow exception on network thread
	at org.opensearch.transport.nio.MockNioTransport$TransportThreadWatchdog.maybeLogElapsedTime(MockNioTransport.java:420) [framework-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
	at org.opensearch.transport.nio.MockNioTransport$TransportThreadWatchdog.unregister(MockNioTransport.java:412) [framework-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
	at org.opensearch.transport.nio.TestEventHandler.handleRead(TestEventHandler.java:167) [framework-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
	at org.opensearch.nio.NioSelector.handleRead(NioSelector.java:433) [opensearch-nio-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
	at org.opensearch.nio.NioSelector.processKey(NioSelector.java:259) [opensearch-nio-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
	at org.opensearch.nio.NioSelector.singleLoop(NioSelector.java:187) [opensearch-nio-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
	at org.opensearch.nio.NioSelector.runLoop(NioSelector.java:144) [opensearch-nio-2.0.0-SNAPSHOT.jar:2.0.0-SNAPSHOT]
	at java.lang.Thread.run(Thread.java:833) [?:?]

AFAIK, this repository plugin tests don't simulate network issues, but I will recheck tomorrow morning.
Thanks again!

tlfeng · 2021-10-14T23:18:28Z

Thanks for your explanation for the repository plugin test 😄. I sometime saw some kinds of network issues with those tests, but didn't investigate more.
The above failure is not reproducible in my host, so I will trigger the check again. start gradle check

opensearch-ci-bot · 2021-10-15T00:03:52Z

✅ Gradle Check success 4349dbf
Log 695

Reports 695

dblock · 2021-10-15T13:03:52Z

The gradle check is still using Java 14.

=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 6.6.1
  OS Info               : Linux 5.4.0-1045-aws (amd64)
  JDK Version           : 14 (AdoptOpenJDK)
  JAVA_HOME             : /var/CITOOL/tools/hudson.model.JDK/JDK14
  Random Testing Seed   : 61237CACA4283E2D
  In FIPS 140 mode      : false
=======================================

I imagine this comes hardcoded from the infrastructure that runs gradle checks. We need it to start picking up Java version from https://github.com/opensearch-project/OpenSearch/blob/main/.ci/java-versions.properties#L16.

@peternied @peterzhuamazon where/how do we make this happen?

reta · 2021-10-15T13:07:18Z

@dblock the JDK-14 is coming from the CI build image, it has JAVA_HOME set to AdoptJDK-14 distribution, the pull request is being finalized (testing it right), should fix that.

reta commented Oct 14, 2021

View reviewed changes

Modernize and consolidate JDKs usage across all stages of the build. …

e18dbd8

…Update JDK-14 requirement, switch to JDK-17 instead Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta force-pushed the issue-1351.2 branch from 4420f2c to e18dbd8 Compare October 14, 2021 16:35

tlfeng reviewed Oct 14, 2021

View reviewed changes

Updating phrasing based on review feedback

f55b0ea

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta commented Oct 14, 2021

View reviewed changes

Fixed runtime Java version usage, it has to be respected when RUNTIME…

fe6157f

…_JAVA_HOME == JAVA_HOME Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta force-pushed the issue-1351.2 branch from d3ab87c to fe6157f Compare October 14, 2021 18:49

Addressing review comments

4349dbf

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

tlfeng added Build Libraries & Interfaces documentation Improvements or additions to documentation v1.2.0 Issues related to version 1.2.0 v2.0.0 Version 2.0.0 labels Oct 14, 2021

dblock approved these changes Oct 15, 2021

View reviewed changes

dblock merged commit 8ea3364 into opensearch-project:main Oct 15, 2021

dblock mentioned this pull request Oct 15, 2021

Lower CI Java Version to Java 11 opensearch-project/opensearch-build#74

Closed

reta mentioned this pull request Oct 15, 2021

[Backport] [1.x] Clarified JDK usage in DEVELOPER_GUIDE.md, fixed minor issue with configuring runtime JDK #1372

Merged

5 tasks

dblock mentioned this pull request Dec 9, 2021

Remove support for Java 8 opensearch-project/opensearch-plugins#110

Closed

18 tasks

reta mentioned this pull request Jan 17, 2022

[Backport] [1.x] Modernize and consolidate JDKs usage across all stages of the build. Use JDK-17 as bundled JDK distribution to run tests #1922

Merged

5 tasks

Modernize and consolidate JDKs usage across all stages of the build. Update JDK-14 requirement, switch to JDK-17 instead #1368

Modernize and consolidate JDKs usage across all stages of the build. Update JDK-14 requirement, switch to JDK-17 instead #1368

Conversation

reta commented Oct 14, 2021

Description

Issues Resolved

Check List

Choose a reason for hiding this comment

opensearch-ci-bot commented Oct 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reta Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

tlfeng Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

reta Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlfeng Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

dblock commented Oct 14, 2021 • edited Loading

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

reta commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

tlfeng commented Oct 14, 2021

opensearch-ci-bot commented Oct 14, 2021

tlfeng commented Oct 14, 2021

reta commented Oct 14, 2021

tlfeng commented Oct 14, 2021

opensearch-ci-bot commented Oct 15, 2021

dblock commented Oct 15, 2021

reta commented Oct 15, 2021

reta Oct 14, 2021 •

edited

Loading

tlfeng Oct 14, 2021 •

edited

Loading

reta Oct 14, 2021 •

edited

Loading

tlfeng Oct 14, 2021 •

edited

Loading

dblock commented Oct 14, 2021 •

edited

Loading