You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/05/29 08:47:54 UTC
[GitHub] [kafka] itantiger opened a new pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
itantiger opened a new pull request #8753:
URL: https://github.com/apache/kafka/pull/8753
…in consumer.config
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r549118304
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
> i dont what should do next？
Could you update the code (according to the reviews) and then push the changes to be a new commit. Also, please rebase your code (by trunk).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r549113162
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
Hi @chia7712 This is my first submission, use `props.putIfAbsent` is better, but i dont what should do next？
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r549126199
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
I had forked once again and then updated the code (according to your suggestion). Can you take a look at this[https://github.com/apache/kafka/pull/9790](url)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#issuecomment-647359639
@ijuma Can you take a look at this (since you requested it in #8390 )?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r549126199
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
I had forked once again and then updated the code (according to your suggestion). Can you take a look at this?
[https://github.com/apache/kafka/pull/9790](url)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r549108606
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
So sorry, no one replied to me before. I haven't paid attention to it for a long time
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 closed pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
chia7712 closed pull request #8753:
URL: https://github.com/apache/kafka/pull/8753
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#issuecomment-635930386
@ijuma Can you take a look at this (since you requested it in #8390 )?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r508235401
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
Is ```props.putIfAbsent``` more better ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r549126199
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
I had forked once again and then updated the code (according to your suggestion). Can you take a look at this?
https://github.com/apache/kafka/pull/9790
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#issuecomment-635877349
I dont kown how to link issues, so paste issues link here [https://issues.apache.org/jira/browse/KAFKA-10043](url)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on a change in pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on a change in pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#discussion_r549119751
##########
File path: core/src/main/scala/kafka/tools/ConsumerPerformance.scala
##########
@@ -271,13 +271,22 @@ object ConsumerPerformance extends LazyLogging {
val brokerHostsAndPorts = options.valueOf(if (options.has(bootstrapServerOpt)) bootstrapServerOpt else brokerListOpt)
props.put(ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG, brokerHostsAndPorts)
- props.put(ConsumerConfig.GROUP_ID_CONFIG, options.valueOf(groupIdOpt))
- props.put(ConsumerConfig.RECEIVE_BUFFER_CONFIG, options.valueOf(socketBufferSizeOpt).toString)
- props.put(ConsumerConfig.MAX_PARTITION_FETCH_BYTES_CONFIG, options.valueOf(fetchSizeOpt).toString)
- props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, if (options.has(resetBeginningOffsetOpt)) "latest" else "earliest")
- props.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, classOf[ByteArrayDeserializer])
- props.put(ConsumerConfig.CHECK_CRCS_CONFIG, "false")
+ if (props.getProperty(ConsumerConfig.GROUP_ID_CONFIG) == null)
Review comment:
Do you mean i need to fork apache/kafka current trunk again? if that i will delete this fork.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#issuecomment-637519467
@guozhangwang Can you take a look at this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] itantiger commented on pull request #8753: KAFKA-10043:Some parameters will be overwritten which was configured …
Posted by GitBox <gi...@apache.org>.
itantiger commented on pull request #8753:
URL: https://github.com/apache/kafka/pull/8753#issuecomment-651738671
@rhauch Can you take a look at this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org