From 57bd9491d7116a64e5806e48a19218f4f50c24fb Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sun, 16 Mar 2025 09:41:57 -0700 Subject: [PATCH 01/19] Use logs dir as working directory 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. --- .../java/org/elasticsearch/server/cli/ServerCli.java | 3 ++- .../elasticsearch/server/cli/ServerProcessBuilder.java | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java index be454350133eb..17e09dccd6850 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java @@ -270,7 +270,8 @@ protected ServerProcess startServer(Terminal terminal, ProcessInfo processInfo, .withProcessInfo(processInfo) .withServerArgs(args) .withTempDir(tempDir) - .withJvmOptions(jvmOptions); + .withJvmOptions(jvmOptions) + .withWorkingDir(args.logsDir()); return serverProcessBuilder.start(); } diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java index adebf6be9842b..243285790c0ce 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java @@ -43,6 +43,7 @@ public class ServerProcessBuilder { private ServerArgs serverArgs; private ProcessInfo processInfo; private List jvmOptions; + private Path workingDir; private Terminal terminal; // this allows mocking the process building by tests @@ -82,6 +83,11 @@ public ServerProcessBuilder withJvmOptions(List 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 */ @@ -155,7 +161,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()); @@ -185,11 +191,13 @@ private static Process createProcess( List jvmArgs, List jvmOptions, Map 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); + builder.directory(workingDir.toFile()); builder.redirectOutput(ProcessBuilder.Redirect.INHERIT); return processStarter.start(builder); From 6bb542b387d4c6a8b39d44344b64a9a9dd98da03 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sun, 16 Mar 2025 09:45:21 -0700 Subject: [PATCH 02/19] Update docs/changelog/124966.yaml --- docs/changelog/124966.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/124966.yaml diff --git a/docs/changelog/124966.yaml b/docs/changelog/124966.yaml new file mode 100644 index 0000000000000..7e962a795a485 --- /dev/null +++ b/docs/changelog/124966.yaml @@ -0,0 +1,5 @@ +pr: 124966 +summary: Use logs dir as working directory +area: Infra/CLI +type: enhancement +issues: [] From ee4956fc4c2a5d3122a12952b5c1c1078db7a232 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sun, 16 Mar 2025 10:28:42 -0700 Subject: [PATCH 03/19] Use homedir to find platform dir --- .../java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java b/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java index d9c725f5a8d3b..98887381e5d40 100644 --- a/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java +++ b/libs/native/src/main/java/org/elasticsearch/nativeaccess/lib/LoaderHelper.java @@ -26,7 +26,8 @@ private static Path findPlatformLibDir() { return Paths.get(path); } - Path platformDir = Paths.get("lib", "platform"); + Path homeDir = Paths.get(System.getProperty("es.path.home")); + Path platformDir = homeDir.resolve("lib/platform"); String osname = System.getProperty("os.name"); String os; From 3f6560394f61b0640bf564f2daacf20c109d1d53 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 17 Mar 2025 09:23:09 -0700 Subject: [PATCH 04/19] iter --- .../LegacyYamlRestTestPluginFuncTest.groovy | 5 ++--- .../testclusters/ElasticsearchNode.java | 19 +++++++++++++------ distribution/build.gradle | 18 ------------------ distribution/src/config/jvm.options | 6 +++--- .../server/cli/JvmOptionsParser.java | 8 ++++---- .../server/cli/SystemJvmOptions.java | 15 +++++++++------ docs/reference/elasticsearch/jvm-settings.md | 4 ++-- .../bootstrap/EntitlementBootstrap.java | 3 ++- .../local/AbstractLocalClusterFactory.java | 8 ++++---- .../TimestampFormatFinderTests.java | 2 +- 10 files changed, 40 insertions(+), 48 deletions(-) diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy index ce5c1519fe11f..aa2dba5caff61 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy @@ -179,9 +179,8 @@ 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 """ file(distProjectFolder, 'build.gradle') << """ import org.gradle.api.internal.artifacts.ArtifactAttributes; diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index f6170693ce8cc..5d5b0c966a2a0 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1451,14 +1451,21 @@ private void tweakJvmOptions(Path configFileRoot) { private Map jvmOptionExpansions() { Map 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()); + String heapDumpPathSub = "# -XX:HeapDumpPath=/heap/dump/path"; + if (version.before("8.18.0") && version.onOrAfter("6.3.0")) { + heapDumpPathSub = "-XX:HeapDumpPath=/heap/dump/path"; } - 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); + String gcLogSub = "gc.log"; + if (version.before("8.18.0") && version.onOrAfter("6.2.0")) { + gcLogSub = "logs/gc.log"; + } + expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString()); + String errorFileSub = "-XX:ErrorFile=hs_err_pid%p.log"; + if (version.before("8.18.0") && getVersion().getMajor() >= 7) { + errorFileSub = "-XX:ErrorFile=logs/hs_err_pid%p.log"; } + expansions.put(errorFileSub, "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log")); return expansions; } diff --git a/distribution/build.gradle b/distribution/build.gradle index 1cdc1acf54f79..821b341977b72 100644 --- a/distribution/build.gradle +++ b/distribution/build.gradle @@ -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) { @@ -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 diff --git a/distribution/src/config/jvm.options b/distribution/src/config/jvm.options index 94fc6f2cb9025..081ee38303d8c 100644 --- a/distribution/src/config/jvm.options +++ b/distribution/src/config/jvm.options @@ -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 diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java index d2ac1549efd1a..dff4979561f3b 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java @@ -145,7 +145,7 @@ private List jvmOptions( final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo(substitutedJvmOptions, new DefaultSystemMemoryInfo()); substitutedJvmOptions.addAll(machineDependentHeap.determineHeapSettings(args.nodeSettings(), memoryInfo, substitutedJvmOptions)); final List ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions, args.nodeSettings()); - final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(args.nodeSettings(), cliSysprops); + final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(args, cliSysprops); final List apmOptions = APMJvmOptions.apmJvmOptions(args.nodeSettings(), args.secrets(), args.logsDir(), tmpDir); @@ -269,13 +269,13 @@ interface InvalidLineConsumer { * and the following JVM options will not be accepted: *
    *
  • - * {@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} *
    • *
  • - * {@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} *
    • *
  • - * {@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} *
    • *
* diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java index 456e9e6303078..b2b4028e9b979 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java @@ -9,6 +9,7 @@ package org.elasticsearch.server.cli; +import org.elasticsearch.bootstrap.ServerArgs; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.core.Booleans; @@ -24,7 +25,8 @@ final class SystemJvmOptions { - static List systemJvmOptions(Settings nodeSettings, final Map sysprops) { + static List systemJvmOptions(ServerArgs args, final Map 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"); @@ -69,17 +71,18 @@ static List systemJvmOptions(Settings nodeSettings, final Map getEnvironmentVariables() { private Map getJvmOptionsReplacements() { return Map.of( - "-XX:HeapDumpPath=data", - "-XX:HeapDumpPath=" + logsDir, - "logs/gc.log", + "# -XX:HeapDumpPath=/heap/dump/path", + logsDir.toString(), + "gc.log", logsDir.resolve("gc.log").toString(), - "-XX:ErrorFile=logs/hs_err_pid%p.log", + "-XX:ErrorFile=hs_err_pid%p.log", "-XX:ErrorFile=" + logsDir.resolve("hs_err_pid%p.log") ); } diff --git a/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java b/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java index 87bb63f442acf..048a4c2e6e512 100644 --- a/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java +++ b/x-pack/plugin/text-structure/src/test/java/org/elasticsearch/xpack/textstructure/structurefinder/TimestampFormatFinderTests.java @@ -1590,7 +1590,7 @@ public void testSelectBestMatchGivenAllSame() { [2018-06-27T11:59:22,202][INFO ][o.e.e.NodeEnvironment ] [node-0] heap size [494.9mb], compressed ordinary object pointers [true] [2018-06-27T11:59:22,204][INFO ][o.e.n.Node ] [node-0] node name [node-0], node ID [Ha1gD8nNSDqjd6PIyu3DJA] [2018-06-27T11:59:22,204][INFO ][o.e.n.Node ] [node-0] version[6.4.0-SNAPSHOT], pid[2785], build[default/zip/3c60efa/2018-06-26T14:55:15.206676Z], OS[Mac OS X/10.12.6/x86_64], JVM["Oracle Corporation"/Java HotSpot(TM) 64-Bit Server VM/10/10+46] - [2018-06-27T11:59:22,205][INFO ][o.e.n.Node ] [node-0] JVM arguments [-Xms1g, -Xmx1g, -XX:+UseConcMarkSweepGC, -XX:CMSInitiatingOccupancyFraction=75, -XX:+UseCMSInitiatingOccupancyOnly, -XX:+AlwaysPreTouch, -Xss1m, -Djava.awt.headless=true, -Dfile.encoding=UTF-8, -Djna.nosys=true, -XX:-OmitStackTraceInFastThrow, -Dio.netty.noUnsafe=true, -Dio.netty.noKeySetOptimization=true, -Dio.netty.recycler.maxCapacityPerThread=0, -Dlog4j.shutdownHookEnabled=false, -Dlog4j2.disable.jmx=true, -Djava.io.tmpdir=/var/folders/k5/5sqcdlps5sg3cvlp783gcz740000h0/T/elasticsearch.nFUyeMH1, -XX:+HeapDumpOnOutOfMemoryError, -XX:HeapDumpPath=data, -XX:ErrorFile=logs/hs_err_pid%p.log, -Xlog:gc*,gc+age=trace,safepoint:file=logs/gc.log:utctime,level,pid,tags:filecount=32,filesize=64m, -Djava.locale.providers=COMPAT, -Dio.netty.allocator.type=unpooled, -ea, -esa, -Xms512m, -Xmx512m, -Des.path.home=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT, -Des.path.conf=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT/config, -Des.distribution.flavor=default, -Des.distribution.type=zip] + [2018-06-27T11:59:22,205][INFO ][o.e.n.Node ] [node-0] JVM arguments [-Xms1g, -Xmx1g, -XX:+UseConcMarkSweepGC, -XX:CMSInitiatingOccupancyFraction=75, -XX:+UseCMSInitiatingOccupancyOnly, -XX:+AlwaysPreTouch, -Xss1m, -Djava.awt.headless=true, -Dfile.encoding=UTF-8, -Djna.nosys=true, -XX:-OmitStackTraceInFastThrow, -Dio.netty.noUnsafe=true, -Dio.netty.noKeySetOptimization=true, -Dio.netty.recycler.maxCapacityPerThread=0, -Dlog4j.shutdownHookEnabled=false, -Dlog4j2.disable.jmx=true, -Djava.io.tmpdir=/var/folders/k5/5sqcdlps5sg3cvlp783gcz740000h0/T/elasticsearch.nFUyeMH1, -XX:+HeapDumpOnOutOfMemoryError, -XX:ErrorFile=hs_err_pid%p.log, -Xlog:gc*,gc+age=trace,safepoint:file=gc.log:utctime,level,pid,tags:filecount=32,filesize=64m, -Djava.locale.providers=COMPAT, -Dio.netty.allocator.type=unpooled, -ea, -esa, -Xms512m, -Xmx512m, -Des.path.home=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT, -Des.path.conf=/Users/dave/elasticsearch/distribution/build/cluster/run node0/elasticsearch-6.4.0-SNAPSHOT/config, -Des.distribution.flavor=default, -Des.distribution.type=zip] [2018-06-27T11:59:22,205][WARN ][o.e.n.Node ] [node-0] version [6.4.0-SNAPSHOT] is a pre-release version of Elasticsearch and is not suitable for production [2018-06-27T11:59:23,585][INFO ][o.e.p.PluginsService ] [node-0] loaded module [aggs-matrix-stats] [2018-06-27T11:59:23,586][INFO ][o.e.p.PluginsService ] [node-0] loaded module [analysis-common] From 0d3cdeaf284d37c12702ea3adcd9a5e4f2836677 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 17 Mar 2025 10:13:31 -0700 Subject: [PATCH 05/19] oops --- .../test/cluster/local/AbstractLocalClusterFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index 402e19887b7d7..2781d8850fed6 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -910,7 +910,7 @@ private Map getEnvironmentVariables() { private Map getJvmOptionsReplacements() { return Map.of( "# -XX:HeapDumpPath=/heap/dump/path", - logsDir.toString(), + "-XX:HeapDumpPath=" + logsDir.toString(), "gc.log", logsDir.resolve("gc.log").toString(), "-XX:ErrorFile=hs_err_pid%p.log", From 5e08b5a377b6851922fe286da3693232ad5af653 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 17 Mar 2025 13:21:52 -0700 Subject: [PATCH 06/19] try to fix bwc --- .../testclusters/ElasticsearchNode.java | 11 +++++- .../launcher/CliToolLauncher.java | 2 +- .../elasticsearch/cli/CliToolProvider.java | 7 ++-- .../local/AbstractLocalClusterFactory.java | 37 ++++++++++++++----- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index 5d5b0c966a2a0..c892fa3caea50 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1438,7 +1438,10 @@ private void tweakJvmOptions(Path configFileRoot) { Map expansions = jvmOptionExpansions(); for (String origin : expansions.keySet()) { if (content.contains(origin) == false) { - throw new IOException("template property " + origin + " not found in template."); + LOGGER.warn("template property '" + origin + "' not found in template."); + continue; + // temporarily just warn during backports for https://github.com/elastic/elasticsearch/pull/124966 + // throw new IOException("template property '" + origin + "' not found in template."); } content = content.replace(origin, expansions.get(origin)); } @@ -1452,16 +1455,22 @@ private Map jvmOptionExpansions() { Map expansions = new HashMap<>(); Version version = getVersion(); String heapDumpPathSub = "# -XX:HeapDumpPath=/heap/dump/path"; + // temporarily duplicate the expansion so both old and new exist during backport + expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs); if (version.before("8.18.0") && version.onOrAfter("6.3.0")) { heapDumpPathSub = "-XX:HeapDumpPath=/heap/dump/path"; } expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs); String gcLogSub = "gc.log"; + expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString()); + // temporarily duplicate the expansion so both old and new exist during backport if (version.before("8.18.0") && version.onOrAfter("6.2.0")) { gcLogSub = "logs/gc.log"; } expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString()); String errorFileSub = "-XX:ErrorFile=hs_err_pid%p.log"; + // temporarily duplicate the expansion so both old and new exist during backport + expansions.put(errorFileSub, "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log")); if (version.before("8.18.0") && getVersion().getMajor() >= 7) { errorFileSub = "-XX:ErrorFile=logs/hs_err_pid%p.log"; } diff --git a/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java b/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java index a3ef768c30100..7ffc9276e2f40 100644 --- a/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java +++ b/distribution/tools/cli-launcher/src/main/java/org/elasticsearch/launcher/CliToolLauncher.java @@ -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)); diff --git a/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java b/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java index 99138f2321475..6d42d3f764df1 100644 --- a/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java +++ b/libs/cli/src/main/java/org/elasticsearch/cli/CliToolProvider.java @@ -19,6 +19,7 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.ServiceLoader; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -43,14 +44,14 @@ public interface CliToolProvider { /** * Loads a tool provider from the Elasticsearch distribution. * + * @param sysprops the system properties of the CLI process * @param toolname the name of the tool to load * @param libs the library directories to load, relative to the Elasticsearch homedir * @return the instance of the loaded tool * @throws AssertionError if the given toolname cannot be found or there are more than one tools found with the same name */ - static CliToolProvider load(String toolname, String libs) { - // the ES homedir is always our working dir - Path homeDir = Paths.get("").toAbsolutePath(); + static CliToolProvider load(Map sysprops, String toolname, String libs) { + Path homeDir = Paths.get(sysprops.get("es.path.home")).toAbsolutePath(); final ClassLoader cliLoader; if (libs.isBlank()) { cliLoader = ClassLoader.getSystemClassLoader(); diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index 2781d8850fed6..5a92c24b38f84 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -439,7 +439,10 @@ private void writeConfiguration() { Map expansions = getJvmOptionsReplacements(); for (String key : expansions.keySet()) { if (content.contains(key) == false) { - throw new IOException("Template property '" + key + "' not found in template."); + LOGGER.warn("Template property '" + key + "' not found in template."); + continue; + // temporarily just warn during backports for https://github.com/elastic/elasticsearch/pull/124966 + // throw new IOException("Template property '" + key + "' not found in template."); } content = content.replace(key, expansions.get(key)); } @@ -908,14 +911,30 @@ private Map getEnvironmentVariables() { } private Map getJvmOptionsReplacements() { - return Map.of( - "# -XX:HeapDumpPath=/heap/dump/path", - "-XX:HeapDumpPath=" + logsDir.toString(), - "gc.log", - logsDir.resolve("gc.log").toString(), - "-XX:ErrorFile=hs_err_pid%p.log", - "-XX:ErrorFile=" + logsDir.resolve("hs_err_pid%p.log") - ); + var expansions = new HashMap(); + var version = spec.getVersion(); + String heapDumpPathSub = "# -XX:HeapDumpPath=/heap/dump/path"; + // temporarily duplicate the expansion so both old and new exist during backport + expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + logsDir); + if (version.before("8.18.0") && version.onOrAfter("6.3.0")) { + heapDumpPathSub = "-XX:HeapDumpPath=/heap/dump/path"; + } + expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + logsDir); + String gcLogSub = "gc.log"; + // temporarily duplicate the expansion so both old and new exist during backport + expansions.put(gcLogSub, logsDir.resolve("gc.log").toString()); + if (version.before("8.18.0") && version.onOrAfter("6.2.0")) { + gcLogSub = "logs/gc.log"; + } + expansions.put(gcLogSub, logsDir.resolve("gc.log").toString()); + // temporarily duplicate the expansion so both old and new exist during backport + String errorFileSub = "-XX:ErrorFile=hs_err_pid%p.log"; + expansions.put(errorFileSub, "-XX:ErrorFile=" + logsDir.resolve("hs_err_pid%p.log")); + if (version.before("8.18.0") && version.getMajor() >= 7) { + errorFileSub = "-XX:ErrorFile=logs/hs_err_pid%p.log"; + } + expansions.put(errorFileSub, "-XX:ErrorFile=" + logsDir.resolve("hs_err_pid%p.log")); + return expansions; } private void runToolScript(String tool, String input, String... args) { From 6be493500d6b6c6b5ce5e1c5315e7dc2492c3c90 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 17 Mar 2025 13:31:41 -0700 Subject: [PATCH 07/19] update server cli --- .../java/org/elasticsearch/server/cli/ServerCli.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java index 17e09dccd6850..8eae170667a84 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java @@ -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; /** @@ -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; @@ -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; @@ -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 sysprops, String toolname, String libs) { + return CliToolProvider.load(sysprops, toolname, libs).create(); } // protected to allow tests to override From f4e0622ce09c40529f9d8b556c2bf3e666f8d749 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 17 Mar 2025 14:46:12 -0700 Subject: [PATCH 08/19] tests --- .../java/org/elasticsearch/server/cli/JvmOptionsParser.java | 2 +- .../java/org/elasticsearch/server/cli/SystemJvmOptions.java | 5 ++--- .../java/org/elasticsearch/server/cli/ServerCliTests.java | 3 ++- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java index dff4979561f3b..e97dc4c06f651 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOptionsParser.java @@ -145,7 +145,7 @@ private List jvmOptions( final SystemMemoryInfo memoryInfo = new OverridableSystemMemoryInfo(substitutedJvmOptions, new DefaultSystemMemoryInfo()); substitutedJvmOptions.addAll(machineDependentHeap.determineHeapSettings(args.nodeSettings(), memoryInfo, substitutedJvmOptions)); final List ergonomicJvmOptions = JvmErgonomics.choose(substitutedJvmOptions, args.nodeSettings()); - final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(args, cliSysprops); + final List systemJvmOptions = SystemJvmOptions.systemJvmOptions(args.nodeSettings(), cliSysprops); final List apmOptions = APMJvmOptions.apmJvmOptions(args.nodeSettings(), args.secrets(), args.logsDir(), tmpDir); diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java index b2b4028e9b979..289a1549ef24c 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/SystemJvmOptions.java @@ -9,7 +9,6 @@ package org.elasticsearch.server.cli; -import org.elasticsearch.bootstrap.ServerArgs; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.core.Booleans; @@ -25,7 +24,7 @@ final class SystemJvmOptions { - static List systemJvmOptions(ServerArgs args, final Map sysprops) { + static List systemJvmOptions(Settings nodeSettings, final Map sysprops) { Path esHome = Path.of(sysprops.get("es.path.home")); String distroType = sysprops.get("es.distribution.type"); String javaType = sysprops.get("es.java.type"); @@ -78,7 +77,7 @@ static List systemJvmOptions(ServerArgs args, final Map ), maybeEnableNativeAccess(useEntitlements), maybeOverrideDockerCgroup(distroType), - maybeSetActiveProcessorCount(args.nodeSettings()), + maybeSetActiveProcessorCount(nodeSettings), maybeSetReplayFile(distroType, isHotspot), maybeWorkaroundG1Bug(), maybeAllowSecurityManager(useEntitlements), diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java index bacc89548c9a1..576e3f3d05a18 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java @@ -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; @@ -558,7 +559,7 @@ private class TestServerCli extends ServerCli { boolean startServerCalled = false; @Override - protected Command loadTool(String toolname, String libs) { + protected Command loadTool(Map 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; From 9fae96ea81a8c37820002c16169cb564764a87e2 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 17 Mar 2025 17:52:17 -0700 Subject: [PATCH 09/19] better substitution --- .../testclusters/ElasticsearchNode.java | 57 +++++++++++-------- .../local/AbstractLocalClusterFactory.java | 52 ++++++++++------- 2 files changed, 65 insertions(+), 44 deletions(-) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index c892fa3caea50..fd508556b05e7 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1435,15 +1435,17 @@ private void tweakJvmOptions(Path configFileRoot) { Path jvmOptions = configFileRoot.resolve("jvm.options"); try { String content = new String(Files.readAllBytes(jvmOptions)); - Map expansions = jvmOptionExpansions(); - for (String origin : expansions.keySet()) { - if (content.contains(origin) == false) { - LOGGER.warn("template property '" + origin + "' not found in template."); - continue; - // temporarily just warn during backports for https://github.com/elastic/elasticsearch/pull/124966 - // throw new IOException("template property '" + origin + "' not found in template."); + Map 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."); + } } - content = content.replace(origin, expansions.get(origin)); + content = content.replace(key, entry.getValue()); } Files.write(jvmOptions, content.getBytes()); } catch (IOException ioException) { @@ -1451,30 +1453,39 @@ private void tweakJvmOptions(Path configFileRoot) { } } - private Map jvmOptionExpansions() { - Map expansions = new HashMap<>(); + private record ReplacementKey(String key, String fallback) {} + + private Map jvmOptionExpansions() { + Map expansions = new HashMap<>(); Version version = getVersion(); - String heapDumpPathSub = "# -XX:HeapDumpPath=/heap/dump/path"; - // temporarily duplicate the expansion so both old and new exist during backport - expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs); + + ReplacementKey heapDumpPathSub; if (version.before("8.18.0") && version.onOrAfter("6.3.0")) { - heapDumpPathSub = "-XX:HeapDumpPath=/heap/dump/path"; + 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"); } expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs); - String gcLogSub = "gc.log"; - expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString()); - // temporarily duplicate the expansion so both old and new exist during backport + + ReplacementKey gcLogSub; if (version.before("8.18.0") && version.onOrAfter("6.2.0")) { - gcLogSub = "logs/gc.log"; + 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()); - String errorFileSub = "-XX:ErrorFile=hs_err_pid%p.log"; - // temporarily duplicate the expansion so both old and new exist during backport - expansions.put(errorFileSub, "-XX:ErrorFile=" + confPathLogs.resolve("hs_err_pid%p.log")); - if (version.before("8.18.0") && getVersion().getMajor() >= 7) { - errorFileSub = "-XX:ErrorFile=logs/hs_err_pid%p.log"; + + 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; } diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index 5a92c24b38f84..f46dfa836c8ad 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -436,15 +436,17 @@ private void writeConfiguration() { // Patch jvm.options file to update paths String content = Files.readString(jvmOptionsFile); - Map expansions = getJvmOptionsReplacements(); - for (String key : expansions.keySet()) { + Map expansions = getJvmOptionsReplacements(); + for (var entry : expansions.entrySet()) { + ReplacementKey replacement = entry.getKey(); + String key = replacement.key(); if (content.contains(key) == false) { - LOGGER.warn("Template property '" + key + "' not found in template."); - continue; - // temporarily just warn during backports for https://github.com/elastic/elasticsearch/pull/124966 - // throw new IOException("Template property '" + key + "' not found in template."); + key = replacement.fallback(); + if (content.contains(key) == false) { + throw new IOException("Template property '" + replacement + "' not found in template."); + } } - content = content.replace(key, expansions.get(key)); + content = content.replace(key, entry.getValue()); } Files.writeString(jvmOptionsFile, content); } catch (IOException e) { @@ -910,28 +912,36 @@ private Map getEnvironmentVariables() { return environment; } - private Map getJvmOptionsReplacements() { - var expansions = new HashMap(); + private record ReplacementKey(String key, String fallback) {} + + private Map getJvmOptionsReplacements() { + var expansions = new HashMap(); var version = spec.getVersion(); - String heapDumpPathSub = "# -XX:HeapDumpPath=/heap/dump/path"; - // temporarily duplicate the expansion so both old and new exist during backport - expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + logsDir); + + ReplacementKey heapDumpPathSub; if (version.before("8.18.0") && version.onOrAfter("6.3.0")) { - heapDumpPathSub = "-XX:HeapDumpPath=/heap/dump/path"; + 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"); } expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + logsDir); - String gcLogSub = "gc.log"; - // temporarily duplicate the expansion so both old and new exist during backport - expansions.put(gcLogSub, logsDir.resolve("gc.log").toString()); + + ReplacementKey gcLogSub; if (version.before("8.18.0") && version.onOrAfter("6.2.0")) { - gcLogSub = "logs/gc.log"; + 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, logsDir.resolve("gc.log").toString()); - // temporarily duplicate the expansion so both old and new exist during backport - String errorFileSub = "-XX:ErrorFile=hs_err_pid%p.log"; - expansions.put(errorFileSub, "-XX:ErrorFile=" + logsDir.resolve("hs_err_pid%p.log")); + + ReplacementKey errorFileSub; if (version.before("8.18.0") && version.getMajor() >= 7) { - errorFileSub = "-XX:ErrorFile=logs/hs_err_pid%p.log"; + 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=" + logsDir.resolve("hs_err_pid%p.log")); return expansions; From 75f664d532820c09ba0bb2fd53e9ddf43b7dfacc Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 18 Mar 2025 01:05:21 +0000 Subject: [PATCH 10/19] [CI] Auto commit changes from spotless --- .../test/cluster/local/AbstractLocalClusterFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index f46dfa836c8ad..55dab480611bb 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -443,7 +443,7 @@ private void writeConfiguration() { if (content.contains(key) == false) { key = replacement.fallback(); if (content.contains(key) == false) { - throw new IOException("Template property '" + replacement + "' not found in template."); + throw new IOException("Template property '" + replacement + "' not found in template."); } } content = content.replace(key, entry.getValue()); From b54b497524a71cdebbf30d305ef7033405d12ff9 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 21 Mar 2025 13:33:07 -0700 Subject: [PATCH 11/19] fixes --- .../rest/LegacyYamlRestTestPluginFuncTest.groovy | 1 + .../gradle/testclusters/ElasticsearchNode.java | 2 +- .../org/elasticsearch/server/cli/JvmOption.java | 13 ++++++++++++- .../server/cli/ServerProcessBuilder.java | 9 ++++++++- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy index aa2dba5caff61..90ac5369f5df4 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/test/rest/LegacyYamlRestTestPluginFuncTest.groovy @@ -181,6 +181,7 @@ echo "Running elasticsearch \$0" file(distProjectFolder, 'src/config/jvm.options') << """ -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; diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index fd508556b05e7..2d75fd24c0ce4 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1442,7 +1442,7 @@ private void tweakJvmOptions(Path configFileRoot) { if (content.contains(key) == false) { key = replacement.fallback(); if (content.contains(key) == false) { - throw new IOException("Template property '" + replacement + "' not found in template."); + throw new IOException("Template property '" + replacement + "' not found in template:\n" + content); } } content = content.replace(key, entry.getValue()); diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java index 0fa3e9463fc48..922878598d399 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java @@ -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; @@ -106,7 +108,11 @@ private static List flagsFinal(final List 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 output = readLinesFromInputStream(process.getInputStream()); final List error = readLinesFromInputStream(process.getErrorStream()); final int status = process.waitFor(); @@ -124,6 +130,11 @@ private static List flagsFinal(final List userDefinedJvmOptions) } } + @SuppressForbidden(reason = "ProcessBuilder takes File") + private static void setWorkingDir(ProcessBuilder builder, Path path) { + builder.directory(path.toFile()); + } + private static List readLinesFromInputStream(final InputStream is) throws IOException { try (InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8); BufferedReader br = new BufferedReader(isr)) { return br.lines().toList(); diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java index 243285790c0ce..09765c1214ab6 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java @@ -16,7 +16,9 @@ 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.File; import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; @@ -197,12 +199,17 @@ private static Process createProcess( var builder = new ProcessBuilder(Stream.concat(Stream.of(command), Stream.concat(jvmOptions.stream(), jvmArgs.stream())).toList()); builder.environment().putAll(environment); - builder.directory(workingDir.toFile()); + 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); From c4c5f317dffd70643dcb055b28bf03e674cc5bbf Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 21 Mar 2025 20:41:49 +0000 Subject: [PATCH 12/19] [CI] Auto commit changes from spotless --- .../java/org/elasticsearch/server/cli/ServerProcessBuilder.java | 1 - 1 file changed, 1 deletion(-) diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java index 09765c1214ab6..12a8bc00d4209 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcessBuilder.java @@ -18,7 +18,6 @@ import org.elasticsearch.core.PathUtils; import org.elasticsearch.core.SuppressForbidden; -import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; From 1301be024da13a250f9e4bbdeade26a34c8cc219 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 21 Mar 2025 15:29:44 -0700 Subject: [PATCH 13/19] iter --- .../java/org/elasticsearch/server/cli/JvmOption.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java index 922878598d399..78993b32b1338 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java @@ -109,9 +109,7 @@ private static List flagsFinal(final List userDefinedJvmOptions) Stream.of("-XX:+PrintFlagsFinal", "-version") ).flatMap(Function.identity()).toList(); 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); + setWorkingDir(builder); final Process process = builder.start(); final List output = readLinesFromInputStream(process.getInputStream()); final List error = readLinesFromInputStream(process.getErrorStream()); @@ -131,8 +129,10 @@ private static List flagsFinal(final List userDefinedJvmOptions) } @SuppressForbidden(reason = "ProcessBuilder takes File") - private static void setWorkingDir(ProcessBuilder builder, Path path) { - builder.directory(path.toFile()); + private static void setWorkingDir(ProcessBuilder builder) throws IOException { + // set temp dir as working dir so it is writeable + final Path tmpDir = Files.createTempDirectory("final-flags"); + builder.directory(tmpDir.toFile()); } private static List readLinesFromInputStream(final InputStream is) throws IOException { From e01136fb5284aebc2f3025dbdb5a0a4253a38ecb Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 7 Apr 2025 16:55:30 -0700 Subject: [PATCH 14/19] fix tests --- .../server/cli/JvmOptionsParserTests.java | 21 ++++++++++--------- .../server/cli/ServerProcessTests.java | 13 ++++++++---- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java index 568068e268b23..f74ab472b2cb2 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java @@ -45,14 +45,15 @@ @WithoutSecurityManager public class JvmOptionsParserTests extends ESTestCase { - private static final Map TEST_SYSPROPS = Map.of("os.name", "Linux", "os.arch", "aarch64"); - - private static final Path ENTITLEMENTS_LIB_DIR = Path.of("lib", "entitlement-bridge"); + private static Map testSysprops; @BeforeClass public static void beforeClass() throws IOException { - Files.createDirectories(ENTITLEMENTS_LIB_DIR); - Files.createTempFile(ENTITLEMENTS_LIB_DIR, "mock-entitlements-bridge", ".jar"); + Path homeDir = createTempDir(); + Path entitlementLibDir = homeDir.resolve("lib/entitlement-bridge"); + Files.createDirectories(entitlementLibDir); + Files.createTempFile(entitlementLibDir, "mock-entitlements-bridge", ".jar"); + testSysprops = Map.of("os.name", "Linux", "os.arch", "aarch64", "es.path.home", homeDir.toString()); } @AfterClass @@ -369,30 +370,30 @@ public void accept(final int lineNumber, final String line) { public void testNodeProcessorsActiveCount() { { - final List jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, TEST_SYSPROPS); + final List jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, testSysprops); assertThat(jvmOptions, not(hasItem(containsString("-XX:ActiveProcessorCount=")))); } { Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 1).build(); - final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, TEST_SYSPROPS); + final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, testSysprops); assertThat(jvmOptions, hasItem("-XX:ActiveProcessorCount=1")); } { // check rounding Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 0.2).build(); - final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, TEST_SYSPROPS); + final List jvmOptions = SystemJvmOptions.systemJvmOptions(nodeSettings, testSysprops); assertThat(jvmOptions, hasItem("-XX:ActiveProcessorCount=1")); } { // check validation Settings nodeSettings = Settings.builder().put(EsExecutors.NODE_PROCESSORS_SETTING.getKey(), 10000).build(); - var e = expectThrows(IllegalArgumentException.class, () -> SystemJvmOptions.systemJvmOptions(nodeSettings, TEST_SYSPROPS)); + var e = expectThrows(IllegalArgumentException.class, () -> SystemJvmOptions.systemJvmOptions(nodeSettings, testSysprops)); assertThat(e.getMessage(), containsString("setting [node.processors] must be <=")); } } public void testCommandLineDistributionType() { - var sysprops = new HashMap<>(TEST_SYSPROPS); + var sysprops = new HashMap<>(testSysprops); sysprops.put("es.distribution.type", "testdistro"); final List jvmOptions = SystemJvmOptions.systemJvmOptions(Settings.EMPTY, sysprops); assertThat(jvmOptions, hasItem("-Des.distribution.type=testdistro")); diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java index d4c05937f35b7..9df7cd428cbc3 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java @@ -66,6 +66,7 @@ public class ServerProcessTests extends ESTestCase { protected final Map sysprops = new HashMap<>(); protected final Map envVars = new HashMap<>(); Path esHomeDir; + Path workingDir; Settings.Builder nodeSettings; ProcessValidator processValidator; MainMethod mainCallback; @@ -88,12 +89,14 @@ int runForeground() throws Exception { @Before public void resetEnv() { + esHomeDir = createTempDir(); terminal.reset(); sysprops.clear(); sysprops.put("os.name", "Linux"); sysprops.put("java.home", "javahome"); + sysprops.put("es.path.home", esHomeDir.toString()); envVars.clear(); - esHomeDir = createTempDir(); + workingDir = createTempDir(); nodeSettings = Settings.builder(); processValidator = null; mainCallback = null; @@ -229,7 +232,8 @@ ServerProcess startProcess(boolean daemonize, boolean quiet) throws Exception { .withProcessInfo(pinfo) .withServerArgs(createServerArgs(daemonize, quiet)) .withJvmOptions(List.of()) - .withTempDir(ServerProcessUtils.setupTempDir(pinfo)); + .withTempDir(ServerProcessUtils.setupTempDir(pinfo)) + .withWorkingDir(workingDir); return serverProcessBuilder.start(starter); } @@ -238,7 +242,7 @@ public void testProcessBuilder() throws Exception { assertThat(pb.redirectInput(), equalTo(ProcessBuilder.Redirect.PIPE)); assertThat(pb.redirectOutput(), equalTo(ProcessBuilder.Redirect.INHERIT)); assertThat(pb.redirectError(), equalTo(ProcessBuilder.Redirect.PIPE)); - assertThat(pb.directory(), nullValue()); // leave default, which is working directory + assertThat(pb.directory().toString(), equalTo(workingDir.toString())); // leave default, which is working directory }; mainCallback = (args, stdin, stderr, exitCode) -> { try (PrintStream err = new PrintStream(stderr, true, StandardCharsets.UTF_8)) { @@ -312,7 +316,8 @@ public void testCommandLineSysprops() throws Exception { .withProcessInfo(createProcessInfo()) .withServerArgs(createServerArgs(false, false)) .withJvmOptions(List.of("-Dfoo1=bar", "-Dfoo2=baz")) - .withTempDir(Path.of(".")); + .withTempDir(Path.of(".")) + .withWorkingDir(workingDir); serverProcessBuilder.start(starter).waitFor(); } From 264158fe6519e9cc5736cc0aa7afed765f9490e0 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Tue, 8 Apr 2025 00:05:13 +0000 Subject: [PATCH 15/19] [CI] Auto commit changes from spotless --- .../java/org/elasticsearch/server/cli/ServerProcessTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java index 9df7cd428cbc3..2544b1331dde0 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java @@ -57,7 +57,6 @@ import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.nullValue; public class ServerProcessTests extends ESTestCase { From ae3eb7131a7728b00f6a9f54f51f41628237cf07 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 7 Apr 2025 21:47:58 -0700 Subject: [PATCH 16/19] sidestep forbidden api --- .../java/org/elasticsearch/server/cli/ServerProcessTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java index 2544b1331dde0..c9e233e22dd76 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java @@ -241,7 +241,7 @@ public void testProcessBuilder() throws Exception { assertThat(pb.redirectInput(), equalTo(ProcessBuilder.Redirect.PIPE)); assertThat(pb.redirectOutput(), equalTo(ProcessBuilder.Redirect.INHERIT)); assertThat(pb.redirectError(), equalTo(ProcessBuilder.Redirect.PIPE)); - assertThat(pb.directory().toString(), equalTo(workingDir.toString())); // leave default, which is working directory + assertThat(String.valueOf(pb.directory()), equalTo(workingDir.toString())); // leave default, which is working directory }; mainCallback = (args, stdin, stderr, exitCode) -> { try (PrintStream err = new PrintStream(stderr, true, StandardCharsets.UTF_8)) { From 0c9c3c4c618a12f015dae2c2b2927b06bd949f1e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Apr 2025 05:57:21 -0700 Subject: [PATCH 17/19] address feedback --- .../gradle/testclusters/ElasticsearchNode.java | 6 +++--- .../main/java/org/elasticsearch/server/cli/JvmOption.java | 3 ++- .../test/cluster/local/AbstractLocalClusterFactory.java | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index 2d75fd24c0ce4..d5c804d93c27e 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1460,7 +1460,7 @@ private Map jvmOptionExpansions() { Version version = getVersion(); ReplacementKey heapDumpPathSub; - if (version.before("8.18.0") && version.onOrAfter("6.3.0")) { + if (version.before("8.19.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 @@ -1469,7 +1469,7 @@ private Map jvmOptionExpansions() { expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + confPathLogs); ReplacementKey gcLogSub; - if (version.before("8.18.0") && version.onOrAfter("6.2.0")) { + if (version.before("8.19.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 @@ -1478,7 +1478,7 @@ private Map jvmOptionExpansions() { expansions.put(gcLogSub, confPathLogs.resolve("gc.log").toString()); ReplacementKey errorFileSub; - if (version.before("8.18.0") && version.getMajor() >= 7) { + if (version.before("8.19.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 diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java index 78993b32b1338..03aa7db72d142 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/JvmOption.java @@ -130,7 +130,8 @@ private static List flagsFinal(final List userDefinedJvmOptions) @SuppressForbidden(reason = "ProcessBuilder takes File") private static void setWorkingDir(ProcessBuilder builder) throws IOException { - // set temp dir as working dir so it is writeable + // The real ES process uses the logs dir as the working directory. Since we don't + // have the logs dir yet, here we use a temp directory for calculating jvm options. final Path tmpDir = Files.createTempDirectory("final-flags"); builder.directory(tmpDir.toFile()); } diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index 77795b133b19e..fb594272f1c74 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -924,7 +924,7 @@ private Map getJvmOptionsReplacements() { var version = spec.getVersion(); ReplacementKey heapDumpPathSub; - if (version.before("8.18.0") && version.onOrAfter("6.3.0")) { + if (version.before("8.19.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 @@ -933,7 +933,7 @@ private Map getJvmOptionsReplacements() { expansions.put(heapDumpPathSub, "-XX:HeapDumpPath=" + logsDir); ReplacementKey gcLogSub; - if (version.before("8.18.0") && version.onOrAfter("6.2.0")) { + if (version.before("8.19.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 @@ -942,7 +942,7 @@ private Map getJvmOptionsReplacements() { expansions.put(gcLogSub, logsDir.resolve("gc.log").toString()); ReplacementKey errorFileSub; - if (version.before("8.18.0") && version.getMajor() >= 7) { + if (version.before("8.19.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 From 5243f7708a9cbc6595d56b874463d1b9a4e75bf8 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 8 Apr 2025 06:03:15 -0700 Subject: [PATCH 18/19] don't use mock filesystems, we expect exactly one file --- .../org/elasticsearch/server/cli/JvmOptionsParserTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java index f74ab472b2cb2..2ff7daa395b1c 100644 --- a/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java +++ b/distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/JvmOptionsParserTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.server.cli; +import org.apache.lucene.tests.util.LuceneTestCase; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.core.IOUtils; @@ -43,6 +44,7 @@ import static org.hamcrest.Matchers.not; @WithoutSecurityManager +@LuceneTestCase.SuppressFileSystems("*") public class JvmOptionsParserTests extends ESTestCase { private static Map testSysprops; From 43db0f26933aef3c18dc9d5fd6242ec701a65ff9 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 9 Apr 2025 05:44:26 -0700 Subject: [PATCH 19/19] feedback --- .../gradle/testclusters/ElasticsearchNode.java | 6 +++--- .../cluster/local/AbstractLocalClusterFactory.java | 12 ++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index d5c804d93c27e..6102ebdb28415 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -1461,7 +1461,7 @@ private Map jvmOptionExpansions() { ReplacementKey heapDumpPathSub; if (version.before("8.19.0") && version.onOrAfter("6.3.0")) { - heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", ""); + heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", null); } 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"); @@ -1470,7 +1470,7 @@ private Map jvmOptionExpansions() { ReplacementKey gcLogSub; if (version.before("8.19.0") && version.onOrAfter("6.2.0")) { - gcLogSub = new ReplacementKey("logs/gc.log", ""); + gcLogSub = new ReplacementKey("logs/gc.log", null); } else { // temporarily check the old substitution first so both old and new work during backport gcLogSub = new ReplacementKey("logs/gc.log", "gc.log"); @@ -1479,7 +1479,7 @@ private Map jvmOptionExpansions() { ReplacementKey errorFileSub; if (version.before("8.19.0") && version.getMajor() >= 7) { - errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", ""); + errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", null); } 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"); diff --git a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java index fb594272f1c74..239d5f6da75cd 100644 --- a/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java +++ b/test/test-clusters/src/main/java/org/elasticsearch/test/cluster/local/AbstractLocalClusterFactory.java @@ -917,7 +917,11 @@ private Map getEnvironmentVariables() { return environment; } - private record ReplacementKey(String key, String fallback) {} + private record ReplacementKey(String key, String fallback) { + ReplacementKey { + assert fallback == null || fallback.isEmpty() == false; // no empty fallback, which would match anything + } + } private Map getJvmOptionsReplacements() { var expansions = new HashMap(); @@ -925,7 +929,7 @@ private Map getJvmOptionsReplacements() { ReplacementKey heapDumpPathSub; if (version.before("8.19.0") && version.onOrAfter("6.3.0")) { - heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", ""); + heapDumpPathSub = new ReplacementKey("-XX:HeapDumpPath=data", null); } 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"); @@ -934,7 +938,7 @@ private Map getJvmOptionsReplacements() { ReplacementKey gcLogSub; if (version.before("8.19.0") && version.onOrAfter("6.2.0")) { - gcLogSub = new ReplacementKey("logs/gc.log", ""); + gcLogSub = new ReplacementKey("logs/gc.log", null); } else { // temporarily check the old substitution first so both old and new work during backport gcLogSub = new ReplacementKey("logs/gc.log", "gc.log"); @@ -943,7 +947,7 @@ private Map getJvmOptionsReplacements() { ReplacementKey errorFileSub; if (version.before("8.19.0") && version.getMajor() >= 7) { - errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", ""); + errorFileSub = new ReplacementKey("-XX:ErrorFile=logs/hs_err_pid%p.log", null); } 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");