[
https://issues.apache.org/jira/browse/KAFKA-608?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13507494#comment-13507494
]
Jun Rao commented on KAFKA-608:
-------------------------------
Thanks for the patch. Looks good overall. Some comments:
1. TopicExistsException: The scala file is missing from the patch.
2. DefaultEventHandler.send(): Do we want to prepend some text to the payload
in the trace logging below?
if (logger.isTraceEnabled) {
val successfullySentData = response.status.filter(_._2.error ==
ErrorMapping.NoError)
successfullySentData.foreach(m =>
messagesPerTopic(m._1).foreach(message =>
trace("%s".format(Utils.readString(message.message.payload)))))
}
3. ProducerPool: Should we removed the commented out createSyncProducer()?
4. ConsoleConsumer: Should numMessages be long instead of int?
5. I was thinking of having a separate config for metadata request timeout. On
second thought, this may not be necessary. The apps that really need a large
metadata request timeout are migration tool and mirror maker since either of
them could be mirroring a large number of topics. However, these apps can live
with a large producer request timeout (which controls the timeout for both
metadata and send requests). So, what this patch does is probably fine.
> getTopicMetadata does not respect producer config settings
> ----------------------------------------------------------
>
> Key: KAFKA-608
> URL: https://issues.apache.org/jira/browse/KAFKA-608
> Project: Kafka
> Issue Type: Bug
> Affects Versions: 0.8
> Reporter: Jay Kreps
> Assignee: Neha Narkhede
> Priority: Blocker
> Fix For: 0.8
>
> Attachments: kafka-608-v1.patch
>
>
> ProducerPool.scala contains the following code:
> object ProducerPool{
> def createSyncProducer(configOpt: Option[ProducerConfig], broker: Broker):
> SyncProducer = {
> val props = new Properties()
> props.put("host", broker.host)
> props.put("port", broker.port.toString)
> if(configOpt.isDefined)
> props.putAll(configOpt.get.props.props)
> new SyncProducer(new SyncProducerConfig(props))
> }
> }
> Note also, that ClientUtils.getTopicMetadata() does the following:
> ProducerPool.createSyncProducer(None, brokers(i))
> As a result there is no way to control the socket settings for the get
> metadata request.
> My recommendation is that we require the config to be specified in the
> Note that this creates a new sync producer without using ANY of the settings
> the user had given for the producer. In particular the socket timeout is
> defaulted to 500ms.
> This causes unit tests to fail a lot since a newly started test may easily
> timeout on a 500ms request.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
