kafka-jira mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (KAFKA-5514) KafkaConsumer ignores default values in Properties object because of incorrect use of Properties object.
Date Fri, 29 Sep 2017 19:49:02 GMT

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

ASF GitHub Bot commented on KAFKA-5514:
---------------------------------------

GitHub user Galus opened a pull request:

    https://github.com/apache/kafka/pull/3995

    KAFKA-5514 KafkaConsumer ignores default values in Properties object because of incorrect
use of Properties object.

    This problem plagues anywhere Properties are being misused. I have this issue with KafkaProducer(Properties
properties) constructor. I bet this problem spans across more classes just like this KafkaConsumer(Properties
properties).
    
    Clarification:
    
    putAll is implemented in Hashtable.java and at that point we are already too late.
    When we cast to Map/Hashtable we lose  Properties's 'defaults member which is another
Properties object. Whenever you use Properties's getProperty() method, if the Property object
does not explicity contain the property, it will refer to it's internal defaults Properties
object which can be set as so:
    Properties myProps = new Properties(oldProperties)
    if oldProperties has a key:value of foo:bar
    calling myProps.get(foo) will not return bar
    whereas, calling myProps.getProperty(foo) will return bar
    
    Reference:
    
    
    ```
    public
    class Properties extends Hashtable<Object,Object> {
    ...
        /**
         * A property list that contains default values for any keys not
         * found in this property list.
         *
         * @serial
         */
        protected Properties defaults;
    
        /**
         * Creates an empty property list with the specified defaults.
         *
         * @param   defaults   the defaults.
         */
        public Properties(Properties defaults) {
            this.defaults = defaults;
        }
    ...
        public String getProperty(String key) {
            Object oval = super.get(key);
            String sval = (oval instanceof String) ? (String)oval : null;
            return ((sval == null) && (defaults != null)) ? defaults.getProperty(key)
: sval;
        }
    ...
    ```
    
    
    
    Here is the fix:
    
    ```
            for (final String name: properties.stringPropertyNames()) {
                if (properties.get(name) == null) {
                    map.put(name, properties.getProperty(name));
                }
            }
    ```
    
    
    This pulls all the 'default' inner Properties object's entries up into the outer level
Properties object.
    This should be applied to all the public methods that are being passed in a Properties
object
    See https://github.com/Galus/kafka/commit/8af611dfd9c8f3cb37f1d2b400890525c5a646d3 and
https://github.com/Galus/kafka/commit/130b4cde2da4d9ede7aa6d6a99997af32d1a00fc
    
    
    Similar Issues addressing this issue: KAFKA-2184, KAFKA-3049, KAFKA-3161
    @Kamal15 
    
    This contribution is my original work and I hereby license it for use by the apache kafka
project's open source license.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Galus/kafka trunk

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/kafka/pull/3995.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3995
    
----
commit 8053f294ff938c6cc295b42f8afa6bec2eb5dc75
Author: Galus <mgalus@gmail.com>
Date:   2017-09-29T19:15:07Z

    KAFKA-5514 Better Handling Of Properties
    
    KafkaProducer update.

commit 8af611dfd9c8f3cb37f1d2b400890525c5a646d3
Author: Galus <mgalus@gmail.com>
Date:   2017-09-29T19:29:34Z

    KAFKA-5514 Better Properties Handling
    
    Fixes KafkaConsumer's inability to handle Properties object's inner Properties object
'defaults'

commit 130b4cde2da4d9ede7aa6d6a99997af32d1a00fc
Author: Galus <mgalus@gmail.com>
Date:   2017-09-29T19:31:56Z

    KAFKA-5514 Better Properties Handling
    
    Fixes KafkaProducer's inability to handle Properties object's inner Properties object
'defaults'

----


> KafkaConsumer ignores default values in Properties object because of incorrect use of
Properties object.
> --------------------------------------------------------------------------------------------------------
>
>                 Key: KAFKA-5514
>                 URL: https://issues.apache.org/jira/browse/KAFKA-5514
>             Project: Kafka
>          Issue Type: Bug
>          Components: clients
>    Affects Versions: 0.10.2.1
>            Reporter: Geert Schuring
>
> When setting default values in a Properties object the KafkaConsumer ignores these values
because the Properties object is being treated as a Map. The ConsumerConfig object uses the
putAll method to copy properties from the incoming object to its local copy. (See https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java#L471)
> This is incorrect because it only copies the explicit properties and ignores the default
values also present in the properties object. (Also see: https://stackoverflow.com/questions/2004833/how-to-merge-two-java-util-properties-objects)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message