Re: Review Request 30570: Patch for KAFKA-1914

2015-02-18 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/#review73020 --- Thanks for the patch. A couple of comments. 1. We already measure

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-18 Thread Joel Koshy
On Feb. 18, 2015, 10:16 p.m., Jun Rao wrote: Thanks for the patch. A couple of comments. 1. We already measure the total produce/fetch request rate in RequestMetrics. Now, we are duplicating that in BrokerTopics. Is there a way to avoid the duplication? 2. The following unit test

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-17 Thread Aditya Auradkar
On Feb. 17, 2015, 11:02 p.m., Guozhang Wang wrote: core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala, line 137 https://reviews.apache.org/r/30570/diff/1/?file=846133#file846133line137 Should this be BrokerTopicStats.getBrokerAllTopicsStats()? Good catch. Fixed - Aditya

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-17 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/ --- (Updated Feb. 17, 2015, 11:46 p.m.) Review request for kafka and Joel Koshy.

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-17 Thread Aditya Auradkar
On Feb. 18, 2015, 12:41 a.m., Joel Koshy wrote: core/src/main/scala/kafka/server/KafkaRequestHandler.scala, line 108 https://reviews.apache.org/r/30570/diff/2/?file=866690#file866690line108 I think the aggregate rates here are redundant to what's already there in RequestChannel's

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-17 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/#review72845 --- Ship it!

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-17 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/#review72817 --- core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-17 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/#review72836 --- Ship it! Ship It! - Guozhang Wang On Feb. 17, 2015, 11:46 p.m.,

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-09 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/#review71646 --- core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-09 Thread Aditya Auradkar
On Feb. 9, 2015, 5:47 p.m., Guozhang Wang wrote: core/src/main/scala/kafka/server/ReplicaManager.scala, lines 296-303 https://reviews.apache.org/r/30570/diff/1/?file=846132#file846132line296 appendToLocalLog can be called for both produce and commit-offset requests. And in

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-09 Thread Aditya Auradkar
On Feb. 9, 2015, 5:47 p.m., Guozhang Wang wrote: core/src/main/scala/kafka/server/ReplicaManager.scala, lines 296-303 https://reviews.apache.org/r/30570/diff/1/?file=846132#file846132line296 appendToLocalLog can be called for both produce and commit-offset requests. And in

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-09 Thread Aditya Auradkar
On Feb. 9, 2015, 6:29 p.m., Joel Koshy wrote: core/src/main/scala/kafka/server/ReplicaManager.scala, lines 296-303 https://reviews.apache.org/r/30570/diff/1/?file=846132#file846132line296 I agree with Guozhang that the offset commits need to be disambiguated. How about

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-09 Thread Joel Koshy
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/#review71660 --- core/src/main/scala/kafka/server/ReplicaManager.scala

Re: Review Request 30570: Patch for KAFKA-1914

2015-02-03 Thread Aditya Auradkar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30570/ --- (Updated Feb. 3, 2015, 6:50 p.m.) Review request for kafka. Bugs: KAFKA-1914