[ 
https://issues.apache.org/jira/browse/KAFKA-13689?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17511136#comment-17511136
 ] 

RivenSun edited comment on KAFKA-13689 at 3/23/22, 10:04 AM:
-------------------------------------------------------------

Hi [~guozhang]  I thought about it carefully, maybe we are thinking too 
complicated, or we have been looking at the logUnused() method from the 
perspective of Kafka.

{*}The logUnused() method is user-oriented{*}, in order to tell the user all 
the configurations he passed in, {*}whether these configurations are known or 
unknown{*}, which configurations are used and which are unused.
In short, the logUnused method should compare the difference between 
`originals` and `used`.

So my few points are:

1. Keep the existing logic of the unused() method, just modify the log output 
information of logUnused(). As you mentioned, printing "isn't a known config" 
in the logUnused() method is both incorrect and weird.
{code:java}
/**
 * Log warnings for any unused configurations
 */
public void logUnused() {
    Set<String> unusedkeys = unused();
    if (!unusedkeys.isEmpty()) {
        log.warn("These configurations '{}' were supplied but are not used.", 
unusedkeys);
    }
} {code}
2. For unenable knownConfig, Kafka do not need to actively `ignore` it. 
As in the example in this JIRA, in the logUnused() method, the 
`enable.idempotence` configuration is not printed, because it really has been 
retrieved by Kafka.
But the `transaction.timeout.ms` configuration is printed. Because the user has 
passed in this configuration, but Kafka has not retrieved this configuration 
when calling the logUnused() method, it should be printed out.

3. For unKnownConfig , if the user has not retrieved these configurations in 
their own custom plugin module by the time the logUnused() method is called, 
then logUnused() will print these unretrieved custom configurations.
By the way, configurations that kafka has removed and deprecated configurations 
(with recommended configurations configured at the same time) can be viewed 
similar to unknownConfig. Maybe users will use these two types of configuration 
in their custom plugins.

WDYT?
Thanks.

 


was (Author: rivensun):
Hi [~guozhang]  I thought about it carefully, maybe we are thinking too 
complicated, or we have been looking at the logUnused() method from the 
perspective of Kafka.

{*}The logUnused() method is user-oriented{*}, in order to tell the user all 
the configurations he passed in, {*}whether these configurations are known or 
unknown{*}, which configurations are used and which are unused.
In short, the logUnused method should compare the difference between 
`originals` and `used`.

So my few points are:

1. Keep the existing logic of the unused() method, just modify the log output 
information of logUnused(). As you mentioned, printing "isn't a known config" 
in the logUnused() method is both incorrect and weird.
{code:java}
/**
 * Log warnings for any unused configurations
 */
public void logUnused() {
    Set<String> unusedkeys = unused();
    if (!unusedkeys.isEmpty()) {
        log.warn("These configurations '{}' were supplied but are not used.", 
unusedkeys);
    }
} {code}
2. For unenable knownConfig, Kafka do not need to actively `ignore` it. 
As in the example in this JIRA, in the logUnused() method, the 
`enable.idempotence` configuration is not printed, because it really has been 
retrieved by Kafka.
But the `transaction.timeout.ms` configuration is printed. Because the user has 
passed in this configuration, but Kafka has not retrieved this configuration 
when calling the logUnused() method, it should be printed out.

3. For unKnownConfig , if the user has not retrieved these configurations in 
their own custom plugin module by the time the logUnused() method is called, 
then logUnused() will print these unretrieved custom configurations.
By the way, configurations that kafka has removed and deprecated configurations 
(with recommended configurations configured at the same time) can be viewed 
similar to unknownConfig.

WDYT?
Thanks.

 

