-----------------------------------------------------------
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
> 
>

Reply via email to