----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15711/#review30829 -----------------------------------------------------------
core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment59019> Could we just write this as preferredREplicaForTopicsByBrokers: Map[Int, Map[TopicAndPartition, Seq[Int]] = controllerContext.controllerLock synchronized { ... } Then, we can make preferredREplicaForTopicsByBrokers a val. core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment59023> Ditto for topicsNotInPreferredReplica. It seems that the last 5 statements don't need to be inside the controller lock. core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment59031> I think we should use liveOrShuttingDownBrokerIds instead. core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment59030> Not sure if it's better to do the preferred partition election through ZK watcher. The issue is that the controller will hold the lock while doing the preferred replica election for multiple partitions, which could take time. This means if there is another more important watcher fired (e.g., a broker failure), the controller won't be able to handle it immediately. If we call onPreferredReplica() directly one partition at a time in the scheduler, the controller will get a chance to process other watchers. Another way is to create the preferred replica path in ZK one partition at a time. However, this is probably too much overhead. - Jun Rao On Dec. 20, 2013, 7:23 p.m., Sriram Subramanian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15711/ > ----------------------------------------------------------- > > (Updated Dec. 20, 2013, 7:23 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-930 > https://issues.apache.org/jira/browse/KAFKA-930 > > > Repository: kafka > > > Description > ------- > > some more changes > > > use zk for auto rebalance > > > Address code review feedbacks > > > Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into > trunk > > > commit missing code > > > some more changes > > > fix merge conflicts > > > Add auto leader rebalance support > > > Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into > trunk > > > Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into > trunk > > Conflicts: > core/src/main/scala/kafka/admin/AdminUtils.scala > core/src/main/scala/kafka/admin/TopicCommand.scala > > change comments > > > commit the remaining changes > > > Move AddPartitions into TopicCommand > > > Diffs > ----- > > core/src/main/scala/kafka/controller/KafkaController.scala > 4c319aba97655e7c4ec97fac2e34de4e28c9f5d3 > core/src/main/scala/kafka/server/KafkaConfig.scala > b324344d0a383398db8bfe2cbeec2c1378fe13c9 > > Diff: https://reviews.apache.org/r/15711/diff/ > > > Testing > ------- > > > Thanks, > > Sriram Subramanian > >
