KAFKA-4432: Added support to supply custom message payloads to perf-producer script. by SandeshKarkera · Pull Request #2158 · apache/kafka

SandeshKarkera · 2016-11-22T15:58:35Z

Current implementation of ProducerPerformance creates static payload. This is not very useful in testing compression or when you want to test with production/custom payloads. So, we decided to add support for providing payload file as an input to producer perf test script.

We made the following changes:

Added support to provide a payload file which can have the list of payloads that you actually want to send.
Moved payload generation inside the send loop for cases when payload file is provided.

Following are the changes to how the producer-performance is evoked:

You must provide "--record-size" or "--payload-file" but not both. This is because, record size cannot be guaranteed when you are using custom events.
e.g. ./kafka-producer-perf-test.sh --topic test_topic --num-records 100000 --producer-props bootstrap.servers=127.0.0.1:9092 acks=0 buffer.memory=33554432 compression.type=gzip batch.size=10240 linger.ms=10 --throughput -1 --payload-file ./test_payloads --payload-delimiter ,
Earlier "--record-size" was a required config, now you must provide exactly one of "--record-size" or "--payload-file". Providing both will result in an error.
Support for an additional parameter "--payload-delimiter" has been added which defaults to "\n"

SandeshKarkera · 2016-11-23T07:27:15Z

@ijuma can you please review?

SandeshKarkera · 2016-11-28T11:00:17Z

@guozhangwang @dguy @mjsax @norwood @enothereska can you please take a look?

norwood · 2016-11-28T18:12:47Z

                throw new ArgumentParserException("Either --producer-props or --producer.config must be specified.", parser);
            }

+            if ((recordSize == null && payloadFilePath == null) || (recordSize != null && payloadFilePath != null)) {


you can validate this in the ArgumentParser by using parser.addMutuallyExclusiveGroup() and adding --record-size and --payload-file to the returned group and making the group .required()

…eGroup

SandeshKarkera · 2016-11-29T06:48:51Z

@norwood Thanks for the review. I've removed the validation for record-size and payload-file and added them to a MutuallyExclusiveGroup as suggested.

SandeshKarkera · 2016-11-29T09:58:07Z

Jenkins build job is failing due to time out issue.

SandeshKarkera · 2016-12-01T04:36:59Z

@norwood, can you please review and let me know if this can be merged?

norwood · 2016-12-01T18:02:42Z

+                    throw new  IllegalArgumentException("File does not exist or empty file provided.");
+                }
+
+                String[] payloadList = new String(Files.readAllBytes(path), "UTF-8").split(payloadDelimiter);


this seems like it could be dangerous depending on how large your input file is. it would probably be best to use BufferedReader, but if you have to support custom delimiters this could get complicated.

@norwood We have to store all the messages in memory, otherwise, picking a new random message to send will be slow and complicated.
Given that all messages are stored in memory, I think its OK to use Files.readAllBytes()

@norwood Do let me know your thoughts on the point mentioned above.

sorry. yeah, this makes sense. thx.

you will still need to get a commiter to 👍 this, but lgtm

SandeshKarkera · 2016-12-13T06:14:53Z

@junrao @ijuma @guozhangwang Can you please review and merge?

asfbot · 2016-12-13T06:54:58Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/92/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2016-12-13T06:55:48Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/93/
Test PASSed (JDK 8 and Scala 2.11).

asfbot · 2016-12-13T07:03:04Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/91/
Test PASSed (JDK 7 and Scala 2.10).

SandeshKarkera · 2017-01-04T03:03:13Z

@junrao @jkreps @guozhangwang @ijuma @mumrah @sriramsub @d3fmacro : Guys, this PR has been open for a long time. It has been reviewed by @norwood . I'd really appreciate if one of you can take a look and merge this.

junrao · 2017-01-20T01:45:30Z

@SandeshKarkera : Thanks for the patch and sorry for the delay. LGTM

