Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kafka.apache.org
Date: Tue, 4 Nov 2014 01:47:34 +0000 (UTC)
From: "Jun Rao (JIRA)" <jira@apache.org>
To: dev@kafka.apache.org
Message-ID: <JIRA.12718028.1401754422000.404955.1415065654802@Atlassian.JIRA>
In-Reply-To: <JIRA.12718028.1401754422000@Atlassian.JIRA>
References: <JIRA.12718028.1401754422000@Atlassian.JIRA>
 <JIRA.12718028.1401754422753@arcas>
Subject: [jira] [Commented] (KAFKA-1481) Stop using dashes AND underscores
 as separators in MBean names
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Jun Rao commented on KAFKA-1481:
--------------------------------

Thanks for the patch. Just some minor comments below. Otherwise, +1 from me.

60. AbstractFetcherManager: In the following, we don't need to wrap new Gauge in {}.
  "MinFetchRate", {
    new Gauge[Double] {
      // current min fetch rate across all fetchers/topics/partitions
      def value = {
        val headRate: Double =
          fetcherThreadMap.headOption.map(_._2.fetcherStats.requestRate.oneMinuteRate).getOrElse(0)

        fetcherThreadMap.foldLeft(headRate)((curMinAll, fetcherThreadMapEntry) => {
          fetcherThreadMapEntry._2.fetcherStats.requestRate.oneMinuteRate.min(curMinAll)
        })
      }
    }
  },

61. AbstractFetcherThread: Could we align "topic" and "partition" to the same column as "clientId"? Ditto in a few other places.
    Map("clientId" -> metricId.clientId,
      "topic" -> metricId.topic,
      "partition" -> metricId.partitionId.toString)
  )

62. FetchRequestAndResponseMetrics: In the following, could we put Map in a separate line?
  val tags = metricId match {
    case ClientIdAndBroker(clientId, brokerHost, brokerPort) => Map("clientId" -> clientId, "brokerHost" -> brokerHost,
      "brokerPort" -> brokerPort.toString)
    case ClientIdAllBrokers(clientId) => Map("clientId" -> clientId, "allBrokers" -> "true")
  }

62. TestUtils: It's a bit weird that sendMessagesToBrokerPartition() has both a config and configs. We actually don't need the config any more. When sending a message to a partition, we actually don't know which broker the partition is on. So, the message can just be of the form test + "-" + partition + "-" + x. We can also rename the method to sendMessagesToPartition().


> 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_2014-11-03_16-39-41_doc.patch, KAFKA-1481_2014-11-03_17-02-23.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)