----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24196/#review50036 -----------------------------------------------------------
Thanks for the patch. A few comments below. core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala <https://reviews.apache.org/r/24196/#comment87545> "shut down" => "shutdown" core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala <https://reviews.apache.org/r/24196/#comment87542> Could we split that into removeAllConsumerMetrics and removeAllProducerMetrics? In the same JVM, it may be possible to have a producer client and a consumer client with the same clientId. If only a consumer client is closed, we don't want to unregister the metrics for the producer with the same clientId. The only exception is probably for metrics in SyncProducer. Since they exist in both the producer and the consumer, we have to remove those when closing either the producer or the consumer. However, the rest of the metrics can be separated. core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala <https://reviews.apache.org/r/24196/#comment87543> Would it be better to remove those before removing the underlying metrics? core/src/main/scala/kafka/producer/Producer.scala <https://reviews.apache.org/r/24196/#comment87544> "shut down" => "shutdown" - Jun Rao On Aug. 7, 2014, 9:26 p.m., Jiangjie Qin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24196/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2014, 9:26 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1567 > https://issues.apache.org/jira/browse/KAFKA-1567 > > > Repository: kafka > > > Description > ------- > > Followed Jun's proposal to remove the metrics based on regex. > > > Diffs > ----- > > core/src/main/scala/kafka/consumer/ConsumerTopicStats.scala > ff5f470f7aa304917d3295fcb7702291ce7fe0b5 > core/src/main/scala/kafka/consumer/FetchRequestAndResponseStats.scala > 875eeeb73cba5bd034349f5e7b6e16dfdf544254 > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > 65f518d47c7555c42c4bff39c211814831f4b8b6 > core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala > a20ab90165cc7ebb1cf44078efe23a53938c8df6 > core/src/main/scala/kafka/producer/Producer.scala > 4798481d573bbdce0ba39035c50f4c4411ad0469 > core/src/main/scala/kafka/producer/ProducerRequestStats.scala > 96942205a6a461e122e003add1ab9bcebde1fe16 > core/src/main/scala/kafka/producer/ProducerStats.scala > e1610d3c602fb0f5f4cc237cb8b4e0d168a41530 > core/src/main/scala/kafka/producer/ProducerTopicStats.scala > ed209f4773dedb09e9a34005e6849730229aa6e9 > > Diff: https://reviews.apache.org/r/24196/diff/ > > > Testing > ------- > > > Thanks, > > Jiangjie Qin > >
