Activity

Also
c. Think through how any generalization would work (e.g. (1) can't really be extended beyond partitions because the partitions are at the top level of the object.
d. Standardize capitalization: we have dashes, underscores, camel case and pretty much everything else
e: Document them and going forward add to the documentation. This will ensure future additions are thought through holistically with all the other structures.

Jay Kreps
added a comment - 11/Feb/13 20:02 Also
c. Think through how any generalization would work (e.g. (1) can't really be extended beyond partitions because the partitions are at the top level of the object.
d. Standardize capitalization: we have dashes, underscores, camel case and pretty much everything else
e: Document them and going forward add to the documentation. This will ensure future additions are thought through holistically with all the other structures.

This patch standardizes the reading and writing to zookeeper values according to the JSON schemas 1 thru 8 defined in the aforementioned wiki.

The changes in this patch involve a bunch of refactoring to make this standardization possible.

This patch does not touch any other use of JSON-like structures in the code that does not involve interacting with zookeeper.

Testing done:

Unit tests pass.

Nuke all the zookeeper data. Start kafka server, produce data using a producer and start a consumer. Observe the zookeeper data.

It would be good if we could check in this 1st patch and let the system test run over it. I will create another patch to make the changes for item 9 and 10 in the wiki as and when my hands are free of other blockers. 9 and 10 will be used only by our tools and are standalone changes in the sense that we can simply delete and recreate the related zookeeper namespaces without disturbing our clients.

Swapnil Ghike
added a comment - 21/Feb/13 07:52 This patch standardizes the reading and writing to zookeeper values according to the JSON schemas 1 thru 8 defined in the aforementioned wiki.
The changes in this patch involve a bunch of refactoring to make this standardization possible.
This patch does not touch any other use of JSON-like structures in the code that does not involve interacting with zookeeper.
Testing done:
Unit tests pass.
Nuke all the zookeeper data. Start kafka server, produce data using a producer and start a consumer. Observe the zookeeper data.
It would be good if we could check in this 1st patch and let the system test run over it. I will create another patch to make the changes for item 9 and 10 in the wiki as and when my hands are free of other blockers. 9 and 10 will be used only by our tools and are standalone changes in the sense that we can simply delete and recreate the related zookeeper namespaces without disturbing our clients.

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":

i. Renamed valueAsString flag to valueInQuotes for clarity.
ii. Json fields should appear in sorted order now.
iii. Changed the whitelist/blacklist detection to use the "pattern" field in the zk json. Accordingly modified the createTopicCount and dbString methods.

Testing done -
1. Testing done for v1 patch.
2. Tried all of non-wildcard, whitelist and blacklist topics with console consumer. Zk data looks ok (it does not include * for non-wildcard topics). Also changed the wiki to reflect this, previously it had entries like "*abc" and "!abc" for wildcard consumer subscription fields.

Swapnil Ghike
added a comment - 22/Feb/13 07:12 Addressed all points.
i. Renamed valueAsString flag to valueInQuotes for clarity.
ii. Json fields should appear in sorted order now.
iii. Changed the whitelist/blacklist detection to use the "pattern" field in the zk json. Accordingly modified the createTopicCount and dbString methods.
Testing done -
1. Testing done for v1 patch.
2. Tried all of non-wildcard, whitelist and blacklist topics with console consumer. Zk data looks ok (it does not include * for non-wildcard topics). Also changed the wiki to reflect this, previously it had entries like "*abc" and "!abc" for wildcard consumer subscription fields.

Jun Rao
added a comment - 22/Feb/13 15:59 Thanks for patch v2. A few more comments.
20. Broker: Can we add brokerInfoString in the following exception?
throw new KafkaException("Failed to parse the broker info from zookeeper", t)
21. TopicCount:
21.1 Can we define "white_list" and "black_list" as constants and reference only the constants instead of the strings?
21.2 Can we rename specialList to subscriptionPattern?
22. Utils: The comment for mapWithSeqValuesToJson() needs to be changed.
Could you rebase too?

Swapnil Ghike
added a comment - 23/Feb/13 00:09 Made the changes and rebased.
writePreferredReplicaElectionData is a bit clunky to read currently, I will take care of it while completing the remaining changes for items 9 adn 10.