> On Nov. 1, 2013, 4:33 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/KafkaController.scala, lines 383-387
> > <https://reviews.apache.org/r/14865/diff/4/?file=375158#file375158line383>
> >
> > In moveReassignedPartitionLeaderIfRequired(), we call
> > partitionStateMachine.handleStateChanges(), which will use the old assigned
> > replicas. It seems that we need to call
> > updateAssignedReplicasForPartition() before this step so that we can pick
> > up the new assigned replicas.
>> In moveReassignedPartitionLeaderIfRequired(), we call
>> partitionStateMachine.handleStateChanges(), which will use the old assigned
>> replicas.
It doesn't. See this
// change the assigned replica list to just the reassigned replicas in the
cache so it gets sent out on the LeaderAndIsr
// request to the current or new leader. This will prevent it from adding
the old replicas to the ISR
val oldAndNewReplicas =
controllerContext.partitionReplicaAssignment(topicAndPartition)
controllerContext.partitionReplicaAssignment.put(topicAndPartition,
reassignedReplicas)
We do this before the partitionStateMachine.handleStateChanges() call and it
correctly sends the shrunk assigned replicas.
> On Nov. 1, 2013, 4:33 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/KafkaController.scala, lines 387-388
> > <https://reviews.apache.org/r/14865/diff/4/?file=375158#file375158line387>
> >
> > We can use reassignedReplicas for
> > reassignedPartitionContext.newReplicas.
Though both are same, changed it
> On Nov. 1, 2013, 4:33 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/controller/KafkaController.scala, lines 631-632
> > <https://reviews.apache.org/r/14865/diff/4/?file=375158#file375158line631>
> >
> > It probably doesn't hurt to send the leaderAndIsr request to both the
> > old and the new replicas. However, it seems that we really just need to
> > send to the new replicas since in the caller onPartitionReassignment(), we
> > will be stopping the old replicas immediately afterwards.
Agree, but I'm afraid I may be missing some corner cases in which case sending
more data will be useful compared to sending less data.
- Neha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14865/#review28011
-----------------------------------------------------------
On Oct. 31, 2013, 5:37 p.m., Neha Narkhede wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14865/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 5:37 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1097
> https://issues.apache.org/jira/browse/KAFKA-1097
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Addressed Guozhang's comment to reduce the data logged to state change log
>
>
> Addressed Jun and Guozhang's review comments
>
>
> Rebased after KAFKA-1001 was checked in
>
>
> 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
> d8078bd4e73a8a80be436396765ead4c62d334bd
> core/src/main/scala/kafka/common/NotAssignedReplicaException.scala
> PRE-CREATION
> 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
> 7b8f89edecec446664fb4abe64f5dfafb9aa3120
> kafka-patch-review.py daf2c3597a81d0d6db574a488f3cf372701fd113
> project/Build.scala bcd1ca546b3f081d669892677276a0f19fb1236e
>
> Diff: https://reviews.apache.org/r/14865/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Neha Narkhede
>
>
