> On Oct. 30, 2013, 5:12 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, lines 397-398 > > <https://reviews.apache.org/r/14865/diff/2/?file=373422#file373422line397> > > > > Should we name this addedReplicas? To me, new and old replicas mean the > > set of replicas after and before the reassignment.
But added vs old doesn't make much sense. We need a clear way to reference replicas-before-assignment and replicas-after-assignment. So, here, new and old replicas does mean the set of replicas after and before the reassignment. > On Oct. 30, 2013, 5:12 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, lines 401-405 > > <https://reviews.apache.org/r/14865/diff/2/?file=373422#file373422line401> > > > > 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? > > Yes, we need to do that for performing the state transition. And anyways, it will ignore it if the leader epoch is not > the previous leader epoch > On Oct. 30, 2013, 5:12 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, lines 874-875 > > <https://reviews.apache.org/r/14865/diff/2/?file=373422#file373422line874> > > > > Should we throw an exception here? Makes sense. Included that change. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14865/#review27787 ----------------------------------------------------------- On Oct. 31, 2013, 4:46 a.m., Neha Narkhede wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14865/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2013, 4:46 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1097 > https://issues.apache.org/jira/browse/KAFKA-1097 > > > Repository: kafka > > > Description > ------- > > 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 > >
