----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26666/#review56958 -----------------------------------------------------------
Since you fixed some other tools as well, can we also fix the preferred replica election command where we can de-dup the partitions? core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala <https://reviews.apache.org/r/26666/#comment97400> I think it's worth telling the user which partition's replicas contain duplicates (and include all such partitions instead of one) since typically partition reassignment operation can contain 100s of partitions. - Neha Narkhede On Oct. 13, 2014, 11:57 p.m., Ewen Cheslack-Postava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26666/ > ----------------------------------------------------------- > > (Updated Oct. 13, 2014, 11:57 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1653 > https://issues.apache.org/jira/browse/KAFKA-1653 > > > Repository: kafka > > > Description > ------- > > KAFKA-1653 Disallow duplicate broker IDs in user input for admin commands. > This covers a few cases besides the one identified in the bug. Aside from a > major refactoring to use Sets for broker/replica lists, sanitizing user input > seems to be the best solution here. I chose to generate errors instead of > just using toSet since a duplicate entry may indicate that a different broker > id was accidentally omitted. > > > Diffs > ----- > > core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala > 691d69a49a240f38883d2025afaec26fd61281b5 > core/src/main/scala/kafka/admin/TopicCommand.scala > 7672c5aab4fba8c23b1bb5cd4785c332d300a3fa > core/src/main/scala/kafka/tools/StateChangeLogMerger.scala > d298e7e81acc7427c6cf4796b445966267ca54eb > > Diff: https://reviews.apache.org/r/26666/diff/ > > > Testing > ------- > > > Thanks, > > Ewen Cheslack-Postava > >