[
https://issues.apache.org/jira/browse/KAFKA-1481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14192034#comment-14192034
]
Jun Rao commented on KAFKA-1481:
--------------------------------
Thanks for the patch. A few more comments.
50. ClientIdBroker: Instead of having 2 subclasses, would it be better to have
just one class
case class ClientIdAndBroker(clientId: String, brokerHost: Option[String],
brokerPort: Option[Int])?
Ditto to ClientIdTopic.
51. TopicPartitionRequestKey: Can this just be TopicAndPartition?
52. MetricsTest:
52.1 Could we remove the extra empty lines after the class?
52.2 remove unnecessary {} in the following (a few other files have a similar
issue)
import java.util.{Properties}
52.3 In testMetricsLeak(), you don't need to create a new zkClient. You can get
one from the base class in ZooKeeperTestHarness.
52.4 Instead of duplicating the createAndShutdownStep() calls, could we use a
loop instead?
52.5 Instead of duplicating sendMessages() and getMessages() from
ZookeeperConsumerConnectorTest, could we extract those methods to TestUtils and
add comments to describe what they do? Then, we can reuse those methods.
53. Could you also include a patch to the 0.8.2 doc
(https://svn.apache.org/repos/asf/kafka/site/082/ops.html) with the metric name
changes?
> Stop using dashes AND underscores as separators in MBean names
> --------------------------------------------------------------
>
> Key: KAFKA-1481
> URL: https://issues.apache.org/jira/browse/KAFKA-1481
> Project: Kafka
> Issue Type: Bug
> Components: core
> Affects Versions: 0.8.1.1
> Reporter: Otis Gospodnetic
> Priority: Critical
> Labels: patch
> Fix For: 0.8.3
>
> Attachments: KAFKA-1481_2014-06-06_13-06-35.patch,
> KAFKA-1481_2014-10-13_18-23-35.patch, KAFKA-1481_2014-10-14_21-53-35.patch,
> KAFKA-1481_2014-10-15_10-23-35.patch, KAFKA-1481_2014-10-20_23-14-35.patch,
> KAFKA-1481_2014-10-21_09-14-35.patch, KAFKA-1481_2014-10-30_21-35-43.patch,
> KAFKA-1481_2014-10-31_14-35-43.patch,
> KAFKA-1481_IDEA_IDE_2014-10-14_21-53-35.patch,
> KAFKA-1481_IDEA_IDE_2014-10-15_10-23-35.patch,
> KAFKA-1481_IDEA_IDE_2014-10-20_20-14-35.patch,
> KAFKA-1481_IDEA_IDE_2014-10-20_23-14-35.patch
>
>
> MBeans should not use dashes or underscores as separators because these
> characters are allowed in hostnames, topics, group and consumer IDs, etc.,
> and these are embedded in MBeans names making it impossible to parse out
> individual bits from MBeans.
> Perhaps a pipe character should be used to avoid the conflict.
> This looks like a major blocker because it means nobody can write Kafka 0.8.x
> monitoring tools unless they are doing it for themselves AND do not use
> dashes AND do not use underscores.
> See: http://search-hadoop.com/m/4TaT4lonIW
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
