-
Notifications
You must be signed in to change notification settings - Fork 903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix async log appender not print error log when bookie starting expectionally #4475
base: master
Are you sure you want to change the base?
Conversation
IIUC, you mean call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the user doesn't include logs4j's jars and uses logback or something else?
All prod components of BK should only depend on slf4j.
Good point. But it seems that there is no way to shutdown or force flush the log through slf4j api. So some log will be lost if user using async logger appender. I think that we could print the key error logs to |
rerun failure checks |
Thank you for the update. I understand the need to ensure logs are flushed properly. However, adding multiple BTW, I am trying to fix the ci in #4476 |
Motivation
Fix #4474
The root cause is that when bookie starting error, the java runtime will exit immediately. And the log4j can not shutdown gracefully:
bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
Lines 199 to 201 in e5c418e
Changes
Call
LogManager#shutdown
in jvm shutdown hook.