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

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


Copy link to this message
-
Re: Review Request 14865: Patch for KAFKA-1097
Jun Rao 2013-10-30, 17:17

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14865/#review27787
-----------------------------------------------------------
Thanks for the patch. Could you rebase?
core/src/main/scala/kafka/cluster/Partition.scala
<https://reviews.apache.org/r/14865/#comment54028>

    Should we create a new NotAssignedReplicaException so that we can propagate the error to the client more accurately?

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

    Should we name this addedReplicas? To me, new and old replicas mean the set of replicas after and before the reassignment.

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

    Since we already send the LeaderAndIsr request to both the old and the new replicas in updateLeaderEpochAndSendRequest(). It seems that sending it again to new replicas in startNewReplicasForReassignedPartition() is redundant. I guess we need that call in order to complete the replica state transition?
    

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

    Should we throw an exception here?
- Jun Rao
On Oct. 29, 2013, 5:49 p.m., Neha Narkhede wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14865/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2013, 5:49 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1097
>     https://issues.apache.org/jira/browse/KAFKA-1097
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Make a reusable updateLeaderEpoch API
>
>
> 1. Reverted changes to test log4j.properties and AdminTest 2. Fixed a bug in moving leader logic during partition reassignment:
>
>
> 1. Changed partition reassignment behavior to first expand the replication factor and then shrink it. 2. Fixed the reassign partition tool to check for error conditions and improve logging 3. Did a minor improvement to the patch review tool 4. Changed the version to 0.8.1 since it should be that on trunk
>
>
> Revert "kafka-1097, unit tests broken due to kafka-1098"
>
> This reverts commit 0da98f7e3b5512bdc876438704c6a895e997daee.
>
> kafka-1097, unit tests broken due to kafka-1098
>
>
> Diffs
> -----
>
>   core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala 2f706c94d340f1cd715210b3d8b00597d65b5dd0
>   core/src/main/scala/kafka/cluster/Partition.scala 5ccecd179d33abfc14dcefc35dd68de7474c6978
>   core/src/main/scala/kafka/controller/KafkaController.scala 88d130f55997b72a8590e4cfe92857a7320e70d5
>   core/src/main/scala/kafka/controller/ReplicaStateMachine.scala 212c05d65dcdc147e55f90875bacc940e30342bf
>   core/src/main/scala/kafka/server/ReplicaManager.scala ee1cc0cf451b691eb91d9158ca765aeb60fc3dc8
>   kafka-patch-review.py daf2c3597a81d0d6db574a488f3cf372701fd113
>   project/Build.scala bcd1ca546b3f081d669892677276a0f19fb1236e
>
> Diff: https://reviews.apache.org/r/14865/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Neha Narkhede
>
>