Yes. That sounds good. It also fixes the visibility of processors, which seems to have a potential issue now.
Thanks, Jun On Tue, Mar 31, 2015 at 11:25 PM, Gwen Shapira <gshap...@cloudera.com> wrote: > Thanks! > > Yeah, the shutdown hook will run on a separate thread. > > In that case, I don't need (or want) to keep acceptors in > ConcurrentHashMap - we are either starting up or shutting down a > broker, but don't want to do both at same time. > > I'll just synchronize the acceptors startup and shutdown in SocketServer. > > Does that make sense? > > > > > > On Tue, Mar 31, 2015 at 7:11 PM, Jun Rao <j...@confluent.io> wrote: > > I think this is so that the thread that calls shutdown() is guaranteed to > > see the latest value of acceptor. The thread that calls shutdown() is > > typically different from the thread that calls startup() where acceptor > is > > initialized. > > > > Thanks, > > > > Jun > > > > On Tue, Mar 31, 2015 at 4:54 PM, Gwen Shapira <gshap...@cloudera.com> > wrote: > > > >> Hi, > >> > >> In SocketServer.scala, acceptor is defined as a volatile var: > >> > >> > https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/network/SocketServer.scala#L54 > >> > >> My understanding is that @volatile is used to protect variables that > >> are accessed and modified by multiple threads. However, it looks like > >> acceptor variable is only accessed by the main KafkaServer thread. > >> > >> Am I missing anything? What are we protecting the acceptor from? > >> > >> The reason I'm asking is that when modifying the code for multi-port > >> access, I turned a single acceptor var into a ConcurrentHashMap. > >> However, if there are no threads accessing this variable, it probably > >> doesn't need to be a ConcurrentHashMap, and it probably doesn't need > >> to be a volatile either. > >> > >> Debugging concurrency issues after the fact is tricky, so it will be > >> nice if someone who worked on this can comment :) > >> > >> Gwen > >> >
