----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16360/#review32242 -----------------------------------------------------------
core/src/main/scala/kafka/cluster/Partition.scala <https://reviews.apache.org/r/16360/#comment60970> Would be better to move this comment to above the method and say what it returns and what it means there. core/src/main/scala/kafka/controller/ReplicaStateMachine.scala <https://reviews.apache.org/r/16360/#comment60972> Clearer to say: "to all the remaining (alive) replicas" of the partition. core/src/main/scala/kafka/server/ReplicaManager.scala <https://reviews.apache.org/r/16360/#comment60981> I'm wondering if it would make the code cleaner/clearer if you move all the code below that deals with partitionsToMakeFollower to this block of the if statement. core/src/main/scala/kafka/server/ReplicaManager.scala <https://reviews.apache.org/r/16360/#comment60982> "aborted the become-follower state change"... Also, I'm not sure this shutting down check is doing what we really want. i.e., the replica fetcher manager may be shutdown right after this, and we could still add fetchers. But that's a separate issue, and I may have misread the code. - Joel Koshy On Jan. 16, 2014, 11:02 p.m., Guozhang Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16360/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2014, 11:02 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1188 > https://issues.apache.org/jira/browse/KAFKA-1188 > > > Repository: kafka > > > Description > ------- > > KAFKA-1188.v6 > > > KAFKA-1188.v5.1 > > > KAFKA-1188.v5 > > > Dummy > > > KAFKA-1188.v4 > > > KAFKA-1188.v3 > > > KAFKA-1118.v2 > > > KAFKA-1188.v1 > > > Diffs > ----- > > core/src/main/scala/kafka/cluster/Partition.scala > 1087a2e91c86e36a2494a95913a3ec2daf238287 > core/src/main/scala/kafka/controller/ReplicaStateMachine.scala > 483559aa64726c51320d18b64a1b48f8fb2905a0 > core/src/main/scala/kafka/server/ReplicaManager.scala > f9d10d385cee49a1e3be8c82e3ffa22ef87a8fd6 > > Diff: https://reviews.apache.org/r/16360/diff/ > > > Testing > ------- > > > Thanks, > > Guozhang Wang > >
