You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jay Kreps <bo...@gmail.com> on 2014/05/23 23:08:30 UTC

Review Request 21878: KAFKA-1468: Misc. improvements to test scripts from benchmarking

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21878/
-----------------------------------------------------------

Review request for kafka.


Bugs: KAFKA-1468
    https://issues.apache.org/jira/browse/KAFKA-1468


Repository: kafka


Description
-------

KAFKA-1468 Misc. improvements from benchmarking.


Diffs
-----

  clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3 
  clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java bc4074ec841650eff48dd8aafc1e6c712d8eae7d 
  clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2 
  clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java eb18739d181662e2dee5dd6a23e0181f4c740139 
  clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java a2b77226f8c58caf632a0f4665bd4e4cd93e643d 
  core/src/main/scala/kafka/server/KafkaConfig.scala c7508d5568ec6b94c47d1839aa8695e2dbb76b97 
  core/src/main/scala/kafka/server/RequestPurgatory.scala c064c5c4cf1191335572da8a2caf5f95dce902c1 
  core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 37a9ec27abfd0acd6c01f11d987dd719bc9138a5 

Diff: https://reviews.apache.org/r/21878/diff/


Testing
-------


Thanks,

Jay Kreps


Re: Review Request 21878: KAFKA-1468: Misc. improvements to test scripts from benchmarking

Posted by Jay Kreps <bo...@gmail.com>.

> On May 28, 2014, 1:38 a.m., Jun Rao wrote:
> >

The target throughput is in records/sec. This is a bit poorly documented. My thinking was that we should separate out the system test logic from the old producer performance test and replace it with this one. When we do that we should move the command line options to use jopt simple which will fix the documentation problem and keep us from having two perf tests (which is silly).


> On May 28, 2014, 1:38 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java, lines 33-34
> > <https://reviews.apache.org/r/21878/diff/1/?file=591138#file591138line33>
> >
> >     Is target_throughput in bytes/sec?

No records/sec. I can change it to target_records_per_sec


> On May 28, 2014, 1:38 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java, lines 80-89
> > <https://reviews.apache.org/r/21878/diff/1/?file=591138#file591138line80>
> >
> >     Not sure that I follow how the sleeping logic works. Could you add some comments?

Added comment.


> On May 28, 2014, 1:38 a.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java, lines 207-208
> > <https://reviews.apache.org/r/21878/diff/1/?file=591138#file591138line207>
> >
> >     Should we call this OnCompletionCallback? OnCompeletion.onCompletion() is a bit confusing.

Clarified.


- Jay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21878/#review44087
-----------------------------------------------------------


