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.

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

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

Merged
merged 4 commits into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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).

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).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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


Expand All @@ -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)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we lowered it to 11

JAVA_HOME : /usr/lib/jvm/java-11-openjdk-amd64
=======================================

...
Expand Down Expand Up @@ -133,7 +133,7 @@ Use `-Dtests.opensearch.` to pass additional settings to the running instance. F

### IntelliJ IDEA

When importing into IntelliJ you will need to define an appropriate JDK. The convention is that **this SDK should be named "14"**, and the project import will detect it automatically. For more details on defining an SDK in IntelliJ please refer to [this documentation](https://www.jetbrains.com/help/idea/sdk.html#define-sdk). Note that SDK definitions are global, so you can add the JDK from any project, or after project import. Importing with a missing JDK will still work, IntelliJ will report a problem and will refuse to build until resolved.
When importing into IntelliJ you will need to define an appropriate JDK. The convention is that **this SDK should be named "11"**, and the project import will detect it automatically. For more details on defining an SDK in IntelliJ please refer to [this documentation](https://www.jetbrains.com/help/idea/sdk.html#define-sdk). Note that SDK definitions are global, so you can add the JDK from any project, or after project import. Importing with a missing JDK will still work, IntelliJ will report a problem and will refuse to build until resolved.

You can import the OpenSearch project into IntelliJ IDEA as follows.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@
# specific language governing permissions and limitations
# under the License.
#
OPENSEARCH_BUILD_JAVA=openjdk14
OPENSEARCH_RUNTIME_JAVA=openjdk14
OPENSEARCH_BUILD_JAVA=openjdk11
OPENSEARCH_RUNTIME_JAVA=java8
GRADLE_TASK=build
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Random;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -75,7 +76,6 @@ public class GlobalBuildInfoPlugin implements Plugin<Project> {
private static final String DEFAULT_LEGACY_VERSION_JAVA_FILE_PATH = "server/src/main/java/org/opensearch/LegacyESVersion.java";
private static final String DEFAULT_VERSION_JAVA_FILE_PATH = "server/src/main/java/org/opensearch/Version.java";
private static Integer _defaultParallel = null;
private static Boolean _isBundledJdkSupported = null;

private final JavaInstallationRegistry javaInstallationRegistry;
private final ObjectFactory objects;
Expand All @@ -101,7 +101,8 @@ 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();
Copy link
Collaborator Author

@reta reta Oct 14, 2021

Choose a reason for hiding this comment

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

@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:

image

Probably could backport it to 1.x as well.

Copy link
Member

Choose a reason for hiding this comment

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

Nice one. Agreed.

Copy link
Collaborator

@tlfeng tlfeng Oct 14, 2021

Choose a reason for hiding this comment

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

Impressed by your enquiring mind! 🏆

File runtimeJavaHome = runtimeJavaHomeOpt.orElse(Jvm.current().getJavaHome());

File rootDir = project.getRootDir();
GitInfo gitInfo = gitInfo(rootDir);
Expand All @@ -113,7 +114,7 @@ public void apply(Project project) {
params.reset();
params.setRuntimeJavaHome(runtimeJavaHome);
params.setRuntimeJavaVersion(determineJavaVersion("runtime java.home", runtimeJavaHome, minimumRuntimeVersion));
params.setIsRutimeJavaHomeSet(Jvm.current().getJavaHome().equals(runtimeJavaHome) == false);
params.setIsRutimeJavaHomeSet(runtimeJavaHomeOpt.isPresent());
params.setRuntimeJavaDetails(getJavaInstallation(runtimeJavaHome).getImplementationName());
params.setJavaVersions(getAvailableJavaVersions(minimumCompilerVersion));
params.setMinimumCompilerVersion(minimumCompilerVersion);
Expand Down Expand Up @@ -262,14 +263,14 @@ private static void throwInvalidJavaHomeException(String description, File javaH
throw new GradleException(message);
}

private static File findRuntimeJavaHome() {
private static Optional<File> findRuntimeJavaHome() {
String runtimeJavaProperty = System.getProperty("runtime.java");

if (runtimeJavaProperty != null) {
return new File(findJavaHome(runtimeJavaProperty));
return Optional.of(new File(findJavaHome(runtimeJavaProperty)));
}

return System.getenv("RUNTIME_JAVA_HOME") == null ? Jvm.current().getJavaHome() : new File(System.getenv("RUNTIME_JAVA_HOME"));
return System.getenv("RUNTIME_JAVA_HOME") == null ? Optional.empty() : Optional.of(new File(System.getenv("RUNTIME_JAVA_HOME")));
}

private static String findJavaHome(String version) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@
public class DistroTestPlugin implements Plugin<Project> {
private static final String SYSTEM_JDK_VERSION = "8u242+b08";
private static final String SYSTEM_JDK_VENDOR = "adoptopenjdk";
private static final String GRADLE_JDK_VERSION = "14+36@076bab302c7b4508975440c56f6cc26a";
private static final String GRADLE_JDK_VENDOR = "openjdk";
private static final String GRADLE_JDK_VERSION = "17+35";
private static final String GRADLE_JDK_VENDOR = "adoptium";

// all distributions used by distro tests. this is temporary until tests are per distribution
private static final String EXAMPLE_PLUGIN_CONFIGURATION = "examplePlugin";
Expand Down