> On Nov. 20, 2013, 6:30 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, lines 260-261 > > <https://reviews.apache.org/r/15711/diff/3/?file=388714#file388714line260> > > > > Instead of delaying by 5 secs, should we just delay by > > config.leaderImbalanceCheckIntervalSeconds?
The problem is that leaderImbalanceCheckIntervalSeconds could be set to any value. If it was set to an hour or more than that and you have a controller failover because of an intermittent GC on the prev controller, and you want to immediately run the rebalnce, it will not happen. There are other cases where this is true. It is better to decouple the values. I can make it configurable but did not see a strong reason to do so. > On Nov. 20, 2013, 6:30 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, lines 745-746 > > <https://reviews.apache.org/r/15711/diff/3/?file=388714#file388714line745> > > > > Could we rename updateZk to sth like isTriggeredByCommandLine? I dont like the external usage to leak into the code. I see your intent to make the usage of this flag more explicit. How about isTriggeredByAutoRebalance and not update zk if it is set? > On Nov. 20, 2013, 6:30 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, lines 236-239 > > <https://reviews.apache.org/r/15711/diff/3/?file=388715#file388715line236> > > > > I am wondering if this config is really necessary. Wouldn't it be > > simpler to always do the balancing on all partitions that are not already > > on the preferred replica? I do think there is value in this. To ensure rebalance happens always we can set it to 0. There are cases where few topic partition movements does not enforce a rebalance and hence cause unavailability. It is useful to have this to operationalize this feature and understand its behavior in production. - Sriram ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15711/#review29177 ----------------------------------------------------------- On Nov. 20, 2013, 1:38 a.m., Sriram Subramanian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15711/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2013, 1:38 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-930 > https://issues.apache.org/jira/browse/KAFKA-930 > > > Repository: kafka > > > Description > ------- > > 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 > 88792c2b2a360e928ab9cd00de151e5d5f94452d > core/src/main/scala/kafka/server/KafkaConfig.scala > b324344d0a383398db8bfe2cbeec2c1378fe13c9 > > Diff: https://reviews.apache.org/r/15711/diff/ > > > Testing > ------- > > > Thanks, > > Sriram Subramanian > >
