Skip to content

Commit 3bac50e

Browse files
authored
Use logs dir as working directory (#124966)
In the unexpected case that Elasticsearch dies due to a segfault or other similar native issue, a core dump is useful in diagnosing the problem. Yet core dumps are written to the working directory, which is read-only for most installations of Elasticsearch. This commit changes the working directory to the logs dir which should always be writeable.
1 parent d82886f commit 3bac50e

File tree

20 files changed

+177
-89
lines changed

20 files changed

+177
-89
lines changed

Diff for: build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy

+3-3
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,9 @@ echo "Running elasticsearch \$0"
179179

180180
file(distProjectFolder, 'src/config/elasticsearch.properties') << "some propes"
181181
file(distProjectFolder, 'src/config/jvm.options') << """
182-
-Xlog:gc*,gc+age=trace,safepoint:file=logs/gc.log:utctime,level,pid,tags:filecount=32,filesize=64m
183-
-XX:ErrorFile=logs/hs_err_pid%p.log
184-
-XX:HeapDumpPath=data
182+
-Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m
183+
-XX:ErrorFile=hs_err_pid%p.log
184+
# -XX:HeapDumpPath=/heap/dump/path
185185
"""
186186
file(distProjectFolder, 'build.gradle') << """
187187
import org.gradle.api.internal.artifacts.ArtifactAttributes;

Diff for: build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

+40-13
Original file line numberDiff line numberDiff line change
@@ -1435,30 +1435,57 @@ private void tweakJvmOptions(Path configFileRoot) {
14351435
Path jvmOptions = configFileRoot.resolve("jvm.options");
14361436
try {
14371437
String content = new String(Files.readAllBytes(jvmOptions));
1438-
Map<String, String> expansions = jvmOptionExpansions();
1439-
for (String origin : expansions.keySet()) {
1440-
if (content.contains(origin) == false) {
1441-
throw new IOException("template property " + origin + " not found in template.");
1438+
Map<ReplacementKey, String> expansions = jvmOptionExpansions();
1439+
for (var entry : expansions.entrySet()) {
1440+
ReplacementKey replacement = entry.getKey();
1441+
String key = replacement.key();
1442+
if (content.contains(key) == false) {
1443+
key = replacement.fallback();
1444+
if (content.contains(key) == false) {
1445+
throw new IOException("Template property '" + replacement + "' not found in template:\n" + content);
1446+
}
14421447
}
1443-
content = content.replace(origin, expansions.get(origin));
1448+
content = content.replace(key, entry.getValue());
14441449
}
14451450
Files.write(jvmOptions, content.getBytes());
14461451
} catch (IOException ioException) {
14471452
throw new UncheckedIOException(ioException);
14481453
}
14491454
}
14501455

1451-
private Map<String, String> jvmOptionExpansions() {
1452-
Map<String, String> expansions = new HashMap<>();
1456+
private record ReplacementKey(String key, String fallback) {}
1457+
1458+
private Map<ReplacementKey, String> jvmOptionExpansions() {
1459+
Map<ReplacementKey, String> expansions = new HashMap<>();
14531460
Version version = getVersion();
1454-
String heapDumpOrigin = getVersion().onOrAfter("6.3.0") ? "-XX:HeapDumpPath=data" : "-XX:HeapDumpPath=/heap/dump/path";
1455-
expansions.put(heapDumpOrigin, "-XX:HeapDumpPath=" + confPathLogs);
1456-
if (version.onOrAfter("6.2.0")) {
1457-
expansions.put("logs/gc.log", confPathLogs.resolve("gc.log").toString());
1461+
1462+
ReplacementKey heapDumpPathSub;
1463+
if (version.before("8.19.0") && version.onOrAfter("6.3.0")) {
1464+
heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", null);
1465+
} else {
1466+
// temporarily fall back to the old substitution so both old and new work during backport
1467+
heapDumpPathSub = new ReplacementKey("# -XX:HeapDumpPath=/heap/dump/path", "-XX:HeapDumpPath=data");
14581468
}
1459-
if (getVersion().getMajor() >= 7) {
1460-
expansions.put("-XX:ErrorFile=logs/hs_err_pid%p.log", "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log"));
1469+
expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs);
1470+
1471+
ReplacementKey gcLogSub;
1472+
if (version.before("8.19.0") && version.onOrAfter("6.2.0")) {
1473+
gcLogSub = new ReplacementKey("logs/gc.log", null);
1474+
} else {
1475+
// temporarily check the old substitution first so both old and new work during backport
1476+
gcLogSub = new ReplacementKey("logs/gc.log", "gc.log");
14611477
}
1478+
expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString());
1479+
1480+
ReplacementKey errorFileSub;
1481+
if (version.before("8.19.0") && version.getMajor() >= 7) {
1482+
errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", null);
1483+
} else {
1484+
// temporarily check the old substitution first so both old and new work during backport
1485+
errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", "-XX:ErrorFile=hs_err_pid%p.log");
1486+
}
1487+
expansions.put(errorFileSub, "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log"));
1488+
14621489
return expansions;
14631490
}
14641491

Diff for: distribution/build.gradle

-18
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,6 @@ subprojects {
531531
final String packagingPathData = "path.data: /var/lib/elasticsearch"
532532
final String pathLogs = "/var/log/elasticsearch"
533533
final String packagingPathLogs = "path.logs: ${pathLogs}"
534-
final String packagingLoggc = "${pathLogs}/gc.log"
535534

536535
String licenseText
537536
if (isTestDistro) {
@@ -576,23 +575,6 @@ subprojects {
576575
'rpm': packagingPathLogs,
577576
'def': '#path.logs: /path/to/logs'
578577
],
579-
'loggc': [
580-
'deb': packagingLoggc,
581-
'rpm': packagingLoggc,
582-
'def': 'logs/gc.log'
583-
],
584-
585-
'heap.dump.path': [
586-
'deb': "-XX:HeapDumpPath=/var/lib/elasticsearch",
587-
'rpm': "-XX:HeapDumpPath=/var/lib/elasticsearch",
588-
'def': "-XX:HeapDumpPath=data"
589-
],
590-
591-
'error.file': [
592-
'deb': "-XX:ErrorFile=/var/log/elasticsearch/hs_err_pid%p.log",
593-
'rpm': "-XX:ErrorFile=/var/log/elasticsearch/hs_err_pid%p.log",
594-
'def': "-XX:ErrorFile=logs/hs_err_pid%p.log"
595-
],
596578

597579
'scripts.footer': [
598580
/* Debian needs exit 0 on these scripts so we add it here and preserve

Diff for: distribution/src/config/jvm.options

+3-3
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@
7777

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

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

8585
## GC logging
86-
-Xlog:gc*,gc+age=trace,safepoint:file=@loggc@:utctime,level,pid,tags:filecount=32,filesize=64m
86+
-Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m

Diff for: distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public static void main(String[] args) throws Exception {
5858
String toolname = getToolName(pinfo.sysprops());
5959
String libs = pinfo.sysprops().getOrDefault("cli.libs", "");
6060

61-
command = CliToolProvider.load(toolname, libs).create();
61+
command = CliToolProvider.load(pinfo.sysprops(), toolname, libs).create();
6262
Terminal terminal = Terminal.DEFAULT;
6363
Runtime.getRuntime().addShutdownHook(createShutdownHook(terminal, command));
6464

Diff for: distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java

+13-1
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
package org.elasticsearch.server.cli;
1111

1212
import org.elasticsearch.common.Strings;
13+
import org.elasticsearch.core.SuppressForbidden;
1314

1415
import java.io.BufferedReader;
1516
import java.io.IOException;
1617
import java.io.InputStream;
1718
import java.io.InputStreamReader;
1819
import java.nio.charset.StandardCharsets;
20+
import java.nio.file.Files;
1921
import java.nio.file.Path;
2022
import java.util.List;
2123
import java.util.Locale;
@@ -106,7 +108,9 @@ private static List<String> flagsFinal(final List<String> userDefinedJvmOptions)
106108
userDefinedJvmOptions.stream(),
107109
Stream.of("-XX:+PrintFlagsFinal", "-version")
108110
).flatMap(Function.identity()).toList();
109-
final Process process = new ProcessBuilder().command(command).start();
111+
final ProcessBuilder builder = new ProcessBuilder().command(command);
112+
setWorkingDir(builder);
113+
final Process process = builder.start();
110114
final List<String> output = readLinesFromInputStream(process.getInputStream());
111115
final List<String> error = readLinesFromInputStream(process.getErrorStream());
112116
final int status = process.waitFor();
@@ -124,6 +128,14 @@ private static List<String> flagsFinal(final List<String> userDefinedJvmOptions)
124128
}
125129
}
126130

131+
@SuppressForbidden(reason = "ProcessBuilder takes File")
132+
private static void setWorkingDir(ProcessBuilder builder) throws IOException {
133+
// The real ES process uses the logs dir as the working directory. Since we don't
134+
// have the logs dir yet, here we use a temp directory for calculating jvm options.
135+
final Path tmpDir = Files.createTempDirectory("final-flags");
136+
builder.directory(tmpDir.toFile());
137+
}
138+
127139
private static List<String> readLinesFromInputStream(final InputStream is) throws IOException {
128140
try (InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(isr)) {
129141
return br.lines().toList();

Diff for: distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -269,13 +269,13 @@ interface InvalidLineConsumer {
269269
* and the following JVM options will not be accepted:
270270
* <ul>
271271
* <li>
272-
* {@code 18:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
272+
* {@code 18:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m}
273273
* </li>
274274
* <li>
275-
* {@code 18-:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
275+
* {@code 18-:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m}
276276
* </li>
277277
* <li>
278-
* {@code 18-19:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
278+
* {@code 18-19:-Xlog:age*=trace,gc*,safepoint:file=gc.log:utctime,pid,tags:filecount=32,filesize=64m}
279279
* </li>
280280
* </ul>
281281
*

Diff for: distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java

+7-5
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.nio.file.Path;
3434
import java.util.Arrays;
3535
import java.util.Locale;
36+
import java.util.Map;
3637
import java.util.concurrent.atomic.AtomicBoolean;
3738

3839
/**
@@ -168,7 +169,7 @@ Environment autoConfigureSecurity(
168169
assert secureSettingsLoader(env) instanceof KeyStoreLoader;
169170

170171
String autoConfigLibs = "modules/x-pack-core,modules/x-pack-security,lib/tools/security-cli";
171-
Command cmd = loadTool("auto-configure-node", autoConfigLibs);
172+
Command cmd = loadTool(processInfo.sysprops(), "auto-configure-node", autoConfigLibs);
172173
assert cmd instanceof EnvironmentAwareCommand;
173174
@SuppressWarnings("raw")
174175
var autoConfigNode = (EnvironmentAwareCommand) cmd;
@@ -210,7 +211,7 @@ Environment autoConfigureSecurity(
210211
// package private for testing
211212
void syncPlugins(Terminal terminal, Environment env, ProcessInfo processInfo) throws Exception {
212213
String pluginCliLibs = "lib/tools/plugin-cli";
213-
Command cmd = loadTool("sync-plugins", pluginCliLibs);
214+
Command cmd = loadTool(processInfo.sysprops(), "sync-plugins", pluginCliLibs);
214215
assert cmd instanceof EnvironmentAwareCommand;
215216
@SuppressWarnings("raw")
216217
var syncPlugins = (EnvironmentAwareCommand) cmd;
@@ -258,8 +259,8 @@ protected ServerProcess getServer() {
258259
}
259260

260261
// protected to allow tests to override
261-
protected Command loadTool(String toolname, String libs) {
262-
return CliToolProvider.load(toolname, libs).create();
262+
protected Command loadTool(Map<String, String> sysprops, String toolname, String libs) {
263+
return CliToolProvider.load(sysprops, toolname, libs).create();
263264
}
264265

265266
// protected to allow tests to override
@@ -270,7 +271,8 @@ protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo,
270271
.withProcessInfo(processInfo)
271272
.withServerArgs(args)
272273
.withTempDir(tempDir)
273-
.withJvmOptions(jvmOptions);
274+
.withJvmOptions(jvmOptions)
275+
.withWorkingDir(args.logsDir());
274276
return serverProcessBuilder.start();
275277
}
276278

Diff for: distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java

+15-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.elasticsearch.common.Strings;
1717
import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
1818
import org.elasticsearch.core.PathUtils;
19+
import org.elasticsearch.core.SuppressForbidden;
1920

2021
import java.io.IOException;
2122
import java.io.OutputStream;
@@ -43,6 +44,7 @@ public class ServerProcessBuilder {
4344
private ServerArgs serverArgs;
4445
private ProcessInfo processInfo;
4546
private List<String> jvmOptions;
47+
private Path workingDir;
4648
private Terminal terminal;
4749

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

87+
public ServerProcessBuilder withWorkingDir(Path workingDir) {
88+
this.workingDir = workingDir;
89+
return this;
90+
}
91+
8592
/**
8693
* Specifies the {@link Terminal} to use for reading input and writing output from/to the cli console
8794
*/
@@ -155,7 +162,7 @@ ServerProcess start(ProcessStarter processStarter) throws UserException {
155162

156163
boolean success = false;
157164
try {
158-
jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), processStarter);
165+
jvmProcess = createProcess(getCommand(), getJvmArgs(), jvmOptions, getEnvironment(), workingDir, processStarter);
159166
errorPump = new ErrorPumpThread(terminal, jvmProcess.getErrorStream());
160167
errorPump.start();
161168
sendArgs(serverArgs, jvmProcess.getOutputStream());
@@ -185,16 +192,23 @@ private static Process createProcess(
185192
List<String> jvmArgs,
186193
List<String> jvmOptions,
187194
Map<String, String> environment,
195+
Path workingDir,
188196
ProcessStarter processStarter
189197
) throws InterruptedException, IOException {
190198

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

195204
return processStarter.start(builder);
196205
}
197206

207+
@SuppressForbidden(reason = "ProcessBuilder takes File")
208+
private static void setWorkingDir(ProcessBuilder builder, Path path) {
209+
builder.directory(path.toFile());
210+
}
211+
198212
private static void sendArgs(ServerArgs args, OutputStream processStdin) {
199213
// DO NOT close the underlying process stdin, since we need to be able to write to it to signal exit
200214
var out = new OutputStreamStreamOutput(processStdin);

Diff for: distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
final class SystemJvmOptions {
2525

2626
static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, String> sysprops) {
27+
Path esHome = Path.of(sysprops.get("es.path.home"));
2728
String distroType = sysprops.get("es.distribution.type");
2829
String javaType = sysprops.get("es.java.type");
2930
boolean isHotspot = sysprops.getOrDefault("sun.management.compiler", "").contains("HotSpot");
@@ -67,7 +68,8 @@ static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, St
6768
"-Djava.locale.providers=CLDR",
6869
// Enable vectorization for whatever version we are running. This ensures we use vectorization even when running EA builds.
6970
"-Dorg.apache.lucene.vectorization.upperJavaFeatureVersion=" + Runtime.version().feature(),
70-
// Pass through distribution type and java type
71+
// Pass through some properties
72+
"-Des.path.home=" + esHome,
7173
"-Des.distribution.type=" + distroType,
7274
"-Des.java.type=" + javaType
7375
),
@@ -77,7 +79,7 @@ static List<String> systemJvmOptions(Settings nodeSettings, final Map<String, St
7779
maybeSetReplayFile(distroType, isHotspot),
7880
maybeWorkaroundG1Bug(),
7981
maybeAllowSecurityManager(useEntitlements),
80-
maybeAttachEntitlementAgent(useEntitlements)
82+
maybeAttachEntitlementAgent(esHome, useEntitlements)
8183
).flatMap(s -> s).toList();
8284
}
8385

@@ -159,12 +161,12 @@ private static Stream<String> maybeAllowSecurityManager(boolean useEntitlements)
159161
return Stream.of();
160162
}
161163

162-
private static Stream<String> maybeAttachEntitlementAgent(boolean useEntitlements) {
164+
private static Stream<String> maybeAttachEntitlementAgent(Path esHome, boolean useEntitlements) {
163165
if (useEntitlements == false) {
164166
return Stream.empty();
165167
}
166168

167-
Path dir = Path.of("lib", "entitlement-bridge");
169+
Path dir = esHome.resolve("lib/entitlement-bridge");
168170
if (Files.exists(dir) == false) {
169171
throw new IllegalStateException("Directory for entitlement bridge jar does not exist: " + dir);
170172
}

0 commit comments

Comments
 (0)