kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jun Rao (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (KAFKA-369) remove ZK dependency on producer
Date Thu, 02 Aug 2012 16:48:02 GMT

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

Jun Rao commented on KAFKA-369:
-------------------------------

Thanks for patch v1. Some comments:

1. ProducerConfig:
1.1 change broker.list's format to host:port; Also, change the comment and explain this is
just for bootstrapping and it just needs to be a subset of all brokers in the cluster.
1.2 We should make broker.list a required property by not providing a default value. If we
do the following, an exception will be thrown if the property is not specified. There is no
need to test for null any more.
     Utils.getString(props, "broker.list")
1.3 There is no need for ProducerConfig to extend ZKConfig.

2. Producer: There is no need to check the existence of brokerList since it will be handled
in ProducerConfig.

3. BrokerPartitionInfo:
3.1 It's better to rename it to something like MetaDataCache
3.2 This class shouldn't depend on ProducerPool, which is supposed to be used for caching
SyncProducer objects for sending produce request. Instead, this class only needs to know broker.list.
The reason is that broker.list is just for bootstrapping and we can pass in a VIP instead
of a real host. Each time we need to send a getMetaData request, we can just pick a host from
broker.list, instantiate a SyncProducer, send the request, get the response, and close the
SyncProducer. Since getMetaData is going to be used rarely, there is no need to cache the
connection. It also avoids the timeout problem with VIP. For new brokers that we get in updateInfo(),
we can instantiate a new SyncProducer and add it to ProducerPool.

4. ProducerPool
4.1 If we do the above in BrokerPartitionInfo, we can get rid of the following methods.
  def addProducers(config: ProducerConfig) 
  def getAnyProducer: SyncProducer 

5. KafkaConfig: The default value of hostName should be InetAddress.getLocalHost.getHostAddress.
We can then get rid of the hostName null check in KafkaZookeeper.registerBrokerInZk().

6. KafkaKig4jAppender: get rid of the commented out lines related to ZkConnect; get rid of
host and port

7. KafkaKig4jAppenderTest: testZkConnectLog4jAppends should be renamed since there is no ZK
connect any more.

8. ProducerTest: 
8.1 all method of testZK* should be renamed properly.
8.2  Quite a few places have the following checks. Can't we just combine them using a check
for leaderIsLive?
    assertTrue("Topic new-topic not created after timeout", TestUtils.waitUntilTrue(() =>
      AdminUtils.getTopicMetaDataFromZK(List("new-topic"),
        zkClient).head.errorCode != ErrorMapping.UnknownTopicCode, zookeeper.tickTime))
    TestUtils.waitUntilLeaderIsElectedOrChanged(zkClient, "new-topic", 0, 500)

9. Utils.getAllBrokersFromBrokerList(): There is no need to get the broker id from broker.list.
We can just assign some sequential ids.

                
> remove ZK dependency on producer
> --------------------------------
>
>                 Key: KAFKA-369
>                 URL: https://issues.apache.org/jira/browse/KAFKA-369
>             Project: Kafka
>          Issue Type: Sub-task
>          Components: core
>    Affects Versions: 0.8
>            Reporter: Jun Rao
>            Assignee: Yang Ye
>         Attachments: kafka_369_v1.diff
>
>   Original Estimate: 252h
>  Remaining Estimate: 252h
>
> Currently, the only place that ZK is actually used is in BrokerPartitionInfo. We use
ZK to get a list of brokers for making TopicMetadataRequest requests. Instead, we can provide
a list of brokers in the producer config directly. That way, the producer client is no longer
dependant on ZK.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message