Re: Review Request 19388: Patch for KAFKA-1251

2014-03-27 Thread Guozhang Wang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/#review38748 --- Could you rebase?

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-26 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/#review38570 --- I ran ProducerPerformance on a local broker and saw one problem. A

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-25 Thread Jay Kreps
On March 23, 2014, 11:57 p.m., Jun Rao wrote: Thanks for the patch. Some comments. 1. For producer level stats (ie., not node or topic level), for jmx beans, the package name becomes kafka.producer (since the bean name is kafka.producer:type=client-id). For the topic and node level

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-25 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/ --- (Updated March 26, 2014, 12:07 a.m.) Review request for kafka. Bugs:

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-24 Thread Jun Rao
On March 23, 2014, 11:57 p.m., Jun Rao wrote: Thanks for the patch. Some comments. 1. For producer level stats (ie., not node or topic level), for jmx beans, the package name becomes kafka.producer (since the bean name is kafka.producer:type=client-id). For the topic and node level

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-24 Thread Jay Kreps
On March 23, 2014, 11:57 p.m., Jun Rao wrote: Thanks for the patch. Some comments. 1. For producer level stats (ie., not node or topic level), for jmx beans, the package name becomes kafka.producer (since the bean name is kafka.producer:type=client-id). For the topic and node level

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-24 Thread Jun Rao
On March 23, 2014, 11:57 p.m., Jun Rao wrote: Thanks for the patch. Some comments. 1. For producer level stats (ie., not node or topic level), for jmx beans, the package name becomes kafka.producer (since the bean name is kafka.producer:type=client-id). For the topic and node level

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-23 Thread Jun Rao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/#review38261 --- Thanks for the patch. Some comments. 1. For producer level stats

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-23 Thread Jay Kreps
On March 23, 2014, 11:57 p.m., Jun Rao wrote: Thanks for the patch. Some comments. 1. For producer level stats (ie., not node or topic level), for jmx beans, the package name becomes kafka.producer (since the bean name is kafka.producer:type=client-id). For the topic and node level

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-21 Thread Jay Kreps
On March 20, 2014, 6:14 p.m., Neha Narkhede wrote: clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java, line 137 https://reviews.apache.org/r/19388/diff/4/?file=528769#file528769line137 In this patch, we introduced a getFoo() usage pattern. I thought we wanted to

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-21 Thread Neha Narkhede
On March 20, 2014, 6:02 p.m., Neha Narkhede wrote: clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, line 814 https://reviews.apache.org/r/19388/diff/4/?file=528767#file528767line814 Can we collapse these 2 statements into simply for(InFlightRequest

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-20 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/#review37843 ---

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-20 Thread Neha Narkhede
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/#review37926 ---

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-20 Thread Jay Kreps
On March 20, 2014, 10:08 p.m., Guozhang Wang wrote: clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java, line 65 https://reviews.apache.org/r/19388/diff/4/?file=528766#file528766line65 sizeInBytes does not work for compression, we could just use

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-20 Thread Guozhang Wang
On March 20, 2014, 10:08 p.m., Guozhang Wang wrote: clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java, line 65 https://reviews.apache.org/r/19388/diff/4/?file=528766#file528766line65 sizeInBytes does not work for compression, we could just use

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-19 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/ --- (Updated March 19, 2014, 5:19 p.m.) Review request for kafka. Summary

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-19 Thread Jay Kreps
On March 19, 2014, 6:49 a.m., Timothy Chen wrote: clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java, line 48 https://reviews.apache.org/r/19388/diff/1/?file=526931#file526931line48 Not sure if this is intentional. Hey that was going to be my legacy

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-19 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/ --- (Updated March 19, 2014, 5:29 p.m.) Review request for kafka. Bugs:

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-19 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/#review37791 --- Ship it! Ship It! - Timothy Chen On March 19, 2014, 5:29 p.m.,

Re: Review Request 19388: Patch for KAFKA-1251

2014-03-19 Thread Jay Kreps
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/ --- (Updated March 20, 2014, 12:30 a.m.) Review request for kafka. Bugs: