[ https://issues.apache.org/jira/browse/KAFKA-755?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13583829#comment-13583829 ]
Jun Rao commented on KAFKA-755: ------------------------------- Thanks for the patch. Some comments. 1. Broker.createBroker(): We should catch all json parsing related exceptions and throw a KafkaException with the problematic json string in it. 2. ZkUtils.partitionReplicasZkData: should we rename it to replicaAssignmentZkData? 3. TopicCount: Instead of throwing RuntimeException when hitting json errors, it's probably better to throw KafkaException. 4. Utils: 4.1 I recommend rename putStringValuesInJsonObject() to mapToJson(). 4.2 I recommend rename putStringSeqInJsonArray() to arrayToJson() and add a valueAsString flag too. 4.3 recommend rename putIntSeqValuesInJson() to mapWithSeqValuesToJson() 5. ZkUtils: 5.1 registerBrokerInZK(): broker is no longer referenced 5.2 There are 2 getReplicaAssignmentForTopics methods and the following one is not used. Let's remove it. def getReplicaAssignmentForTopics(zkClient: ZkClient, topics: Iterator[String]): 6. ZookeeperConsumerConnector: the line calling mergeJsonObjects is too long 7.ZookeeperConsumerConnectorTest: no real change 8. The following map is not sorted by field names. /consumers/console-consumer-8915/ids/console-consumer-8915_jrao-ld-1361498895428-3268a767 { "subscription":{ "*test" : 1 }, "version":1, "pattern":"white_list" } > standardizing json values stored in ZK > -------------------------------------- > > Key: KAFKA-755 > URL: https://issues.apache.org/jira/browse/KAFKA-755 > Project: Kafka > Issue Type: Bug > Components: core > Affects Versions: 0.8 > Reporter: Jun Rao > Assignee: Swapnil Ghike > Priority: Blocker > Fix For: 0.8 > > Attachments: kafka-755-v1.patch > > > Currently, we have the following paths in ZK that stores non-singleton values. > 1. Topic assignment value: > /brokers/topics/topic > { "0": ["0"] } > 2. LeaderAndISR info: > /brokers/topics/test/partitions/0/leaderAndISR > { "ISR":"0,1","leader":"0","controllerEpoch":"1","leaderEpoch":"0" } > 3. broker registration: > /brokers/ids/0 > 192.168.1.148:9092:9999 > 4. partition reassignment path > It would be good if we do the following: > a. make them true json (e.g., using number as the value for broker/partition, > instead of string). > b. add version support for future growth. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira