> On Dec. 3, 2013, 1:30 a.m., Neha Narkhede wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, line 235 > > <https://reviews.apache.org/r/15953/diff/1/?file=392920#file392920line235> > > > > it seems that onControllerFailover is already protected by the > > controllerLock. The elect() API of ZookeeperLeaderElector is invoked in 3 > > places and each of those acquires the controllerLock
You are right. The real issue is not that onControllerFailover is not synchronized, but is that the sendRequest is asynchronized. Hence in onControllerFailover, it just put the request on the queue, and while the send thread wakes up to send the message, it may have already been closed by the handleNewSession procedure. I think the correct fix should be, in ControllerChannelManager.removeExistingBroker, we should also clear the request queue. - Guozhang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15953/#review29636 ----------------------------------------------------------- On Dec. 3, 2013, 12:58 a.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15953/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2013, 12:58 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1134 > https://issues.apache.org/jira/browse/KAFKA-1134 > > > Repository: kafka > > > Description > ------- > > KAFKA-1134.v1 > > > Diffs > ----- > > core/src/main/scala/kafka/controller/KafkaController.scala > 4c319aba97655e7c4ec97fac2e34de4e28c9f5d3 > > Diff: https://reviews.apache.org/r/15953/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >