From 2534671d75b21ebef4f0f05d1384396a14cd9f5f Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 24 Jan 2023 06:57:28 -0800 Subject: [PATCH 1/2] Remove user exception marker from server cli Previously UserExceptions could be thrown from the main Elasticsearch process during startup. With recent refactorings all remaining validation of arguments was moved to the server cli. Since there are no longer any uses of the user exception in server, the marker code is no longer needed. This commit removes that marker and the associated code in the server cli error pump. --- .../server/cli/ErrorPumpThread.java | 23 ++++--------------- .../server/cli/ServerProcessTests.java | 22 +++++------------- 2 files changed, 10 insertions(+), 35 deletions(-) diff --git a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ErrorPumpThread.java b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ErrorPumpThread.java index d8f0c4471c658..397e6a327d1ab 100644 --- a/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ErrorPumpThread.java +++ b/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ErrorPumpThread.java @@ -19,19 +19,14 @@ import java.util.concurrent.CountDownLatch; import static org.elasticsearch.bootstrap.BootstrapInfo.SERVER_READY_MARKER; -import static org.elasticsearch.bootstrap.BootstrapInfo.USER_EXCEPTION_MARKER; import static org.elasticsearch.server.cli.ProcessUtil.nonInterruptibleVoid; /** * A thread which reads stderr of the jvm process and writes it to this process' stderr. * - *

Two special state markers are watched for. These are ascii control characters which signal - * to the cli process something has changed in the server process. The two possible special messages are: - *

+ *

The thread watches for a special state marker from the process. The ascii character + * {@link BootstrapInfo#SERVER_READY_MARKER} signals the server is ready and the cli may + * detach if daemonizing. All other messages are passed through to stderr. */ class ErrorPumpThread extends Thread { private final BufferedReader reader; @@ -43,9 +38,6 @@ class ErrorPumpThread extends Thread { // a flag denoting whether the ready marker has been received by the server process private volatile boolean ready; - // an exception message received alongside the user exception marker, if a bootstrap error has occurred - private volatile String userExceptionMsg; - // an unexpected io failure that occurred while pumping stderr private volatile IOException ioFailure; @@ -66,10 +58,6 @@ String waitUntilReady() throws IOException { if (ioFailure != null) { throw ioFailure; } - if (ready == false) { - return userExceptionMsg; - } - assert userExceptionMsg == null; return null; } @@ -85,10 +73,7 @@ public void run() { try { String line; while ((line = reader.readLine()) != null) { - if (line.isEmpty() == false && line.charAt(0) == USER_EXCEPTION_MARKER) { - userExceptionMsg = line.substring(1); - readyOrDead.countDown(); - } else if (line.isEmpty() == false && line.charAt(0) == SERVER_READY_MARKER) { + if (line.isEmpty() == false && line.charAt(0) == SERVER_READY_MARKER) { ready = true; readyOrDead.countDown(); } else { 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 f0fa37227119d..b4171c326929c 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 @@ -78,9 +78,9 @@ interface ProcessValidator { void validate(ProcessBuilder processBuilder) throws IOException; } - void runForeground() throws Exception { + int runForeground() throws Exception { var server = startProcess(false, false, ""); - server.waitFor(); + return server.waitFor(); } @Before @@ -228,22 +228,12 @@ public void testPid() throws Exception { public void testBootstrapError() throws Exception { mainCallback = (args, stdin, stderr, exitCode) -> { - stderr.println(BootstrapInfo.USER_EXCEPTION_MARKER + "a bootstrap exception"); + stderr.println("a bootstrap exception"); exitCode.set(ExitCodes.CONFIG); }; - var e = expectThrows(UserException.class, () -> runForeground()); - assertThat(e.exitCode, equalTo(ExitCodes.CONFIG)); - assertThat(e.getMessage(), equalTo("a bootstrap exception")); - } - - public void testUserError() throws Exception { - mainCallback = (args, stdin, stderr, exitCode) -> { - stderr.println(BootstrapInfo.USER_EXCEPTION_MARKER + "a user exception"); - exitCode.set(ExitCodes.USAGE); - }; - var e = expectThrows(UserException.class, () -> runForeground()); - assertThat(e.exitCode, equalTo(ExitCodes.USAGE)); - assertThat(e.getMessage(), equalTo("a user exception")); + int exitCode = runForeground(); + assertThat(exitCode, equalTo(ExitCodes.CONFIG)); + assertThat(terminal.getErrorOutput(), containsString("a bootstrap exception")); } public void testStartError() throws Exception { From 3bfaed0bad478dcb0f97738f635b4ad8474261ed Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 24 Jan 2023 07:49:43 -0800 Subject: [PATCH 2/2] actually remove the marker --- .../java/org/elasticsearch/bootstrap/BootstrapInfo.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java index 0e565f22ba763..740833dffc7c8 100644 --- a/server/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java +++ b/server/src/main/java/org/elasticsearch/bootstrap/BootstrapInfo.java @@ -66,13 +66,6 @@ public static ConsoleLoader.Console getConsole() { */ public static final String UNTRUSTED_CODEBASE = "/untrusted"; - /** - * A non-printable character denoting a UserException has occurred. - * - * This is sent over stderr to the controlling CLI process. - */ - public static final char USER_EXCEPTION_MARKER = '\u0015'; - /** * A non-printable character denoting the server is ready to process requests. *