----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15711/#review29597 -----------------------------------------------------------
core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment56993> instead of hardcoding this to 5 seconds, how about delaying it by leaderImbalanceCheckIntervalSeconds? core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment57002> this API is now a little awkward due to the updateZK parameter. Do we really need it? Another way is for the partition-rebalance-thread to always ensure creating the path and let this API delete it. This will keep the API clean. core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment57001> it seems we only need the preferred replica per partition, not the entire set of replicas right? In that case, we can simplify preferredReplicasForTopicsByBrokers to Map[Int, Map[TopicAndPartition, Int]] and call it preferredReplicaForPartitionsByBrokers core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment56995> It seems we don't need the brokerIds variable since it is never reused beyond the check in the if statement core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment56999> we also don't have semicolons as a coding convention. Difficult to switch between java and scala, eh? :) core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment56997> "trigger a leader rebalance for partitions that should have a leader on this broker" ? core/src/main/scala/kafka/controller/KafkaController.scala <https://reviews.apache.org/r/15711/#comment57000> could we rename topicPartition to replicasPerPartition? core/src/main/scala/kafka/server/KafkaConfig.scala <https://reviews.apache.org/r/15711/#comment56992> do we need this config option? It seems that the same could be achieved by setting a very high value for leader.imbalance.check.interval.seconds. - Neha Narkhede On Nov. 21, 2013, 5:42 p.m., Sriram Subramanian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15711/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2013, 5:42 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-930 > https://issues.apache.org/jira/browse/KAFKA-930 > > > Repository: kafka > > > Description > ------- > > 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 > >
