gharris1727 commented on code in PR #14729:
URL: https://github.com/apache/kafka/pull/14729#discussion_r1395228455
##########
core/src/main/scala/kafka/network/SocketServer.scala:
##########
@@ -1373,6 +1384,9 @@ private[kafka] class Processor(
try {
beginShutdown()
thread.join()
+ if (!started) {
Review Comment:
> what I mean is: serverChannel and nioSelector are closed in closeAll.
There only be one of these two objects in an Acceptor
I think an Acceptor will always have an nioSelector, and has a serverChannel
sometimes before Acceptor#start is called, and always after Acceptor#start is
called. That seems irrelevant to the Processor discussion though.
> So, is there no need to do extra closeAll at the Processor level?
When Processor is instantiated, Processor#selector is initialized. This is
closed in Processor#stopAll, so we need to ensure that Processor#stopAll is
called exactly once. It's not a ServerSocketChannel, but it should still be
closed like the sockets are.
There are two calls to Processor#closeAll, (A) the Processor#run finally
clause, and (B) the new Processor#close call that I added.
1. If Processor#start is called, the Processor#thread will start, and
Processor#run will eventually exit, call site (A) will be executed. This is the
existing happy-path. When Processor#close is called, call site (B) will be
skipped because the thread was started.
2. If Processor#start is not called, the Processor#thread will not start,
and call site (1) will never activate. Instead call site (2) will activate,
because Processor#started will be false.
If you're asking why we don't remove call site (A) and make call site (B)
unconditional: I didn't want to change the existing happy-path behavior.
If this isn't satisfactory I'll take some time to re-read your messages and
put together a convincing test that demonstrates the need for the Processor
change.
--
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]
