> On Nov. 14, 2014, 2:39 a.m., Jun Rao wrote:
> > Thanks for the patch. I am not sure if that's the right place to fix it 
> > though. The issue is that when KafkaController.onControllerResignation() is 
> > called, the controller wasn't actually active. A better fix is probably to 
> > guard this in onControllerResignation() and only go through the logic if 
> > the controller is active.
> 
> Sriharsha Chintalapani wrote:
>     Jun, Thanks for the review. When KafkaController.startup() calls it sets 
> the isRunning to true before calling controllerElector.startup which calls 
> elect and if the /controller from previous run still exists it sets leaderId 
> and returns, meanwhile the /controller gets deleted triggering 
> handleDataDelete which calls onControllerResgination(). As per these events 
> KafkaController is started . I tried setting kafkaController.isRunning to 
> false , not sure why its true as default and only do onControllerResignation 
> if isRunning is true. This doesn't work because KafkaController.startup() 
> makes it true before calling controllerElector.startup() which is going to 
> trigger onControllerResignation().  
>     Regarding the patch I don't think Kafkascheduler should throw an 
> exception when a shutdown is called.
>     On a side note onControllerResignation should be a wrapper around if 
> (isRunning) and isRunning by default should be false.
>     Please let me know your thoughts.

Sorry for the late response. It seems that currently onControllerResignation() 
can be called even when the broker is not a controller (e.g. from 
SessionExpirationListener). So, we probably should just make the logic in 
onControllerResignation() more robust. I was thinking that before we shut down 
the KafkaScheduler, we can first check if it's started already (trunk already 
has such a method) and only shut down the scheduler if it's started. This is 
probably better than making the change at the scheduler level since in some 
other use cases, we do expect the shutdown call to only happen after startup.


- Jun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28027/#review61391
-----------------------------------------------------------


On Nov. 14, 2014, 1:34 a.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28027/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2014, 1:34 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1724
>     https://issues.apache.org/jira/browse/KAFKA-1724
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1724. Errors after reboot in single node setup.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/utils/KafkaScheduler.scala 
> 9a16343d2ff7192b741f0b23a6bdf58d8f2bbd3e 
> 
> Diff: https://reviews.apache.org/r/28027/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>

Reply via email to