Fix server cli to always pass through exit code #104943
Conversation
A previous refactoring remove the user exception messages once passed back from Elasticsearch. However, ES may still die while starting up, and the cli needs to itself exit nicely when that happens. This commit passes back the ready state of ES when checked in the cli. relates elastic#104055
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
Hi @rjernst, I've created a changelog YAML for you. |
| @@ -54,12 +54,12 @@ class ErrorPumpThread extends Thread { | |||
| * @return a bootstrap exeption message if a bootstrap error occurred, or null otherwise | |||
There was a problem hiding this comment.
| * @return a bootstrap exeption message if a bootstrap error occurred, or null otherwise | |
| * @return {@code true} if successful, {@code false} if a bootstrap error occurred |
| @@ -54,12 +54,12 @@ class ErrorPumpThread extends Thread { | |||
| * @return a bootstrap exeption message if a bootstrap error occurred, or null otherwise | |||
| * @throws IOException if there was a problem reading from stderr of the process | |||
| */ | |||
| String waitUntilReady() throws IOException { | |||
| boolean waitUntilReady() throws IOException { | |||
| nonInterruptibleVoid(readyOrDead::await); | |||
| if (ioFailure != null) { | |||
| throw ioFailure; | |||
There was a problem hiding this comment.
Currently you have waitUntilReady rethrowing exceptions. That would not wait for the forked process to exit, nor convert that exception into a UserException as the existing code does.
Just a question regarding your point on the expected behavior on IOExceptions, currently these are rethrown as UncheckedIOException in ServerProcessBuilder. Are we good keeping it like that?
There was a problem hiding this comment.
At least for now, yes. The way we get to an IOException is, afaik, a broken pipe. That could happen because of bad code, or because ES got a SIGKILL (I think?). Perhaps we should provide a "nicer" message to users in this case than an exception stacktrace, but I leave that for a future discussion/PR.
rjernst
added
the
auto-merge
In certain circumstances if Elasticsearch encounters an error while starting up, the server cli may exit with no error. This commit fixes the cli to always check and wait on the Elasticsearch process and exit with the same exit code. relates elastic#104055
In certain circumstances if Elasticsearch encounters an error while starting up, the server cli may exit with no error. This commit fixes the cli to always check and wait on the Elasticsearch process and exit with the same exit code.
relates #104055