KAFKA-19598: Command-line arguments for producer perf test #20361
Conversation
AndrewJSchofield
changed the title
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
@AndrewJSchofield thanks for this beautiful patch. I just have a few minor comments. Please take a look.
| cmd += " export KAFKA_LOG4J_OPTS=\"%s%s\"; " % (get_log4j_config_param(node), get_log4j_config_for_tools(node)) | ||
| cmd += "KAFKA_OPTS=%(kafka_opts)s KAFKA_HEAP_OPTS=\"-XX:+HeapDumpOnOutOfMemoryError\" %(kafka_run_class)s org.apache.kafka.tools.ProducerPerformance " \ | ||
| "--topic %(topic)s --num-records %(num_records)d --record-size %(record_size)d --throughput %(throughput)d --producer-props bootstrap.servers=%(bootstrap_servers)s client.id=%(client_id)s %(metrics_props)s" % args | ||
| "--topic %(topic)s --num-records %(num_records)d --record-size %(record_size)d --throughput %(throughput)d --command-property bootstrap.servers=%(bootstrap_servers)s client.id=%(client_id)s %(metrics_props)s" % args |
There was a problem hiding this comment.
I assumed this change would cause an issue when running ProducerPerformanceService on an older kafka. However, it seems that ProducerPerformanceService always uses the tool jar from the development branch
| "There are three ways to specify the transactional id: set transactional.id=<id> via --command-property, " + | ||
| "set transactional.id=<id> in the config file via --command-config, or use --transactional-id <id>."); | ||
|
|
||
| parser.addArgument("--bootstrap-server") |
There was a problem hiding this comment.
Should those configuration changes be documented in upgrade.html?
There was a problem hiding this comment.
Good idea, but I think a single task to do the entire KIP once the PRs are done is best. https://issues.apache.org/jira/browse/KAFKA-19635
| if (bootstrapServers == null && commandProperties == null && producerConfigs == null && producerConfigFile == null && commandConfigFile == null) { | ||
| throw new ArgumentParserException("At least one of --bootstrap-server, --command-property, --producer-props, --producer.config or --command-config must be specified.", parser); | ||
| } | ||
| if (commandProperties != null && producerConfigs != null) { |
There was a problem hiding this comment.
it seems this case does not have a unit test
There was a problem hiding this comment.
Yes, as temporary code, I didn't write one. But it was a shortcut and I will add one.
| } | ||
| if (producerConfigs == null && producerConfigFile == null) { | ||
| throw new ArgumentParserException("Either --producer-props or --producer.config must be specified.", parser); | ||
| if (commandConfigFile != null && producerConfigFile != null) { |
There was a problem hiding this comment.
Yup, I'll add a test.
2 participants
This implements KIP-1147 for kafka-producer-perf-test.sh.
Reviewers: Chia-Ping Tsai chia7712@gmail.com