----------------------------------------------------------- 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 per node sensor "node--1" gets registered (in addition to the correct one "node-0"), possibly since the node id somehow gets passed in as -1. Other than the issue above, I have minor review comments. clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java <https://reviews.apache.org/r/19388/#comment70843> ellapsed->elapsed ;-) clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java <https://reviews.apache.org/r/19388/#comment70870> javadoc comments are obsolete now that we got rid of the key value pair clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java <https://reviews.apache.org/r/19388/#comment70871> I couldn't think of a better name for this either. Should we reintroduce this getFoo() naming scheme in our coding style? clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java <https://reviews.apache.org/r/19388/#comment70873> Could we please fix the typo here as well? clients/src/main/java/org/apache/kafka/common/network/Selector.java <https://reviews.apache.org/r/19388/#comment70875> NetworkSend has a size() API but NetworkReceive doesn't. Can we add it so this would've simplified to transmissions.receive.size() similar to transmissions.send.size()? clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java <https://reviews.apache.org/r/19388/#comment70876> can we please remove this unused import? clients/src/test/java/org/apache/kafka/test/MetricsBench.java <https://reviews.apache.org/r/19388/#comment70872> Can we delete the commented out code? We can always get it from git if we need it in the future. - Neha Narkhede On March 26, 2014, 12:07 a.m., Jay Kreps wrote: > > ----------------------------------------------------------- > 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: KAFKA-1251 > https://issues.apache.org/jira/browse/KAFKA-1251 > > > Repository: kafka > > > Description > ------- > > KAFKA-1251: Add metrics to the producer. > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 1ac69436f117800815b8d50f042e9e2a29364b43 > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > 32e12ad149f6d70c96a498d0a390976f77bf9e2a > > clients/src/main/java/org/apache/kafka/clients/producer/internals/FutureRecordMetadata.java > aec31c381658f85ab6394a6475b4989177a31a3d > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java > 673b2962771c28ceb3c7a6c0fd6f69521bd7ed16 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordBatch.java > 038a05a94b795ec0a95b2d40a89222394b5a74c4 > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java > 565331dfb9cd1d65be37ed97830aa42e44d2e127 > > clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java > 3ebbb804242be6a001b3bae6524afccc85a87602 > clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java > 6db2dfbe94c940efa37463298f0b0b1893e646e1 > clients/src/main/java/org/apache/kafka/common/metrics/Sensor.java > 7e4849b7a148009c8a878349d7f0239108ccad8c > > clients/src/main/java/org/apache/kafka/common/metrics/stats/Percentiles.java > 4d549167ea7c1390ecf1440e40332375f247088f > clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java > 3b0454f26490d1f4a2a80efb00165fc72587fbf8 > > clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java > f8b413a8c273cdad56177fbc6971fece4feb86b3 > clients/src/main/java/org/apache/kafka/common/network/ByteBufferSend.java > 9305b61ddeaa2bb400cbbb6d3c99c8ecaade6b8f > clients/src/main/java/org/apache/kafka/common/network/NetworkReceive.java > 51d4892dfc18580e5e213d386c5de387a47d3c6b > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 983963200ce81614577cd6182a5d2f10c22b95d4 > clients/src/main/java/org/apache/kafka/common/utils/CopyOnWriteMap.java > 187d22fba5fe225546738fbf43b25fa9e5b4f334 > clients/src/test/java/org/apache/kafka/clients/producer/MetadataTest.java > 09a5355d25a3b94c8e23caa2adc77cb1c59368b9 > > clients/src/test/java/org/apache/kafka/clients/producer/RecordAccumulatorTest.java > ed5690641a22fbe4bd91b0c6055d465944b08c06 > clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java > 12c9500ce4387306ab5aa7a5781b4aca52b86604 > clients/src/test/java/org/apache/kafka/common/metrics/MetricsTest.java > fdd89141579b6ab2bfaf2b1588440909ae0a7bfd > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java > 90e2dcf5434db546387302fb0219edfdb363592e > clients/src/test/java/org/apache/kafka/test/MetricsBench.java > 7239b4a56e93f019e66aa2cf2aa9b04c26908bfd > > Diff: https://reviews.apache.org/r/19388/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
