----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28481/#review63757 -----------------------------------------------------------
core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106053> Would be good to clarify what i stands for core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106054> In this example, when you describe an assignment as 2 numbers, it is unclear what each number stands for. Would be good to clarify that core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106057> Would be good to clarify what you mean by "load". Is that # of replicas on the broker? core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106061> Wouldn't the number be negative here? Did you mean sum(abs(FL(i) - L(i))) ? core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106062> same here. What about when FL(i) < L(i) core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106074> With every offload(i), the load status of the cluster changes. So, is one iterator of offload(i) on every broker sufficient to read an ideal state? I guess it is, but this is unclear from the explanation core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106064> This is confusing. How is is that you are offloading from every single broker? core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106066> Actually D(i) < 0 should mean that the actual load on the broker is more than the ideal or fair load, so shouldn't we offload a few replicas from this broker? core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106069> Would be good to clarify if the load is spread in a round robin fashion core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106071> Because it's unclear what the <number>-<number> convention means, it is difficult to understand this part. core/src/main/scala/kafka/admin/AdminUtils.scala <https://reviews.apache.org/r/28481/#comment106072> I think it may be illegal to create a topic with replication factor greater than # of brokers. Would be good to clarify the behavior here. core/src/test/scala/unit/kafka/admin/AdminTest.scala <https://reviews.apache.org/r/28481/#comment106075> Same here. Please explain the convention. <something>-<something>|... - Neha Narkhede On Dec. 3, 2014, 5:25 p.m., Dmitry Pekar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28481/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2014, 5:25 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1792 > https://issues.apache.org/jira/browse/KAFKA-1792 > > > Repository: kafka > > > Description > ------- > > KAFKA-1792: CR > > > Diffs > ----- > > core/src/main/scala/kafka/admin/AdminUtils.scala > 28b12c7b89a56c113b665fbde1b95f873f8624a3 > core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala > 979992b68af3723cd229845faff81c641123bb88 > core/src/test/scala/unit/kafka/admin/AdminTest.scala > e28979827110dfbbb92fe5b152e7f1cc973de400 > topics.json ff011ed381e781b9a177036001d44dca3eac586f > > Diff: https://reviews.apache.org/r/28481/diff/ > > > Testing > ------- > > > Thanks, > > Dmitry Pekar > >