C0urante commented on PR #12485: URL: https://github.com/apache/kafka/pull/12485#issuecomment-1208522020
This should definitely come with a test :) I'm also not sure this is the best approach, since the [ExecutorService::shutdownNow Javadocs](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/concurrent/ExecutorService.html#shutdownNow()) don't give us any guarantees about threads being interrupted: > There are no guarantees beyond best-effort attempts to stop processing actively executing tasks. For example, typical implementations will cancel via [Thread.interrupt()](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/Thread.html#interrupt()), so any task that fails to respond to interrupts may never terminate. And on top of that, by the time the thread is interrupted--if it's interrupted at all--we've already exhausted our graceful shutdown timeout. Can we leverage the existing shutdown logic in `KafkaBasedLog::stop` somehow, possibly by accounting for a `WakeupException` being thrown while reading to the end of the log during `KafkaBasedLog::start`? I'm not certain that it's safe to stop a log while another thread is in the middle of starting the log; we may have to tweak some of the logic there. We may also have to wake up/interrupt/shut down the admin client (if we're using one to read offsets) since that too could potentially block (but perhaps not indefinitely). -- 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]