…roducer script. Current implementation of ProducerPerformance creates static payload. This is not very useful in testing compression or when you want to test with production/custom payloads. So, we decided to add support for providing payload file as an input to producer perf test script. We made the following changes: 1. Added support to provide a payload file which can have the list of payloads that you actually want to send. 2. Moved payload generation inside the send loop for cases when payload file is provided. Following are the changes to how the producer-performance is evoked: 1. You must provide "--record-size" or "--payload-file" but not both. This is because, record size cannot be guaranteed when you are using custom events. e.g. ./kafka-producer-perf-test.sh --topic test_topic --num-records 100000 --producer-props bootstrap.servers=127.0.0.1:9092 acks=0 buffer.memory=33554432 compression.type=gzip batch.size=10240 linger.ms=10 --throughput -1 --payload-file ./test_payloads --payload-delimiter , 2. Earlier "--record-size" was a required config, now you must provide exactly one of "--record-size" or "--payload-file". Providing both will result in an error. 3. Support for an additional parameter "--payload-delimiter" has been added which defaults to "\n" Author: Sandesh K <sandesh.karkera@flipkart.com> Reviewers: dan norwood <norwood@confluent.io>, Jun Rao <junrao@gmail.com> Closes #2158 from SandeshKarkera/PerfProducerChanges (cherry picked from commit a37bf5f) Signed-off-by: Jun Rao <junrao@gmail.com>

asfbot · 2017-01-20T02:50:51Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1055/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2017-01-20T02:57:44Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1057/
Test FAILed (JDK 8 and Scala 2.11).

asfbot · 2017-01-20T03:02:47Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1055/
Test PASSed (JDK 7 and Scala 2.10).

…roducer script. Current implementation of ProducerPerformance creates static payload. This is not very useful in testing compression or when you want to test with production/custom payloads. So, we decided to add support for providing payload file as an input to producer perf test script. We made the following changes: 1. Added support to provide a payload file which can have the list of payloads that you actually want to send. 2. Moved payload generation inside the send loop for cases when payload file is provided. Following are the changes to how the producer-performance is evoked: 1. You must provide "--record-size" or "--payload-file" but not both. This is because, record size cannot be guaranteed when you are using custom events. e.g. ./kafka-producer-perf-test.sh --topic test_topic --num-records 100000 --producer-props bootstrap.servers=127.0.0.1:9092 acks=0 buffer.memory=33554432 compression.type=gzip batch.size=10240 linger.ms=10 --throughput -1 --payload-file ./test_payloads --payload-delimiter , 2. Earlier "--record-size" was a required config, now you must provide exactly one of "--record-size" or "--payload-file". Providing both will result in an error. 3. Support for an additional parameter "--payload-delimiter" has been added which defaults to "\n" Author: Sandesh K <sandesh.karkera@flipkart.com> Reviewers: dan norwood <norwood@confluent.io>, Jun Rao <junrao@gmail.com> Closes apache#2158 from SandeshKarkera/PerfProducerChanges

SandeshKarkera added 2 commits November 22, 2016 16:38

KAFKA-4432: Added support to supply custom message payloads

af6c6d9

KAFKA-4432: Fixed checkstyle issues

df27806

norwood reviewed Nov 28, 2016

View reviewed changes

KAFKA-4432: Added record-size and payload-file to addMutuallyExclusiv…

ab80c6e

…eGroup

Dummy commit to retrigger Jenkins job.

9e09e96

norwood reviewed Dec 1, 2016

View reviewed changes

asfgit closed this in a37bf5f Jan 20, 2017

Conversation

SandeshKarkera commented Nov 22, 2016

Uh oh!

SandeshKarkera commented Nov 23, 2016

Uh oh!

SandeshKarkera commented Nov 28, 2016

Uh oh!

norwood Nov 28, 2016

Choose a reason for hiding this comment

Uh oh!

SandeshKarkera commented Nov 29, 2016

Uh oh!

SandeshKarkera commented Nov 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SandeshKarkera commented Dec 1, 2016

Uh oh!

norwood Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

SandeshKarkera Dec 5, 2016

Choose a reason for hiding this comment

Uh oh!

SandeshKarkera Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

norwood Dec 12, 2016

Choose a reason for hiding this comment

Uh oh!

SandeshKarkera commented Dec 13, 2016

Uh oh!

asfbot commented Dec 13, 2016

Uh oh!

asfbot commented Dec 13, 2016

Uh oh!

asfbot commented Dec 13, 2016

Uh oh!

SandeshKarkera commented Jan 4, 2017

Uh oh!

junrao commented Jan 20, 2017

Uh oh!

asfbot commented Jan 20, 2017

Uh oh!

asfbot commented Jan 20, 2017

Uh oh!

asfbot commented Jan 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SandeshKarkera commented Nov 29, 2016 •

edited

Loading