> AbstractConfig log print information is incorrect
> -------------------------------------------------
>
>                 Key: KAFKA-13689
>                 URL: https://issues.apache.org/jira/browse/KAFKA-13689
>             Project: Kafka
>          Issue Type: Bug
>          Components: config
>    Affects Versions: 3.0.0
>            Reporter: RivenSun
>            Assignee: RivenSun
>            Priority: Major
>             Fix For: 3.2.0
>
>
> h1. 1.Example
> KafkaClient version is 3.1.0, KafkaProducer init properties:
>  
> {code:java}
> Properties props = new Properties();
> props.put(ProducerConfig.ENABLE_IDEMPOTENCE_CONFIG, false);
> props.put(ProducerConfig.TRANSACTION_TIMEOUT_CONFIG, 60003);{code}
>  
>  
> Partial log of KafkaProducer initialization:
> {code:java}
>     ssl.truststore.location = C:\Personal 
> File\documents\KafkaSSL\client.truststore.jks
>     ssl.truststore.password = [hidden]
>     ssl.truststore.type = JKS
>     transaction.timeout.ms = 60003
>     transactional.id = null
>     value.serializer = class 
> org.apache.kafka.common.serialization.StringSerializer[main] INFO 
> org.apache.kafka.common.security.authenticator.AbstractLogin - Successfully 
> logged in.
> [main] WARN org.apache.kafka.clients.producer.ProducerConfig - The 
> configuration 'transaction.timeout.ms' was supplied but isn't a known config.
> [main] INFO org.apache.kafka.common.utils.AppInfoParser - Kafka version: 3.1.0
> [main] INFO org.apache.kafka.common.utils.AppInfoParser - Kafka commitId: 
> 37edeed0777bacb3
> [main] INFO org.apache.kafka.common.utils.AppInfoParser - Kafka startTimeMs: 
> 1645602332999 {code}
> From the above log, you can see that KafkaProducer has applied the user's 
> configuration, {*}transaction.timeout.ms=60003{*}, the default value of this 
> configuration is 60000.
> But we can see another line of log:
> [main] WARN org.apache.kafka.clients.producer.ProducerConfig - The 
> configuration *'transaction.timeout.ms'* was supplied but isn't a 
> *{color:#ff0000}known{color}* config.
>  
> h1. 2.RootCause:
> 1) ProducerConfig.ENABLE_IDEMPOTENCE_CONFIG is set to {*}false{*}.
> So the configurations related to the KafkaProducer transaction will not be 
> requested.
> See the source code: KafkaProducer#configureTransactionState(...) .
> 2) AbstractConfig#logUnused() -> AbstractConfig#unused()
> {code:java}
> public Set<String> unused() {
>     Set<String> keys = new HashSet<>(originals.keySet());
>     keys.removeAll(used);
>     return keys;
> } {code}
> If a configuration has not been requested, the configuration will not be put 
> into the used variable. SourceCode see as below:
> AbstractConfig#get(String key)
>  
> {code:java}
> protected Object get(String key) {
>     if (!values.containsKey(key))
>         throw new ConfigException(String.format("Unknown configuration '%s'", 
> key));
>     used.add(key);
>     return values.get(key);
> } {code}
> h1.  
> h1. Solution:
> 1. AbstractConfig#logUnused() method
> Modify the log printing information of this method,and the unused 
> configuration log print level can be changed to {*}INFO{*}, what do you think?
> {code:java}
> /**
>  * Log infos for any unused configurations
>  */
> public void logUnused() {     for (String key : unused())
>         log.info("The configuration '{}' was supplied but isn't a used 
> config.", key);
> }{code}
>  
>  
> 2. AbstractConfig provides two new methods: logUnknown() and unknown()
> {code:java}
> /**
>  * Log warnings for any unknown configurations
>  */
> public void logUnknown() {
>     for (String key : unknown())
>         log.warn("The configuration '{}' was supplied but isn't a known 
> config.", key);
> } {code}
>  
> {code:java}
> public Set<String> unknown() {
>     Set<String> keys = new HashSet<>(originals.keySet());
>     keys.removeAll(values.keySet());
>     return keys;
> } {code}
>  
>  
>  



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to