showuon commented on a change in pull request #11855:
URL: https://github.com/apache/kafka/pull/11855#discussion_r825266382
##########
File path: core/src/main/scala/kafka/tools/ConsoleProducer.scala
##########
@@ -138,6 +140,12 @@ object ConsoleProducer {
.withOptionalArg()
.describedAs("compression-codec")
.ofType(classOf[String])
+ val batchSizeOpt = parser.accepts("batch-size", "Number of messages to
send in a single batch if they are not being sent synchronously. "+
+ "please note that this opt will be replaced if
max-partition-memory-bytes is also set")
+ .withRequiredArg
+ .describedAs("size")
+ .ofType(classOf[java.lang.Integer])
+ .defaultsTo(200)
Review comment:
We should make the default value the same the
`max-partition-memory-bytes`, which is `16 * 1024`, so that we can do this:
`this opt will be replaced if max-partition-memory-bytes is also set`
##########
File path: core/src/main/scala/kafka/tools/ConsoleProducer.scala
##########
@@ -110,6 +110,8 @@ object ConsoleProducer {
props, ProducerConfig.SEND_BUFFER_CONFIG, config.options,
config.socketBufferSizeOpt)
CommandLineUtils.maybeMergeOptions(
props, ProducerConfig.BUFFER_MEMORY_CONFIG, config.options,
config.maxMemoryBytesOpt)
+ CommandLineUtils.maybeMergeOptions(
+ props, ProducerConfig.BATCH_SIZE_CONFIG, config.options,
config.maxPartitionMemoryBytesOpt)
Review comment:
We can try to merge the `config.batchSizeOpt` first, and then merge
`config.maxPartitionMemoryBytesOpt`. Like this:
```java
CommandLineUtils.maybeMergeOptions(
props, ProducerConfig.BATCH_SIZE_CONFIG, config.options,
config.batchSizeOpt)
CommandLineUtils.maybeMergeOptions(
props, ProducerConfig.BATCH_SIZE_CONFIG, config.options,
config.maxPartitionMemoryBytesOpt)
```
This way, the batch.size value will be replaced with
`maxPartitionMemoryBytesOpt` option value (if set), otherwise, the
`batchSizeOpt` option value will be used (if set), otherwise, default value
will be used (i.e. 16 * 1024).
These following cases will have such results:
1. bin/kafka-console-producer.sh --bootstrap-server localhost:9092 --topic
test **--batch-size 123**
--> batch.size = 123
2. bin/kafka-console-producer.sh --bootstrap-server localhost:9092 --topic
test **--max-partition-memory-bytes 456**
--> batch.size = 456
3. bin/kafka-console-producer.sh --bootstrap-server localhost:9092 --topic
test **--max-partition-memory-bytes 456 --batch-size 123**
--> batch.size = 456
4. bin/kafka-console-producer.sh --bootstrap-server localhost:9092 --topic
test
--> batch.size = 16 * 1024
Does that make sense?
If so, please help add some tests in `ConsoleProducerTest` for the above 4
cases.
Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