On May 23, 2014, 9:08 p.m., Jay Kreps wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21878/
> -----------------------------------------------------------
> 
> (Updated May 23, 2014, 9:08 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1468
>     https://issues.apache.org/jira/browse/KAFKA-1468
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1468 Misc. improvements from benchmarking.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java bc4074ec841650eff48dd8aafc1e6c712d8eae7d 
>   clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2 
>   clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java eb18739d181662e2dee5dd6a23e0181f4c740139 
>   clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java a2b77226f8c58caf632a0f4665bd4e4cd93e643d 
>   core/src/main/scala/kafka/server/KafkaConfig.scala c7508d5568ec6b94c47d1839aa8695e2dbb76b97 
>   core/src/main/scala/kafka/server/RequestPurgatory.scala c064c5c4cf1191335572da8a2caf5f95dce902c1 
>   core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 37a9ec27abfd0acd6c01f11d987dd719bc9138a5 
> 
> Diff: https://reviews.apache.org/r/21878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>


Re: Review Request 21878: KAFKA-1468: Misc. improvements to test scripts from benchmarking

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21878/#review44087
-----------------------------------------------------------



clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java
<https://reviews.apache.org/r/21878/#comment78418>

    Is target_throughput in bytes/sec?



clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java
<https://reviews.apache.org/r/21878/#comment78419>

    Not sure that I follow how the sleeping logic works. Could you add some comments?



clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java
<https://reviews.apache.org/r/21878/#comment78416>

    Should we call this OnCompletionCallback? OnCompeletion.onCompletion() is a bit confusing.



clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java
<https://reviews.apache.org/r/21878/#comment78417>

    Should we handle the error case here?


- Jun Rao


On May 23, 2014, 9:08 p.m., Jay Kreps wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21878/
> -----------------------------------------------------------
> 
> (Updated May 23, 2014, 9:08 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1468
>     https://issues.apache.org/jira/browse/KAFKA-1468
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1468 Misc. improvements from benchmarking.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java bc4074ec841650eff48dd8aafc1e6c712d8eae7d 
>   clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2 
>   clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java eb18739d181662e2dee5dd6a23e0181f4c740139 
>   clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java a2b77226f8c58caf632a0f4665bd4e4cd93e643d 
>   core/src/main/scala/kafka/server/KafkaConfig.scala c7508d5568ec6b94c47d1839aa8695e2dbb76b97 
>   core/src/main/scala/kafka/server/RequestPurgatory.scala c064c5c4cf1191335572da8a2caf5f95dce902c1 
>   core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 37a9ec27abfd0acd6c01f11d987dd719bc9138a5 
> 
> Diff: https://reviews.apache.org/r/21878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>


Re: Review Request 21878: Patch for KAFKA-1468

Posted by Jay Kreps <bo...@gmail.com>.

> On May 29, 2014, 5:20 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/server/RequestPurgatory.scala, line 64
> > <https://reviews.apache.org/r/21878/diff/2/?file=598165#file598165line64>
> >
> >     Why we want to reduce the default purgatory size?

This interval is in terms of requests. Consider a case where you have 1000 partitions on your broker and set your fetch timeout to be 10ms (either for consumers or for replication), then in a period of inactivity you will have 10,000,000 obsolete entries in purgatory and purge them all at once. During this time no other purgatory expiration happens which leads to large latency spikes. 


> On May 29, 2014, 5:20 p.m., Guozhang Wang wrote:
> > core/src/main/scala/kafka/tools/TestEndToEndLatency.scala, line 25
> > <https://reviews.apache.org/r/21878/diff/2/?file=598166#file598166line25>
> >
> >     Can we add some comments here indicating the end-to-end latency is for async producer mode?

Well I'm not sure how relevant the acknowledgement mode is as the consumer can't consume until full acknowledgement. Since the consumer fetch and the producer ack happen in parallel I would expect the difference to be negligible.


- Jay


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21878/#review44266
-----------------------------------------------------------


On May 28, 2014, 11:15 p.m., Jay Kreps wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21878/
> -----------------------------------------------------------
> 
> (Updated May 28, 2014, 11:15 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1468
>     https://issues.apache.org/jira/browse/KAFKA-1468
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1468 Misc. improvements from benchmarking.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java bc4074ec841650eff48dd8aafc1e6c712d8eae7d 
>   clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2 
>   clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java eb18739d181662e2dee5dd6a23e0181f4c740139 
>   clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java a2b77226f8c58caf632a0f4665bd4e4cd93e643d 
>   core/src/main/scala/kafka/server/KafkaConfig.scala c7508d5568ec6b94c47d1839aa8695e2dbb76b97 
>   core/src/main/scala/kafka/server/RequestPurgatory.scala c064c5c4cf1191335572da8a2caf5f95dce902c1 
>   core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 37a9ec27abfd0acd6c01f11d987dd719bc9138a5 
> 
> Diff: https://reviews.apache.org/r/21878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>


Re: Review Request 21878: Patch for KAFKA-1468

Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21878/#review44266
-----------------------------------------------------------



core/src/main/scala/kafka/server/RequestPurgatory.scala
<https://reviews.apache.org/r/21878/#comment78613>

    Why we want to reduce the default purgatory size?



core/src/main/scala/kafka/tools/TestEndToEndLatency.scala
<https://reviews.apache.org/r/21878/#comment78614>

    Can we add some comments here indicating the end-to-end latency is for async producer mode?


- Guozhang Wang


On May 28, 2014, 11:15 p.m., Jay Kreps wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21878/
> -----------------------------------------------------------
> 
> (Updated May 28, 2014, 11:15 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1468
>     https://issues.apache.org/jira/browse/KAFKA-1468
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1468 Misc. improvements from benchmarking.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java bc4074ec841650eff48dd8aafc1e6c712d8eae7d 
>   clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2 
>   clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java eb18739d181662e2dee5dd6a23e0181f4c740139 
>   clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java a2b77226f8c58caf632a0f4665bd4e4cd93e643d 
>   core/src/main/scala/kafka/server/KafkaConfig.scala c7508d5568ec6b94c47d1839aa8695e2dbb76b97 
>   core/src/main/scala/kafka/server/RequestPurgatory.scala c064c5c4cf1191335572da8a2caf5f95dce902c1 
>   core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 37a9ec27abfd0acd6c01f11d987dd719bc9138a5 
> 
> Diff: https://reviews.apache.org/r/21878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>


Re: Review Request 21878: Patch for KAFKA-1468

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21878/#review44261
-----------------------------------------------------------

Ship it!



clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java
<https://reviews.apache.org/r/21878/#comment78606>

    Perhaps we can rename target_throughput to target_records_per_sec?


- Jun Rao


On May 28, 2014, 11:15 p.m., Jay Kreps wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21878/
> -----------------------------------------------------------
> 
> (Updated May 28, 2014, 11:15 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1468
>     https://issues.apache.org/jira/browse/KAFKA-1468
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1468 Misc. improvements from benchmarking.
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java bc4074ec841650eff48dd8aafc1e6c712d8eae7d 
>   clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2 
>   clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java eb18739d181662e2dee5dd6a23e0181f4c740139 
>   clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java a2b77226f8c58caf632a0f4665bd4e4cd93e643d 
>   core/src/main/scala/kafka/server/KafkaConfig.scala c7508d5568ec6b94c47d1839aa8695e2dbb76b97 
>   core/src/main/scala/kafka/server/RequestPurgatory.scala c064c5c4cf1191335572da8a2caf5f95dce902c1 
>   core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 37a9ec27abfd0acd6c01f11d987dd719bc9138a5 
> 
> Diff: https://reviews.apache.org/r/21878/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jay Kreps
> 
>


Re: Review Request 21878: Patch for KAFKA-1468

Posted by Jay Kreps <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21878/
-----------------------------------------------------------

(Updated May 28, 2014, 11:15 p.m.)


Review request for kafka.


Summary (updated)
-----------------

Patch for KAFKA-1468


Bugs: KAFKA-1468
    https://issues.apache.org/jira/browse/KAFKA-1468


Repository: kafka


Description
-------

KAFKA-1468 Misc. improvements from benchmarking.


Diffs (updated)
-----

  clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java f1def508153b7361b8720dbcc939e06b0e7e45d3 
  clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java bc4074ec841650eff48dd8aafc1e6c712d8eae7d 
  clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java 3e83ae0fefb7578cccbd8dc0cd9b28f22e0951b2 
  clients/src/main/java/org/apache/kafka/clients/tools/ProducerPerformance.java eb18739d181662e2dee5dd6a23e0181f4c740139 
  clients/src/test/java/org/apache/kafka/clients/producer/SenderTest.java a2b77226f8c58caf632a0f4665bd4e4cd93e643d 
  core/src/main/scala/kafka/server/KafkaConfig.scala c7508d5568ec6b94c47d1839aa8695e2dbb76b97 
  core/src/main/scala/kafka/server/RequestPurgatory.scala c064c5c4cf1191335572da8a2caf5f95dce902c1 
  core/src/main/scala/kafka/tools/TestEndToEndLatency.scala 37a9ec27abfd0acd6c01f11d987dd719bc9138a5 

Diff: https://reviews.apache.org/r/21878/diff/


Testing
-------


Thanks,

Jay Kreps