> 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 > > request : requests){} since it seems that we don't use the for loop index > > elsewhere other than getting the request from the list? > > Jay Kreps wrote: > I try to avoid that style of for loop for lists as it ends up creating an > iterator object. Maybe that is crazy?
Ah I see. Yes, that makes sense. We should just continue following it as a coding pattern. > On March 20, 2014, 6:02 p.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java, > > line 424 > > <https://reviews.apache.org/r/19388/diff/4/?file=528767#file528767line424> > > > > Is it required to pass in 'now'? Could it just be computed inside > > handleResponses just like 'ns'? > > Jay Kreps wrote: > It's not required. The goal was (1) to make the full execution of a > select loop occur with a single unique timestamp, and (2) avoid checking the > clock 12 times since it is expensive. Gotcha > On March 20, 2014, 6:02 p.m., Neha Narkhede wrote: > > clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java, > > line 114 > > <https://reviews.apache.org/r/19388/diff/4/?file=528765#file528765line114> > > > > maybe I'm missing something but shouldn't this be one of the Sender > > metrics and get updated in run() after this - > > > > // get the list of partitions with data ready to send > > List<TopicPartition> ready = this.accumulator.ready(now); > > > > Jay Kreps wrote: > It can work either way. That style just directly exposes that method as a > metric. We could alternately record each check and take the running average > as you propose. Ya, I guess it's fine. I think I found the running average easier to understand at first :-) - Neha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19388/#review37843 ----------------------------------------------------------- On March 20, 2014, 12:30 a.m., Jay Kreps wrote: > > ----------------------------------------------------------- > 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: 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/internals/Metadata.java > 33d62a4b83fbab5b22b91b22f6b744af1c98d262 > > 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/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/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/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 > >
