[ https://issues.apache.org/jira/browse/KAFKA-2146?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14539129#comment-14539129 ]
Guozhang Wang commented on KAFKA-2146: -------------------------------------- [~chenshangan...@163.com] Thanks for the patch, and sorry for the late review. I agree with you on #1, but for #2 there is a case when some brokers are temporarily not available, while the existing replica list is set statically. So <code> brokerList.indexOf(existingReplicaList.head) <code> may return -1 in this case, causing a random pick of the starting index. In this case, we should probably pick the next available broker as the starting broker, i.e. if now the available broker list is (1,2,5) and the replica list of partition 0 is 3, we should pick broker-5 as starting broker. Also for #1, I think for better replica distribution we should pick the starting index as the first replica of the LAST partition's list plus one. That is, if we already have three partitions with replica list: 1, 2, 3 2, 3, 4 3, 4, 5 Then if we add another partition, its starting broker should be 4 given if all (1,2,3,4,5) brokers are available; or should be 5 if only (1,2,3,5) are available Another minor comment: not introduced in this patch, but the name of "existingReplicaList" was originally misleading, we could rename it to "existingReplicaListForLastPartition" or "existingReplicaListForPartitionZero" with your current patch. 2. > adding partition did not find the correct startIndex > ----------------------------------------------------- > > Key: KAFKA-2146 > URL: https://issues.apache.org/jira/browse/KAFKA-2146 > Project: Kafka > Issue Type: Bug > Components: admin > Affects Versions: 0.8.2.0 > Reporter: chenshangan > Priority: Minor > Fix For: 0.8.3 > > Attachments: KAFKA-2146.patch > > > TopicCommand provide a tool to add partitions for existing topics. It try to > find the startIndex from existing partitions. There's a minor flaw in this > process, it try to use the first partition fetched from zookeeper as the > start partition, and use the first replica id in this partition as the > startIndex. > One thing, the first partition fetched from zookeeper is not necessary to be > the start partition. As partition id begin from zero, we should use partition > with id zero as the start partition. > The other, broker id does not necessary begin from 0, so the startIndex is > not necessary to be the first replica id in the start partition. > -- This message was sent by Atlassian JIRA (v6.3.4#6332)