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

Set target and source compatibility to 11, required by Lucene 9. #2407

Merged
merged 4 commits into from
Mar 8, 2022
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
27 changes: 24 additions & 3 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,38 @@ Fork [opensearch-project/OpenSearch](https://github.com/opensearch-project/OpenS

#### JDK 11

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`.
OpenSearch builds using Java 11 at a minimum, using the Adoptium distribution. 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`. This is configured in [buildSrc/build.gradle](buildSrc/build.gradle) and [distribution/tools/java-version-checker/build.gradle](distribution/tools/java-version-checker/build.gradle).

```
allprojects {
targetCompatibility = JavaVersion.VERSION_11
sourceCompatibility = JavaVersion.VERSION_11
}
```

```
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
```

Download Java 11 from [here](https://adoptium.net/releases.html?variant=openjdk11).

#### JDK 14

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).

#### Runtime JDK
#### JDK 17

By default, the test tasks use bundled JDK runtime, configured in [buildSrc/version.properties](buildSrc/version.properties), and set to JDK 17 (LTS).

```
bundled_jdk_vendor = adoptium
bundled_jdk = 17.0.2+8
```

#### Custom Runtime JDK

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, 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-14`. Alternatively, 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 example `./gradlew -Druntime.java=17 ...` (in this case, the tooling expects `JAVA17_HOME` environment variable to be set).
Other kind of test tasks (integration, cluster, etc.) use the same runtime as `JAVA_HOME`. However, the build also supports compiling with one version of JDK, and testing on a different version. 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-14`. Alternatively, 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 example `./gradlew -Druntime.java=17 ...` (in this case, the tooling expects `JAVA17_HOME` environment variable to be set).

#### Windows

Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ allprojects {
compile.options.compilerArgs << '-Xlint:opens'
compile.options.compilerArgs << '-Xlint:overloads'
compile.options.compilerArgs << '-Xlint:overrides'
compile.options.compilerArgs << '-Xlint:processing'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Weird, but I also cannot reproduce it locally ....

Copy link
Member Author

@dblock dblock Mar 8, 2022

Choose a reason for hiding this comment

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

I saw we suppress it and call it a day for now. This PR is ready.

compile.options.compilerArgs << '-Xlint:-processing'
compile.options.compilerArgs << '-Xlint:rawtypes'
compile.options.compilerArgs << '-Xlint:removal'
compile.options.compilerArgs << '-Xlint:requires-automatic'
Expand Down
4 changes: 2 additions & 2 deletions buildSrc/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ if (project != rootProject) {
apply plugin: 'opensearch.publish'

allprojects {
targetCompatibility = 8
sourceCompatibility = 8
targetCompatibility = JavaVersion.VERSION_11
sourceCompatibility = JavaVersion.VERSION_11
}

// groovydoc succeeds, but has some weird internal exception...
Expand Down
29 changes: 6 additions & 23 deletions buildSrc/src/main/java/org/opensearch/gradle/ReaperPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.gradle.api.Plugin;
import org.gradle.api.Project;

import java.lang.management.ManagementFactory;
import java.nio.file.Path;

/**
Expand All @@ -52,31 +51,15 @@ public void apply(Project project) {

project.getPlugins().apply(GlobalBuildInfoPlugin.class);

Path inputDir = project.getRootDir().toPath().resolve(".gradle").resolve("reaper").resolve("build-" + getProcessId("xx"));
Path inputDir = project.getRootDir()
.toPath()
.resolve(".gradle")
.resolve("reaper")
.resolve("build-" + ProcessHandle.current().pid());

ReaperService service = project.getExtensions()
.create("reaper", ReaperService.class, project, project.getBuildDir().toPath(), inputDir);

project.getGradle().buildFinished(result -> service.shutdown());
}

private static String getProcessId(final String fallback) {
// Note: may fail in some JVM implementations
// therefore fallback has to be provided

// something like '<pid>@<hostname>', at least in SUN / Oracle JVMs
final String jvmName = ManagementFactory.getRuntimeMXBean().getName();
final int index = jvmName.indexOf('@');

if (index < 1) {
// part before '@' empty (index = 0) / '@' not found (index = -1)
return fallback;
}

try {
return Long.toString(Long.parseLong(jvmName.substring(0, index)));
} catch (NumberFormatException e) {
// ignore
}
return fallback;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,7 @@ private Path locateReaperJar() {
InputStream jarInput = this.getClass().getResourceAsStream("/META-INF/reaper.jar");
) {
logger.info("Copying reaper.jar...");
byte[] buffer = new byte[4096];
int len;
while ((len = jarInput.read(buffer)) > 0) {
out.write(buffer, 0, len);
}
jarInput.transferTo(out);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiConsumer;
import java.util.function.Function;
Expand Down Expand Up @@ -908,7 +910,7 @@ private void startOpenSearchProcess() {
} catch (IOException e) {
throw new TestClustersException("Failed to start " + currentConfig.command + " process for " + this, e);
}
// reaper.registerPid(toString(), opensearchProcess.pid());
reaper.registerPid(toString(), opensearchProcess.pid());
}

@Internal
Expand Down Expand Up @@ -975,7 +977,7 @@ public synchronized void stop(boolean tailLogs) {
LOGGER.info("Stopping `{}`, tailLogs: {}", this, tailLogs);
requireNonNull(opensearchProcess, "Can't stop `" + this + "` as it was not started or already stopped.");
// Test clusters are not reused, don't spend time on a graceful shutdown
stopHandle(opensearchProcess, true);
stopProcess(opensearchProcess.toHandle(), true);
reaper.unregister(toString());
if (tailLogs) {
logFileContents("Standard output of node", currentConfig.stdoutFile);
Expand All @@ -1000,9 +1002,9 @@ public void setNameCustomization(Function<String, String> nameCustomizer) {
this.nameCustomization = nameCustomizer;
}

private void stopHandle(Process process, boolean forcibly) {
private void stopProcess(ProcessHandle processHandle, boolean forcibly) {
// No-op if the process has already exited by itself.
if (process.isAlive() == false) {
if (processHandle.isAlive() == false) {
LOGGER.info("Process was not running when we tried to terminate it.");
return;
}
Expand All @@ -1011,53 +1013,51 @@ private void stopHandle(Process process, boolean forcibly) {
// they'll be recorded as having failed and won't restart when the cluster restarts.
// ES could actually be a child when there's some wrapper process like on Windows,
// and in that case the ML processes will be grandchildren of the wrapper.
// List<Process> children = process.children().collect(Collectors.toList());
List<ProcessHandle> children = processHandle.children().collect(Collectors.toList());
try {
// logProcessInfo(
// "Terminating " + currentConfig.command + " process" + (forcibly ? " forcibly " : "gracefully") + ":",
// process.info()
// );
logProcessInfo(
"Terminating " + currentConfig.command + " process" + (forcibly ? " forcibly " : "gracefully") + ":",
processHandle.info()
);

if (forcibly) {
process.destroyForcibly();
processHandle.destroyForcibly();
} else {
process.destroy();
waitForProcessToExit(process);
if (process.isAlive() == false) {
processHandle.destroy();
waitForProcessToExit(processHandle);
if (processHandle.isAlive() == false) {
return;
}
LOGGER.info(
"process did not terminate after {} {}, stopping it forcefully",
OPENSEARCH_DESTROY_TIMEOUT,
OPENSEARCH_DESTROY_TIMEOUT_UNIT
);
process.destroyForcibly();
processHandle.destroyForcibly();
}

waitForProcessToExit(process);
if (process.isAlive()) {
waitForProcessToExit(processHandle);
if (processHandle.isAlive()) {
throw new TestClustersException("Was not able to terminate " + currentConfig.command + " process for " + this);
}
} finally {
// children.forEach(each -> stopHandle(each, forcibly));
children.forEach(each -> stopProcess(each, forcibly));
}

// waitForProcessToExit(process);
// if (process.isAlive()) {
// throw new TestClustersException("Was not able to terminate " + currentConfig.command + " process for " + this);
// }
waitForProcessToExit(processHandle);
if (processHandle.isAlive()) {
throw new TestClustersException("Was not able to terminate " + currentConfig.command + " process for " + this);
}
}

/*
private void logProcessInfo(String prefix, Process info) {
private void logProcessInfo(String prefix, ProcessHandle.Info info) {
LOGGER.info(
prefix + " commandLine:`{}` command:`{}` args:`{}`",
info.commandLine().orElse("-"),
info.command().orElse("-"),
Arrays.stream(info.arguments().orElse(new String[] {})).map(each -> "'" + each + "'").collect(Collectors.joining(" "))
);
}
*/

private void logFileContents(String description, Path from) {
final Map<String, Integer> errorsAndWarnings = new LinkedHashMap<>();
Expand Down Expand Up @@ -1126,14 +1126,16 @@ private String normalizeLogLine(String line) {
return line;
}

private void waitForProcessToExit(Process process) {
private void waitForProcessToExit(ProcessHandle processHandle) {
try {
process.waitFor(OPENSEARCH_DESTROY_TIMEOUT, OPENSEARCH_DESTROY_TIMEOUT_UNIT);
processHandle.onExit().get(OPENSEARCH_DESTROY_TIMEOUT, OPENSEARCH_DESTROY_TIMEOUT_UNIT);
} catch (InterruptedException e) {
LOGGER.info("Interrupted while waiting for {} process", currentConfig.command, e);
Thread.currentThread().interrupt();
} catch (NullPointerException e) {
} catch (ExecutionException e) {
LOGGER.info("Failure while waiting for process to exist", e);
} catch (TimeoutException e) {
LOGGER.info("Timed out waiting for process to exit", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,7 @@ public void unpack(File tarFile, File targetDir) throws IOException {
// copy the file from the archive using a small buffer to avoid heaping
Files.createFile(destination);
try (FileOutputStream fos = new FileOutputStream(destination.toFile())) {
byte[] buffer = new byte[4096];
int len;
while ((len = tar.read(buffer)) > 0) {
fos.write(buffer, 0, len);
}
tar.transferTo(fos);
}
}
if (entry.isSymbolicLink() == false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.gradle.api.tasks.TaskState;

import java.io.ByteArrayOutputStream;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.function.Consumer;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -101,7 +102,7 @@ void checkVersion(Project project, String tool, Pattern versionRegex, int... min
spec.setCommandLine(tool, "--version");
spec.setStandardOutput(pipe);
});
String output = pipe.toString().trim();
String output = pipe.toString(StandardCharsets.UTF_8).trim();
Matcher matcher = versionRegex.matcher(output);
if (matcher.find() == false) {
throw new IllegalStateException(
Expand Down
2 changes: 1 addition & 1 deletion buildSrc/src/main/resources/minimumRuntimeVersion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.8
11
4 changes: 2 additions & 2 deletions client/rest/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis
apply plugin: 'opensearch.build'
apply plugin: 'opensearch.publish'

targetCompatibility = JavaVersion.VERSION_1_8
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_11
sourceCompatibility = JavaVersion.VERSION_11

group = 'org.opensearch.client'
archivesBaseName = 'opensearch-rest-client'
Expand Down
4 changes: 2 additions & 2 deletions client/sniffer/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
apply plugin: 'opensearch.build'
apply plugin: 'opensearch.publish'

targetCompatibility = JavaVersion.VERSION_1_8
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_11
sourceCompatibility = JavaVersion.VERSION_11

group = 'org.opensearch.client'
archivesBaseName = 'opensearch-rest-client-sniffer'
Expand Down
4 changes: 2 additions & 2 deletions client/test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
*/
apply plugin: 'opensearch.build'

targetCompatibility = JavaVersion.VERSION_1_8
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_11
sourceCompatibility = JavaVersion.VERSION_11

group = "${group}.client.test"

Expand Down
5 changes: 3 additions & 2 deletions distribution/tools/java-version-checker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@

apply plugin: 'opensearch.build'

sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11

// targetting very old java versions enables a warning by default on newer JDK: disable it.
compileJava.options.compilerArgs += '-Xlint:-options'

Expand Down
6 changes: 3 additions & 3 deletions libs/core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ if (!isEclipse) {
}

compileJava11Java {
sourceCompatibility = 11
targetCompatibility = 11
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

forbiddenApisJava11 {
if (BuildParams.runtimeJavaVersion < JavaVersion.VERSION_11) {
targetCompatibility = JavaVersion.VERSION_11.getMajorVersion()
targetCompatibility = JavaVersion.VERSION_11
}
replaceSignatureFiles 'jdk-signatures'
}
Expand Down
6 changes: 3 additions & 3 deletions server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,14 @@ if (!isEclipse) {
}

compileJava11Java {
sourceCompatibility = 11
targetCompatibility = 11
sourceCompatibility = JavaVersion.VERSION_11
targetCompatibility = JavaVersion.VERSION_11
}

tasks.named('forbiddenApisJava11').configure {
doFirst {
if (BuildParams.runtimeJavaVersion < JavaVersion.VERSION_11) {
targetCompatibility = JavaVersion.VERSION_11.getMajorVersion()
targetCompatibility = JavaVersion.VERSION_11
}
}
}
Expand Down