[ https://issues.apache.org/jira/browse/KAFKA-237?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13185158#comment-13185158 ]
Neha Narkhede commented on KAFKA-237: ------------------------------------- Took a look at the patch. Good to see work started on KAFKA-50. I have a couple of comments - 1. It will be good to use a logger in all the tools. This is mainly to be able to cleanly log stack traces. For example, println("delection failed because of " + e) does not print out the stack trace as desired. 2. In the create topic tool, there is a getBrokerList API. This seems like a pretty useful utility API. Would it make sense to expose it in ZkUtils instead of rewriting it there ? 3. In the create topic tool, can the broker list returned by getBrokerList be a Set ? It is expected to be a Set in getManualReplicaAssignment, and also expected to be one. 4. In the create topic tool, replica-assignment-list format is very pretty to read. Can this be described in more detail ? Also, usually entries are described in csv format. Seems like it is easier to understand if the list of entries per partition are comma separated, and the replica ids within a partitions list are ":" separated. 5. In createReplicaAssignmentPathInZK API, if a topic already exists, it will be good to print its version (timestamp) as part of the exception. 6. Can getTopicPartitionsPartPath API be renamed so that it is easier to understand what it is meant for ? It seems like it returns the replication information for a particular partition id. 7. In AdminUtils.assginReplicasToBrokers(), why is secondReplicaShift required ? Can the next replica index be selected like this - (firstReplicaIndex + j + 1) % num_brokers. Or maybe I'm missing something ? 8. There are a couple of typos in the create topic admin tool > create/delete ZK path for a topic in an admin tool > -------------------------------------------------- > > Key: KAFKA-237 > URL: https://issues.apache.org/jira/browse/KAFKA-237 > Project: Kafka > Issue Type: Sub-task > Components: core > Affects Versions: 0.8 > Reporter: Jun Rao > Assignee: Jun Rao > Attachments: kafka-237.patch > > > This subtask will implement a create/delete admin tool that create/delete the > ZK paths needed for a new topic. The needed ZK paths are described in the > design doc in KAFKA-50 and the discussion in KAFKA-47. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira