junrao commented on a change in pull request #8826:
URL: https://github.com/apache/kafka/pull/8826#discussion_r527972552
##########
File path:
clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java
##########
@@ -105,7 +105,9 @@ public AbstractConfig(ConfigDef definition, Map<?, ?>
originals, Map<String, ?>
throw new ConfigException(entry.getKey().toString(),
entry.getValue(), "Key must be a string.");
this.originals = resolveConfigVariables(configProviderProps,
(Map<String, Object>) originals);
- this.values = definition.parse(this.originals);
+ // pass a copy to definition.parse. Otherwise, the definition.parse
adds all keys of definitions to "used" group
+ // since definition.parse needs to call "RecordingMap#get" when
checking all definitions.
+ this.values = definition.parse(new HashMap<>(this.originals));
Review comment:
Ok. I guess the issue is in the following, where we pass in a
RecordingMap to construct ProducerConfig.
https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java#L384
However, that code seems no longer necessary since we are now setting
clientId in ProducerConfig.postProcessParsedConfig(). Could we just avoid
constructing ProducerConfig there?
----------------------------------------------------------------
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:
us...@infra.apache.org
