Fix async log appender not print error log when bookie starting expectionally

[AnonHxy]

Motivation

Fix https://github.com/apache/bookkeeper/issues/4474

The root cause is that when bookie starting error, the java runtime will exit immediately. And the log4j can not shutdown gracefully: https://github.com/apache/bookkeeper/blob/e5c418e06e1cdba1c4681159c7266a43fa403cb1/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java#L199-L201

Changes

Call LogManager#shutdown in jvm shutdown hook.

[hezhangjian]

IIUC, you mean call LogManager#shutdown will flush the log to the disk?

[AnonHxy]

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 stderr. So that users can find cause for abnormal exit from the termianl or stderr logs. @dlg99 @shoothzj

[AnonHxy]

rerun failure checks

[hezhangjian]

Thank you for the update. I understand the need to ensure logs are flushed properly. However, adding multiple System.err outputs might not be the most elegant solution. Could we consider pushing for adding this hook to log4j2 instead?

BTW, I am trying to fix the ci in #4476

[StevenLuMT]

reopen's reason: rerun failure checks

[lhotari]

Thank you for the update. I understand the need to ensure logs are flushed properly. However, adding multiple System.err outputs might not be the most elegant solution. Could we consider pushing for adding this hook to log4j2 instead?

When using Log4J2, it would be necessary to call org.apache.logging.log4j.LogManager.shutdown() to ensure that logs get flushed before a forceful JVM exit with Runtime.getRuntime().halt(int).

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.

In Pulsar, this is handled with reflection: https://github.com/apache/pulsar/blob/master/pulsar-common/src/main/java/org/apache/pulsar/common/util/ShutdownUtil.java#L27-L40