Vanlightly commented on pull request #2888: URL: https://github.com/apache/bookkeeper/pull/2888#issuecomment-965028346
The order of the shutdown looks good. One significant problem is that the shutdown of the OrderExecutors was not done quite right, causing the shutdown to block for a significant time period (1000 seconds x number of total threads). The forceShutdown method should only be called after having called shutdown() as what forceShutdown does is simply wait for the ExecutorService to terminate, for those 1000 seconds calling shutdownNow() if the timeout is reached. Because shutdown() wasn't called first the ExecutorService is still running normally. So you need to ensure that `service.shutdown()` is called first. I would also say 1000 seconds is quite high, this is the upper bound for each thread, called serially. If we call shutdown() on all executors first, then we could time box the total time we're willing to wait for all executors to be terminated. -- 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]
