From beb12c3beec5165e4c419c3d3cc73f9f72086ecb Mon Sep 17 00:00:00 2001 From: cryptoe Date: Mon, 20 Jul 2026 17:17:56 -0400 Subject: [PATCH 1/3] Update forking task runner to use bin/run-java --- docs/configuration/index.md | 2 +- .../indexing/overlord/ForkingTaskRunner.java | 25 +++++++++++- .../config/ForkingTaskRunnerConfig.java | 4 +- .../overlord/ForkingTaskRunnerTest.java | 40 +++++++++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/docs/configuration/index.md b/docs/configuration/index.md index ab864cb21ce9..746eb7537526 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -1315,7 +1315,7 @@ Middle Managers pass their configurations down to their child peons. The Middle |--------|-----------|-------| |`druid.indexer.runner.allowedPrefixes`|Whitelist of prefixes for configs that can be passed down to child peons.|`com.metamx`, `druid`, `org.apache.druid`, `user.timezone`, `file.encoding`, `java.io.tmpdir`, `hadoop`| |`druid.indexer.runner.classpath`|Java classpath for the peon.|`System.getProperty("java.class.path")`| -|`druid.indexer.runner.javaCommand`|Command required to execute java.|java| +|`druid.indexer.runner.javaCommand`| Command required to execute java. When left at the *default*, Druid uses the bundled `bin/run-java` script if it is present in the working directory (it honors the `DRUID_JAVA_HOME`/`JAVA_HOME` environment variables), otherwise it falls back to `java` on the `PATH`. |java| |`druid.indexer.runner.javaOpts`|_DEPRECATED_ A string of -X Java options to pass to the peon's JVM. Quotable parameters or parameters with spaces are encouraged to use javaOptsArray|`''`| |`druid.indexer.runner.javaOptsArray`|A JSON array of strings to be passed in as options to the peon's JVM. This is additive to `druid.indexer.runner.javaOpts` and is recommended for properly handling arguments which contain quotes or spaces like `["-XX:OnOutOfMemoryError=kill -9 %p"]`|`[]`| |`druid.indexer.runner.startPort`|Starting port used for Peon services, should be greater than 1023 and less than 65536.|8100| diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java index 52c2dcc7ba38..0b26f27c4024 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java @@ -100,6 +100,7 @@ public class ForkingTaskRunner private static final EmittingLogger LOGGER = new EmittingLogger(ForkingTaskRunner.class); private static final String CHILD_PROPERTY_PREFIX = "druid.indexer.fork.property."; + private static final String RUN_JAVA_COMMAND = "bin/run-java"; /** * Properties to add on Java 11+. When updating this list, update all four: * 1) ForkingTaskRunner#STRONG_ENCAPSULATION_PROPERTIES (here) --> @@ -245,7 +246,7 @@ public TaskStatus call() taskClasspath = config.getClasspath(); } - command.add(config.getJavaCommand()); + command.add(getJavaCommand()); if (JvmUtils.majorVersion() >= 11) { command.addAll(STRONG_ENCAPSULATION_PROPERTIES); @@ -506,6 +507,28 @@ public TaskStatus call() } } + private String getJavaCommand() + { + return getJavaCommand(config.getJavaCommand(), new File(".")); + } + + /** + * Resolves the command used to launch the peon JVM. When the operator has not overridden + * {@code druid.indexer.runner.javaCommand} (i.e. it is still the default {@code "java"}), and the bundled + * {@link #RUN_JAVA_COMMAND} script is present in {@code workingDir}, prefer it so that {@code DRUID_JAVA_HOME} / + * {@code JAVA_HOME} are honored. Otherwise fall back to the configured command (the plain {@code java} on the + * PATH by default). The existence check against {@code workingDir} is reliable because peons inherit this + * process's working directory. + */ + public static String getJavaCommand(String configuredJavaCommand, File workingDir) + { + if (ForkingTaskRunnerConfig.DEFAULT_JAVA_COMMAND.equals(configuredJavaCommand) + && new File(workingDir, RUN_JAVA_COMMAND).exists()) { + return RUN_JAVA_COMMAND; + } + return configuredJavaCommand; + } + @VisibleForTesting ProcessHolder runTaskProcess(List command, File logFile, TaskLocation taskLocation) throws IOException { diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java index 2f0add97a857..ed89fff9d742 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java @@ -36,9 +36,11 @@ public class ForkingTaskRunnerConfig public static final String JAVA_OPTS_ARRAY_PROPERTY = IndexingServiceModuleHelper.INDEXER_RUNNER_PROPERTY_PREFIX + ".javaOptsArray"; + public static final String DEFAULT_JAVA_COMMAND = "java"; + @JsonProperty @NotNull - private String javaCommand = "java"; + private String javaCommand = DEFAULT_JAVA_COMMAND; /** * This is intended for setting -X parameters on the underlying java. It is used by first splitting on whitespace, diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java index 103d57e13437..f0a12d32c42c 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java @@ -79,6 +79,46 @@ public class ForkingTaskRunnerTest @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Test + public void testGetJavaCommandPrefersRunJavaScriptWhenPresent() throws IOException + { + final File workingDir = temporaryFolder.newFolder(); + final File binDir = new File(workingDir, "bin"); + Assert.assertTrue(binDir.mkdirs()); + Assert.assertTrue(new File(binDir, "run-java").createNewFile()); + + Assert.assertEquals( + "bin/run-java", + ForkingTaskRunner.getJavaCommand(new ForkingTaskRunnerConfig().getJavaCommand(), workingDir) + ); + } + + @Test + public void testGetJavaCommandFallsBackToJavaWhenScriptAbsent() throws IOException + { + final File workingDir = temporaryFolder.newFolder(); + + Assert.assertEquals( + "java", + ForkingTaskRunner.getJavaCommand(new ForkingTaskRunnerConfig().getJavaCommand(), workingDir) + ); + } + + @Test + public void testGetJavaCommandRespectsExplicitOverride() throws IOException + { + final File workingDir = temporaryFolder.newFolder(); + final File binDir = new File(workingDir, "bin"); + Assert.assertTrue(binDir.mkdirs()); + Assert.assertTrue(new File(binDir, "run-java").createNewFile()); + + // An explicit, non-default javaCommand must be used verbatim even when the run-java script is present. + Assert.assertEquals( + "/opt/jdk/bin/java", + ForkingTaskRunner.getJavaCommand("/opt/jdk/bin/java", workingDir) + ); + } + // This tests the test to make sure the test fails when it should. @Test(expected = AssertionError.class) public void testPatternMatcherFailureForJavaOptions() From 57ad0a9e60f1a9df7446b3f23b26fdcbc0135075 Mon Sep 17 00:00:00 2001 From: cryptoe Date: Tue, 21 Jul 2026 10:15:58 -0400 Subject: [PATCH 2/3] Address comments --- docs/configuration/index.md | 2 +- .../indexing/overlord/ForkingTaskRunner.java | 25 +++++++++++-------- .../config/ForkingTaskRunnerConfig.java | 15 +++++++++-- .../overlord/ForkingTaskRunnerTest.java | 15 ++++++++--- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 746eb7537526..308b523a0da4 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -1315,7 +1315,7 @@ Middle Managers pass their configurations down to their child peons. The Middle |--------|-----------|-------| |`druid.indexer.runner.allowedPrefixes`|Whitelist of prefixes for configs that can be passed down to child peons.|`com.metamx`, `druid`, `org.apache.druid`, `user.timezone`, `file.encoding`, `java.io.tmpdir`, `hadoop`| |`druid.indexer.runner.classpath`|Java classpath for the peon.|`System.getProperty("java.class.path")`| -|`druid.indexer.runner.javaCommand`| Command required to execute java. When left at the *default*, Druid uses the bundled `bin/run-java` script if it is present in the working directory (it honors the `DRUID_JAVA_HOME`/`JAVA_HOME` environment variables), otherwise it falls back to `java` on the `PATH`. |java| +|`druid.indexer.runner.javaCommand`| Command required to execute java. If not specified, Druid uses the bundled `bin/run-java` script when it is present in the working directory (it honors the `DRUID_JAVA_HOME`/`JAVA_HOME` environment variables), otherwise it falls back to `java` on the `PATH`.|`bin/run-java` if present, else `java`| |`druid.indexer.runner.javaOpts`|_DEPRECATED_ A string of -X Java options to pass to the peon's JVM. Quotable parameters or parameters with spaces are encouraged to use javaOptsArray|`''`| |`druid.indexer.runner.javaOptsArray`|A JSON array of strings to be passed in as options to the peon's JVM. This is additive to `druid.indexer.runner.javaOpts` and is recommended for properly handling arguments which contain quotes or spaces like `["-XX:OnOutOfMemoryError=kill -9 %p"]`|`[]`| |`druid.indexer.runner.startPort`|Starting port used for Peon services, should be greater than 1023 and less than 65536.|8100| diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java index 0b26f27c4024..1c9b13a11752 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java @@ -513,20 +513,25 @@ private String getJavaCommand() } /** - * Resolves the command used to launch the peon JVM. When the operator has not overridden - * {@code druid.indexer.runner.javaCommand} (i.e. it is still the default {@code "java"}), and the bundled - * {@link #RUN_JAVA_COMMAND} script is present in {@code workingDir}, prefer it so that {@code DRUID_JAVA_HOME} / - * {@code JAVA_HOME} are honored. Otherwise fall back to the configured command (the plain {@code java} on the - * PATH by default). The existence check against {@code workingDir} is reliable because peons inherit this - * process's working directory. + * Resolves the command used to launch the peon JVM, in order of precedence: + *
    + *
  1. the operator-specified {@code druid.indexer.runner.javaCommand}, if set;
    2. + *
  2. the bundled {@link #RUN_JAVA_COMMAND} script if present in {@code workingDir}, so that + * {@code DRUID_JAVA_HOME} / {@code JAVA_HOME} are honored;
    3. + *
  3. otherwise plain {@code java} on the {@code PATH}.
    4. + *
+ * The existence check against {@code workingDir} is reliable because peons inherit this process's working + * directory. */ - public static String getJavaCommand(String configuredJavaCommand, File workingDir) + public static String getJavaCommand(@Nullable String configuredJavaCommand, File workingDir) { - if (ForkingTaskRunnerConfig.DEFAULT_JAVA_COMMAND.equals(configuredJavaCommand) - && new File(workingDir, RUN_JAVA_COMMAND).exists()) { + if (configuredJavaCommand != null) { + return configuredJavaCommand; + } + if (new File(workingDir, RUN_JAVA_COMMAND).exists()) { return RUN_JAVA_COMMAND; } - return configuredJavaCommand; + return ForkingTaskRunnerConfig.DEFAULT_JAVA_COMMAND; } @VisibleForTesting diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java index ed89fff9d742..e6ee15ccca75 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/config/ForkingTaskRunnerConfig.java @@ -24,9 +24,11 @@ import com.google.common.collect.Lists; import org.apache.druid.guice.IndexingServiceModuleHelper; +import javax.annotation.Nullable; import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotNull; +import java.io.File; import java.util.List; public class ForkingTaskRunnerConfig @@ -38,9 +40,12 @@ public class ForkingTaskRunnerConfig public static final String DEFAULT_JAVA_COMMAND = "java"; + /** + * Intentionally has no default. A null value means the operator has not overridden it, which lets + * {@code ForkingTaskRunner} decide between the bundled {@code bin/run-java} script and plain {@code java}. + */ @JsonProperty - @NotNull - private String javaCommand = DEFAULT_JAVA_COMMAND; + private String javaCommand = null; /** * This is intended for setting -X parameters on the underlying java. It is used by first splitting on whitespace, @@ -89,6 +94,12 @@ public class ForkingTaskRunnerConfig "hadoop" ); + /** + * See method {@link org.apache.druid.indexing.overlord.ForkingTaskRunner#getJavaCommand(String, File)} method to see + * how this config is used. + * @return + */ + @Nullable public String getJavaCommand() { return javaCommand; diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java index f0a12d32c42c..073df21668ee 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java @@ -41,6 +41,7 @@ import org.apache.druid.indexing.seekablestream.supervisor.SeekableStreamSupervisorStateTest; import org.apache.druid.indexing.worker.config.WorkerConfig; import org.apache.druid.jackson.DefaultObjectMapper; +import org.apache.druid.java.util.common.FileUtils; import org.apache.druid.java.util.common.Pair; import org.apache.druid.java.util.common.RE; import org.apache.druid.java.util.common.granularity.AllGranularity; @@ -84,12 +85,13 @@ public void testGetJavaCommandPrefersRunJavaScriptWhenPresent() throws IOExcepti { final File workingDir = temporaryFolder.newFolder(); final File binDir = new File(workingDir, "bin"); - Assert.assertTrue(binDir.mkdirs()); + FileUtils.mkdirp(binDir); Assert.assertTrue(new File(binDir, "run-java").createNewFile()); + // No configured command (null) and the run-java script is present -> use it. Assert.assertEquals( "bin/run-java", - ForkingTaskRunner.getJavaCommand(new ForkingTaskRunnerConfig().getJavaCommand(), workingDir) + ForkingTaskRunner.getJavaCommand(null, workingDir) ); } @@ -97,13 +99,18 @@ public void testGetJavaCommandPrefersRunJavaScriptWhenPresent() throws IOExcepti public void testGetJavaCommandFallsBackToJavaWhenScriptAbsent() throws IOException { final File workingDir = temporaryFolder.newFolder(); - Assert.assertEquals( "java", - ForkingTaskRunner.getJavaCommand(new ForkingTaskRunnerConfig().getJavaCommand(), workingDir) + ForkingTaskRunner.getJavaCommand(null, workingDir) ); } + @Test + public void testConfigHasNoDefaultJavaCommand() + { + Assert.assertNull(new ForkingTaskRunnerConfig().getJavaCommand()); + } + @Test public void testGetJavaCommandRespectsExplicitOverride() throws IOException { From c1a40dd593a9778146ed03674ee25858c869558d Mon Sep 17 00:00:00 2001 From: cryptoe Date: Tue, 21 Jul 2026 10:24:55 -0400 Subject: [PATCH 3/3] Fix forbidden issues. --- .../apache/druid/indexing/overlord/ForkingTaskRunnerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java index 073df21668ee..a3e6cec6bd2b 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/ForkingTaskRunnerTest.java @@ -116,7 +116,7 @@ public void testGetJavaCommandRespectsExplicitOverride() throws IOException { final File workingDir = temporaryFolder.newFolder(); final File binDir = new File(workingDir, "bin"); - Assert.assertTrue(binDir.mkdirs()); + FileUtils.mkdirp(binDir); Assert.assertTrue(new File(binDir, "run-java").createNewFile()); // An explicit, non-default javaCommand must be used verbatim even when the run-java script is present.