> On Oct. 29, 2013, 9:12 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/cluster/Partition.scala, line 163 > > <https://reviews.apache.org/r/14865/diff/2/?file=373421#file373421line163> > > > > Also add "// add replicas that are new"
Done > On Oct. 29, 2013, 9:12 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/cluster/Partition.scala, line 166 > > <https://reviews.apache.org/r/14865/diff/2/?file=373421#file373421line166> > > > > Can we do this removal in one line? Done > On Oct. 29, 2013, 9:12 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, line 366 > > <https://reviews.apache.org/r/14865/diff/2/?file=373422#file373422line366> > > > > I think if controller failover at step 3, the new controller will start > > again from 1, and the AR it reads from ZK has already be AR + RAR, and > > hence step 1/2 will be skipped, is that right? If the controller fails over at step 3, the new controller will start by checking if the replicas are in ISR. If yes, it will skip 1 & 2. If not, then it will repeat 1 & 2 (which are idempotent). Either way, it will still complete the reassignment successfully. > On Oct. 29, 2013, 9:12 p.m., Guozhang Wang wrote: > > core/src/main/scala/kafka/controller/KafkaController.scala, line 379 > > <https://reviews.apache.org/r/14865/diff/2/?file=373422#file373422line379> > > > > Will the LeaderAndISR request be rejected by the current leader due to > > its leader epoch? Yes, if there has been no leader or isr change, it will be rightly ignored. The places where we want to force a state change is where we need to communicate the expanded and shrunk assigned replica list to the leader. That is when we do a dummy leader epoch increment write to the partition's state path. - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14865/#review27724 ----------------------------------------------------------- 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 > >