1.2, 2.1, 3.2, 5.3, 9.2: Renamed *Stat to *Stats and get*Stat to get*Stats. Looks better.
1.3, 2.3, 5.4: Yes, I agree. Changed.
2.2, 2.4, 3.1, 4, 5.1, 6.1, 7.1, 8.1: I was not sure if I should modify a whole bunch of unit tests or provide defaults in constructors. The v2 patch touches a whole bunch of unit tests and some tools.
1.1 Yes, clientId is validated in ZookeeperConsumerConnector which is eventually passed to ConsumerFetcherThread (which extends AbstractFetcherThread). We control the clientId for ReplicaManagerThread (which also extends AbstractFetcherThread) in code, so I guess we can safely remove clientId validation from AbstractFetcherThread.
1.4 I can file another Jira to take a look at this.
5.2 Yes, my bad. To correct this, I had to hack around the Producer constructor. The same *Stats objects need to be passed to DefaultEventHandler and Producer, and they can be constructed only after validating the clientId. Scala does not allow you to put any statement (ClientId.validate(clientId) in our case) before calling a (n-1) level constructor, so you need to put the clientId validation in a block and return a quadruplet from the block which is evaluated as the constructor argument. This constructor acts as an intermediary to separate out the elements of the quadruplet. Suggestions are welcome.
5.5 Moved DroppedMessagePerSec to ProducerStats and deleted AsyncProducerStats.
10 That was because I was trying to modify and rename a file at the same time, and svn has issues cleanly applying such patch. On a second thought, separated TopicTest and ClientIdTest. This patch contains deletion of ConsumerTopicStat.scala and addition of ConsumerTopicStats.scala for the same reason.
Change of plan:
8.2, 8.3, 9.1 Now, each SyncProducer and SimpleConsumer will register its own ProducerRequestStats/ConsumerRequestStats. That removes the need to pass a *Stats object via their constructors. Aggregation of ProducerRequestStats/ConsumerRequestStats at the high level Producer or ZookeeperConsumerConnector level will probably require an aggregator object to be passed to the constructors of SyncProducer/SimpleConsumer. So, I have left that part out.
a. Not quite sure what should go as the clientId in KafkaETLContext.
b. Overloaded the static createSyncProducer API. The two separate APIs remove the need to use Option and also make it easy to pass "FetchMetadata" as the clientId from ClientUtils.
c. Renamed kafka.controller.ControllerStat to ControllerStats and kafka.server.BrokerTopicStat to BrokerTopicStats.