[GitHub] reddycharan commented on issue #1543: Issue 1540: Bookie/BookieServer components shutdown will fail to end exit the BookieProcess

[GitBox <gi...@apache.org>]

reddycharan commented on issue #1543: Issue 1540: Bookie/BookieServer components shutdown will fail to end exit the BookieProcess
URL: https://github.com/apache/bookkeeper/pull/1543#issuecomment-405357331
 
 
   @sijie 
   
   hmm.. I kind of understood what you are doing, but I'm not sure why it is called "ExceptionHandler" (all across the code) and in the fist place why we need 'UncaughtExceptionHandler'.
   
   In this change, BookieServer is signaling that it is not running and it is done, so https://github.com/apache/bookkeeper/pull/1543/files#diff-e7149cec76f474d8ae8a515cbe0464e8R74 can start ‘shutdownHookThread’ (ComponentShutdownHook) in ComponentStarter. So all it is doing is signaling  ‘ComponentStarter’ to start ‘shutdownHookThread’, which completes future, so that Main.doMain need not wait for future anymore. Also exception passed to handler is dropped/ignored in the uncaught exception handler you created https://github.com/apache/bookkeeper/pull/1543/files#diff-e7149cec76f474d8ae8a515cbe0464e8R74. So it doesn’t makes sense completely to call ExceptionHandler.
   
   And the other (major) concern I have is, in the previous versions we make sure “System.Exit” is called in the shutdown path of Bookie/BookieServer - https://github.com/apache/bookkeeper/blob/branch-4.4/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java#L401 . This is completely missing in the current LifeCycleComponent infrastructure and also in your current change. This is needed because for whatsoever reason non-daemon thread could be running in this Bookie process, though we have come to this (or completed) shutdown path and it could prevent Bookie process from shutdown, no matter how thorough we are with our lifecyclecomponent and core components handling/management.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services