mynameborat commented on a change in pull request #1583:
URL: https://github.com/apache/samza/pull/1583#discussion_r806295337
##########
File path:
samza-core/src/main/java/org/apache/samza/runtime/ContainerLaunchUtil.java
##########
@@ -179,15 +187,33 @@ private static void run(
if (containerRunnerException != null) {
log.error("Container stopped with Exception. Exiting process now.",
containerRunnerException);
- System.exit(1);
+ exitCode = 1;
}
} catch (Throwable e) {
- log.error("Container stopped with Exception. ",
containerRunnerException);
+ /*
+ * Two separate log statements are intended to print the entire stack
trace as part of the logs. Using
+ * single log statement with custom format requires explicitly fetching
stack trace and null checks which makes
+ * the code slightly hard to read in comparison with the current choice.
+ */
+ log.error("Exiting the process due to", e);
+ log.error("Container runner exception: ", containerRunnerException);
+ exitCode = 1;
} finally {
coordinatorStreamStore.close();
+ exitProcess(exitCode);
Review comment:
Agreed. I modified it to just exit only the in event of non-zero exit
code.
However, testing this scenario in unit test is extremely painful in the
current setup and hence I am going to leave that code path where we don't exit
the process untested.
e.g., to get the run method fully working as expected and setting up all
components.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
