[ 
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

Reply via email to