Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Kafka, mail # dev - Review Request 15711: Patch for KAFKA-930


Copy link to this message
-
Re: Review Request 15711: Patch for KAFKA-930
Jun Rao 2013-11-21, 18:12

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15711/#review29240
-----------------------------------------------------------

core/src/main/scala/kafka/controller/KafkaController.scala
<https://reviews.apache.org/r/15711/#comment56377>

    Does this need to be at info level?

core/src/main/scala/kafka/controller/KafkaController.scala
<https://reviews.apache.org/r/15711/#comment56373>

    Could we rewrite it as the following to make it clear?
    
    groupBy { case(topicAndParttion, assignedReplicas) : => .. }

core/src/main/scala/kafka/controller/KafkaController.scala
<https://reviews.apache.org/r/15711/#comment56374>

    Can we do the following so that we can use named variables?
    
    foreach { case(leaderBroker, (topicAndPartition, assignedReplicas)) => ..
    
    

core/src/main/scala/kafka/controller/KafkaController.scala
<https://reviews.apache.org/r/15711/#comment56375>

    Same here. Could we give item._1 a name so that it's clear what the referenced value is?

core/src/main/scala/kafka/controller/KafkaController.scala
<https://reviews.apache.org/r/15711/#comment56376>

    Should this be info level logging?

core/src/main/scala/kafka/controller/KafkaController.scala
<https://reviews.apache.org/r/15711/#comment56378>

    Same here. It would be better to give a referenced name for topicPartition._1.
- Jun Rao
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
>
>