Jeffrey Olchovy created KAFKA-3049:
--------------------------------------

             Summary: VerifiableProperties does not respect 'default' 
properties of underlying java.util.Properties instance
                 Key: KAFKA-3049
                 URL: https://issues.apache.org/jira/browse/KAFKA-3049
             Project: Kafka
          Issue Type: Bug
          Components: config, core
            Reporter: Jeffrey Olchovy
            Priority: Minor


When retrieving values from the underlying {{Properties}} instance with the 
{{getString}}, {{get<Type>}}, etc. methods of a {{VerifiableProperties}} 
instance, a call to the underlying {{Properties.containsKey}} method is made. 
This method will not search the default properties values of the instance, 
rendering any default properties defined on the {{Properties}} instance useless.

A practical example is shown below:

{noformat}
// suppose we have a base, default set of properties to supply to all consumer 
groups
val baseProps = new Properties
baseProps.setProperty("zookeeper.connect", "localhost:2181/kafka")
baseProps.setProperty("zookeeper.connection.timeout.ms", "2000")

// additional we have discrete properties instances for each consumer group 
that utilize these defaults
val groupProps1 = new Properties(baseProps)
groupProps1.setProperty("group.id", "test-1")

val groupProps2 = new Properties(baseProps)
groupProps2.setProperty("group.id", "test-2")

// when attempting to create an instance of a high-level Consumer with the 
above properties an exception will be thrown due to the aforementioned problem 
description
java.lang.IllegalArgumentException: requirement failed: Missing required 
property 'zookeeper.connect'
        at scala.Predef$.require(Predef.scala:233)
        at 
kafka.utils.VerifiableProperties.getString(VerifiableProperties.scala:177)
        at kafka.utils.ZKConfig.<init>(ZkUtils.scala:879)
        at kafka.consumer.ConsumerConfig.<init>(ConsumerConfig.scala:100)
        at kafka.consumer.ConsumerConfig.<init>(ConsumerConfig.scala:104)

// however, the groupProps instances will return the correct value for 
"zookeeper.connect" when using `Properties.getProperty`
assert(groupProps1.getProperty("zookeeper.connect", "localhost:2181/kafka"))
assert(groupProps2.getProperty("zookeeper.connect", "localhost:2181/kafka"))
{noformat}

I believe it is worthwhile for Kafka to respect the default properties feature 
of {{java.util.Properties}}, and further, that Kafka should discourage the use 
of the methods on {{Properties}} that are inherited from {{Hashtable}} (e.g. 
{{containsKey}}). One can argue that {{VerifiableProperties}} is not 'correct' 
due to this behavior, but a user can always workaround this by creating 
discrete instances of {{Properties}} with a set of default properties manually 
added to each instance. However, this is inconvenient and may only encourage 
the use of the discouraged {{Hashtable}} methods like {{putAll}}.

Two proposed solutions follow:
1. Do not delegate to the {{Properties.containsKey}} method during the 
invocation of {{VerifiableProperties.containsKey}}. One can use a null check in 
conjunction with {{getProperty}} in its place.
2. Treat the underlying {{Properties}} instance as immutable and assign the 
result of {{Properties.stringPropertyNames()}} to a member of 
{{VerifiableProperties}}. One can check this set of known, available property 
names, which respects the optional default properties, when 
{{VerifiableProperties.containsKey}} is invoked.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to