----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16814/#review31626 -----------------------------------------------------------
core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/16814/#comment60298> the summary of steps that we had before is helpful, in addition to the comments inside the API. It is helpful to move it back here. core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/16814/#comment60297> It is a bit confusing to read the code that starts with step #4. Makes you wonder what about steps 1-3. I think it is useful to have the summary of what it does in the API description, in addition to comments inside the API core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala <https://reviews.apache.org/r/16814/#comment60301> new -> New core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala <https://reviews.apache.org/r/16814/#comment60303> receiving replicas -> Replicas that receive the LeaderAndIsr request sent by the leader selector core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala <https://reviews.apache.org/r/16814/#comment60305> Like the comment above says "The reassigned replicas are ALL in the in sync replicas set before this API is invoked." Hence, this check seems unnecessary right? core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala <https://reviews.apache.org/r/16814/#comment60306> Same changes as above core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala <https://reviews.apache.org/r/16814/#comment60308> and here - Neha Narkhede On Jan. 13, 2014, 5:57 a.m., Jun Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16814/ > ----------------------------------------------------------- > > (Updated Jan. 13, 2014, 5:57 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1202 > https://issues.apache.org/jira/browse/KAFKA-1202 > > > Repository: kafka > > > Description > ------- > > 1. Changed ununcessary ZK reads to reading from the controller cache. 2. > ReassignedPartitionLeaderSelector: Make sure the new leader is in the current > isr. 3. ControlledShutdownLeaderSelector: Only make sure that the new leader > is not being shutting down. This means that if the leader is already moved > off the shutting down broker, we will let leader election succeed (even > though it's not necessary). This increases the chance of a successful > controlled shutdown. 4. Added various comments. > > > Diffs > ----- > > core/src/main/scala/kafka/controller/KafkaController.scala > 03ef9cf00602cfb6c6485c5a818a2f54a9e499b7 > core/src/main/scala/kafka/controller/PartitionLeaderSelector.scala > a47b142940fd8aaebbecebf67eb6d74a4dc871de > core/src/main/scala/kafka/controller/PartitionStateMachine.scala > 5859ce7abaf0b675693a025f867ae9cef59f3bf7 > core/src/main/scala/kafka/controller/ReplicaStateMachine.scala > ad4ee53a9516e7e6ea14a9ec72a5aab511810f83 > core/src/main/scala/kafka/utils/ZkUtils.scala > 73902b2bd18b95e00e610fca86d6ed8b4af30e9a > > Diff: https://reviews.apache.org/r/16814/diff/ > > > Testing > ------- > > > Thanks, > > Jun Rao > >
