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

Use logs dir as working directory #124966

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,9 @@ echo "Running elasticsearch \$0"

file(distProjectFolder, 'src/config/elasticsearch.properties') << "some propes"
file(distProjectFolder, 'src/config/jvm.options') << """
-Xlog:gc*,gc+age=trace,safepoint:file=logs/gc.log:utctime,level,pid,tags:filecount=32,filesize=64m
-XX:ErrorFile=logs/hs_err_pid%p.log
-XX:HeapDumpPath=data
-Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m
-XX:ErrorFile=hs_err_pid%p.log
# -XX:HeapDumpPath=/heap/dump/path
"""
file(distProjectFolder, 'build.gradle') << """
import org.gradle.api.internal.artifacts.ArtifactAttributes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1435,30 +1435,57 @@ private void tweakJvmOptions(Path configFileRoot) {
Path jvmOptions = configFileRoot.resolve("jvm.options");
try {
String content = new String(Files.readAllBytes(jvmOptions));
Map<String, String> expansions = jvmOptionExpansions();
for (String origin : expansions.keySet()) {
if (content.contains(origin) == false) {
throw new IOException("template property " + origin + " not found in template.");
Map<ReplacementKey, String> expansions = jvmOptionExpansions();
for (var entry : expansions.entrySet()) {
ReplacementKey replacement = entry.getKey();
String key = replacement.key();
if (content.contains(key) == false) {
key = replacement.fallback();
if (content.contains(key) == false) {
throw new IOException("Template property '" + replacement + "' not found in template:\n" + content);
}
}
content = content.replace(origin, expansions.get(origin));
content = content.replace(key, entry.getValue());
}
Files.write(jvmOptions, content.getBytes());
} catch (IOException ioException) {
throw new UncheckedIOException(ioException);
}
}

private Map<String, String> jvmOptionExpansions() {
Map<String, String> expansions = new HashMap<>();
private record ReplacementKey(String key, String fallback) {}

private Map<ReplacementKey, String> jvmOptionExpansions() {
Map<ReplacementKey, String> expansions = new HashMap<>();
Version version = getVersion();
String heapDumpOrigin = getVersion().onOrAfter("6.3.0") ? "-XX:HeapDumpPath=data" : "-XX:HeapDumpPath=/heap/dump/path";
expansions.put(heapDumpOrigin, "-XX:HeapDumpPath=" + confPathLogs);
if (version.onOrAfter("6.2.0")) {
expansions.put("logs/gc.log", confPathLogs.resolve("gc.log").toString());

ReplacementKey heapDumpPathSub;
if (version.before("8.18.0") && version.onOrAfter("6.3.0")) {
heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", "");
} else {
// temporarily fall back to the old substitution so both old and new work during backport
heapDumpPathSub = new ReplacementKey("# -XX:HeapDumpPath=/heap/dump/path", "-XX:HeapDumpPath=data");
}
if (getVersion().getMajor() >= 7) {
expansions.put("-XX:ErrorFile=logs/hs_err_pid%p.log", "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log"));
expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs);

ReplacementKey gcLogSub;
if (version.before("8.18.0") && version.onOrAfter("6.2.0")) {
gcLogSub = new ReplacementKey("logs/gc.log", "");
} else {
// temporarily check the old substitution first so both old and new work during backport
gcLogSub = new ReplacementKey("logs/gc.log", "gc.log");
}
expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString());

ReplacementKey errorFileSub;
if (version.before("8.18.0") && version.getMajor() >= 7) {
errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", "");
} else {
// temporarily check the old substitution first so both old and new work during backport
errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", "-XX:ErrorFile=hs_err_pid%p.log");
}
expansions.put(errorFileSub, "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log"));

return expansions;
}

Expand Down
18 changes: 0 additions & 18 deletions distribution/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,6 @@ subprojects {
final String packagingPathData = "path.data: /var/lib/elasticsearch"
final String pathLogs = "/var/log/elasticsearch"
final String packagingPathLogs = "path.logs: ${pathLogs}"
final String packagingLoggc = "${pathLogs}/gc.log"

String licenseText
if (isTestDistro) {
Expand Down Expand Up @@ -576,23 +575,6 @@ subprojects {
'rpm': packagingPathLogs,
'def': '#path.logs: /path/to/logs'
],
'loggc': [
'deb': packagingLoggc,
'rpm': packagingLoggc,
'def': 'logs/gc.log'
],

'heap.dump.path': [
'deb': "-XX:HeapDumpPath=/var/lib/elasticsearch",
'rpm': "-XX:HeapDumpPath=/var/lib/elasticsearch",
'def': "-XX:HeapDumpPath=data"
],

'error.file': [
'deb': "-XX:ErrorFile=/var/log/elasticsearch/hs_err_pid%p.log",
'rpm': "-XX:ErrorFile=/var/log/elasticsearch/hs_err_pid%p.log",
'def': "-XX:ErrorFile=logs/hs_err_pid%p.log"
],

'scripts.footer': [
/* Debian needs exit 0 on these scripts so we add it here and preserve
Expand Down
6 changes: 3 additions & 3 deletions distribution/src/config/jvm.options
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@

# specify an alternative path for heap dumps; ensure the directory exists and
# has sufficient space
@heap.dump.path@
# -XX:HeapDumpPath=/heap/dump/path

# specify an alternative path for JVM fatal error logs
@error.file@
-XX:ErrorFile=hs_err_pid%p.log

## GC logging
-Xlog:gc*,gc+age=trace,safepoint:file=@loggc@:utctime,level,pid,tags:filecount=32,filesize=64m
-Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static void main(String[] args) throws Exception {
String toolname = getToolName(pinfo.sysprops());
String libs = pinfo.sysprops().getOrDefault("cli.libs", "");

command = CliToolProvider.load(toolname, libs).create();
command = CliToolProvider.load(pinfo.sysprops(), toolname, libs).create();
Terminal terminal = Terminal.DEFAULT;
Runtime.getRuntime().addShutdownHook(createShutdownHook(terminal, command));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
package org.elasticsearch.server.cli;

import org.elasticsearch.common.Strings;
import org.elasticsearch.core.SuppressForbidden;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -106,7 +108,11 @@ private static List<String> flagsFinal(final List<String> userDefinedJvmOptions)
userDefinedJvmOptions.stream(),
Stream.of("-XX:+PrintFlagsFinal", "-version")
).flatMap(Function.identity()).toList();
final Process process = new ProcessBuilder().command(command).start();
final ProcessBuilder builder = new ProcessBuilder().command(command);
// set temp dir as working dir so it is writeable
final Path tmpDir = Files.createTempDirectory("final-flags");
setWorkingDir(builder, tmpDir);
final Process process = builder.start();
final List<String> output = readLinesFromInputStream(process.getInputStream());
final List<String> error = readLinesFromInputStream(process.getErrorStream());
final int status = process.waitFor();
Expand All @@ -124,6 +130,11 @@ private static List<String> flagsFinal(final List<String> userDefinedJvmOptions)
}
}

@SuppressForbidden(reason = "ProcessBuilder takes File")
private static void setWorkingDir(ProcessBuilder builder, Path path) {
builder.directory(path.toFile());
}

private static List<String> readLinesFromInputStream(final InputStream is) throws IOException {
try (InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(isr)) {
return br.lines().toList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,13 @@ interface InvalidLineConsumer {
* and the following JVM options will not be accepted:
* <ul>
* <li>
* {@code 18:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
* {@code 18:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m}
* </li>
* <li>
* {@code 18-:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
* {@code 18-:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m}
* </li>
* <li>
* {@code 18-19:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
* {@code 18-19:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m}
* </li>
* </ul>
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Locale;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;

/**
Expand Down Expand Up @@ -168,7 +169,7 @@ Environment autoConfigureSecurity(
assert secureSettingsLoader(env) instanceof KeyStoreLoader;

String autoConfigLibs = "modules/x-pack-core,modules/x-pack-security,lib/tools/security-cli";
Command cmd = loadTool("auto-configure-node", autoConfigLibs);
Command cmd = loadTool(processInfo.sysprops(), "auto-configure-node", autoConfigLibs);
assert cmd instanceof EnvironmentAwareCommand;
@SuppressWarnings("raw")
var autoConfigNode = (EnvironmentAwareCommand) cmd;
Expand Down Expand Up @@ -210,7 +211,7 @@ Environment autoConfigureSecurity(
// package private for testing
void syncPlugins(Terminal terminal, Environment env, ProcessInfo processInfo) throws Exception {
String pluginCliLibs = "lib/tools/plugin-cli";
Command cmd = loadTool("sync-plugins", pluginCliLibs);
Command cmd = loadTool(processInfo.sysprops(), "sync-plugins", pluginCliLibs);
assert cmd instanceof EnvironmentAwareCommand;
@SuppressWarnings("raw")
var syncPlugins = (EnvironmentAwareCommand) cmd;
Expand Down Expand Up @@ -258,8 +259,8 @@ protected ServerProcess getServer() {
}

// protected to allow tests to override
protected Command loadTool(String toolname, String libs) {
return CliToolProvider.load(toolname, libs).create();
protected Command loadTool(Map<String, String> sysprops, String toolname, String libs) {
return CliToolProvider.load(sysprops, toolname, libs).create();
}

// protected to allow tests to override
Expand All @@ -270,7 +271,8 @@ protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo,
.withProcessInfo(processInfo)
.withServerArgs(args)
.withTempDir(tempDir)
.withJvmOptions(jvmOptions);
.withJvmOptions(jvmOptions)
.withWorkingDir(args.logsDir());
return serverProcessBuilder.start();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
import org.elasticsearch.core.PathUtils;
import org.elasticsearch.core.SuppressForbidden;

import java.io.IOException;
import java.io.OutputStream;
Expand Down Expand Up @@ -43,6 +44,7 @@ public class ServerProcessBuilder {
private ServerArgs serverArgs;
private ProcessInfo processInfo;
private List<String> jvmOptions;
private Path workingDir;
private Terminal terminal;

// this allows mocking the process building by tests
Expand Down Expand Up @@ -82,6 +84,11 @@ public ServerProcessBuilder withJvmOptions(List<String> jvmOptions) {
return this;
}

public ServerProcessBuilder withWorkingDir(Path workingDir) {
this.workingDir = workingDir;
return this;
}

/**
* Specifies the {@link Terminal} to use for reading input and writing output from/to the cli console
*/
Expand Down Expand Up @@ -155,7 +162,7 @@ ServerProcess start(ProcessStarter processStarter) throws UserException {

boolean success = false;
try {
jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), processStarter);
jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), workingDir, processStarter);
errorPump = new ErrorPumpThread(terminal, jvmProcess.getErrorStream());
errorPump.start();
sendArgs(serverArgs, jvmProcess.getOutputStream());
Expand Down Expand Up @@ -185,16 +192,23 @@ private static Process createProcess(
List<String> jvmArgs,
List<String> jvmOptions,
Map<String, String> environment,
Path workingDir,
ProcessStarter processStarter
) throws InterruptedException, IOException {

var builder = new ProcessBuilder(Stream.concat(Stream.of(command), Stream.concat(jvmOptions.stream(), jvmArgs.stream())).toList());
builder.environment().putAll(environment);
setWorkingDir(builder, workingDir);
builder.redirectOutput(ProcessBuilder.Redirect.INHERIT);

return processStarter.start(builder);
}

@SuppressForbidden(reason = "ProcessBuilder takes File")
private static void setWorkingDir(ProcessBuilder builder, Path path) {
builder.directory(path.toFile());
}

private static void sendArgs(ServerArgs args, OutputStream processStdin) {
// DO NOT close the underlying process stdin, since we need to be able to write to it to signal exit
var out = new OutputStreamStreamOutput(processStdin);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
final class SystemJvmOptions {

static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, String> sysprops) {
Path esHome = Path.of(sysprops.get("es.path.home"));
String distroType = sysprops.get("es.distribution.type");
String javaType = sysprops.get("es.java.type");
boolean isHotspot = sysprops.getOrDefault("sun.management.compiler", "").contains("HotSpot");
Expand Down Expand Up @@ -67,7 +68,8 @@ static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, St
"-Djava.locale.providers=CLDR",
// Enable vectorization for whatever version we are running. This ensures we use vectorization even when running EA builds.
"-Dorg.apache.lucene.vectorization.upperJavaFeatureVersion=" + Runtime.version().feature(),
// Pass through distribution type and java type
// Pass through some properties
"-Des.path.home=" + esHome,
"-Des.distribution.type=" + distroType,
"-Des.java.type=" + javaType
),
Expand All @@ -77,7 +79,7 @@ static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, St
maybeSetReplayFile(distroType, isHotspot),
maybeWorkaroundG1Bug(),
maybeAllowSecurityManager(useEntitlements),
maybeAttachEntitlementAgent(useEntitlements)
maybeAttachEntitlementAgent(esHome, useEntitlements)
).flatMap(s -> s).toList();
}

Expand Down Expand Up @@ -159,12 +161,12 @@ private static Stream<String> maybeAllowSecurityManager(boolean useEntitlements)
return Stream.of();
}

private static Stream<String> maybeAttachEntitlementAgent(boolean useEntitlements) {
private static Stream<String> maybeAttachEntitlementAgent(Path esHome, boolean useEntitlements) {
if (useEntitlements == false) {
return Stream.empty();
}

Path dir = Path.of("lib", "entitlement-bridge");
Path dir = esHome.resolve("lib/entitlement-bridge");
if (Files.exists(dir) == false) {
throw new IllegalStateException("Directory for entitlement bridge jar does not exist: " + dir);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.nio.file.Path;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.BrokenBarrierException;
import java.util.concurrent.CyclicBarrier;
Expand Down Expand Up @@ -558,7 +559,7 @@ private class TestServerCli extends ServerCli {
boolean startServerCalled = false;

@Override
protected Command loadTool(String toolname, String libs) {
protected Command loadTool(Map<String, String> sysprops, String toolname, String libs) {
if (toolname.equals("auto-configure-node")) {
assertThat(libs, equalTo("modules/x-pack-core,modules/x-pack-security,lib/tools/security-cli"));
return AUTO_CONFIG_CLI;
Expand Down
5 changes: 5 additions & 0 deletions docs/changelog/124966.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 124966
summary: Use logs dir as working directory
area: Infra/CLI
type: enhancement
issues: []
4 changes: 2 additions & 2 deletions docs/reference/elasticsearch/jvm-settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ If you are running {{es}} as a Windows service, you can change the heap size usi

## JVM heap dump path setting [heap-dump-path-setting]

By default, {{es}} configures the JVM to dump the heap on out of memory exceptions to the default data directory. On [RPM](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-with-rpm.md) and [Debian](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-with-debian-package.md) packages, the data directory is `/var/lib/elasticsearch`. On [Linux and MacOS](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-from-archive-on-linux-macos.md) and [Windows](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-with-zip-on-windows.md) distributions, the `data` directory is located under the root of the {{es}} installation.
By default, {{es}} configures the JVM to dump the heap on out of memory exceptions to the default logs directory. On [RPM](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-with-rpm.md) and [Debian](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-with-debian-package.md) packages, the logs directory is `/var/log/elasticsearch`. On [Linux and MacOS](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-from-archive-on-linux-macos.md) and [Windows](docs-content://deploy-manage/deploy/self-managed/install-elasticsearch-with-zip-on-windows.md) distributions, the `logs` directory is located under the root of the {{es}} installation.

If this path is not suitable for receiving heap dumps, modify the `-XX:HeapDumpPath=...` entry in [`jvm.options`](#set-jvm-options):
If this path is not suitable for receiving heap dumps, add the `-XX:HeapDumpPath=...` entry in [`jvm.options`](#set-jvm-options):

* If you specify a directory, the JVM will generate a filename for the heap dump based on the PID of the running instance.
* If you specify a fixed filename instead of a directory, the file must not exist when the JVM needs to perform a heap dump on an out of memory exception. Otherwise, the heap dump will fail.
Expand Down
Loading
Loading