From de147837dda5865565d718d32465ec41f8f44d25 Mon Sep 17 00:00:00 2001 From: Fedor Bobin Date: Thu, 3 May 2018 19:46:30 +0300 Subject: [PATCH] KAFKA-6853: ZooKeeperRequestLatencyMs is incorrect (#4961) ResponseMetadata.responseTimeMs is always 0 or negative. Reviewers: Rajini Sivaram , Ismael Juma --- .../scala/kafka/zookeeper/ZooKeeperClient.scala | 2 +- .../scala/integration/kafka/api/MetricsTest.scala | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) mode change 100644 => 100755 core/src/main/scala/kafka/zookeeper/ZooKeeperClient.scala diff --git a/core/src/main/scala/kafka/zookeeper/ZooKeeperClient.scala b/core/src/main/scala/kafka/zookeeper/ZooKeeperClient.scala old mode 100644 new mode 100755 index 74a3a2db7ff..5c4cd685565 --- a/core/src/main/scala/kafka/zookeeper/ZooKeeperClient.scala +++ b/core/src/main/scala/kafka/zookeeper/ZooKeeperClient.scala @@ -476,7 +476,7 @@ sealed abstract class AsyncResponse { } case class ResponseMetadata(sendTimeMs: Long, receivedTimeMs: Long) { - def responseTimeMs: Long = sendTimeMs - receivedTimeMs + def responseTimeMs: Long = receivedTimeMs - sendTimeMs } case class CreateResponse(resultCode: Code, path: String, ctx: Option[Any], name: String, metadata: ResponseMetadata) extends AsyncResponse diff --git a/core/src/test/scala/integration/kafka/api/MetricsTest.scala b/core/src/test/scala/integration/kafka/api/MetricsTest.scala index baadd66382f..cea3d279d62 100644 --- a/core/src/test/scala/integration/kafka/api/MetricsTest.scala +++ b/core/src/test/scala/integration/kafka/api/MetricsTest.scala @@ -217,12 +217,16 @@ class MetricsTest extends IntegrationTestHarness with SaslSetup { } private def verifyBrokerZkMetrics(server: KafkaServer, topic: String): Unit = { - // Latency is rounded to milliseconds, so check the count instead. - val initialCount = yammerHistogramCount("kafka.server:type=ZooKeeperClientMetrics,name=ZooKeeperRequestLatencyMs") + val histogram = yammerHistogram("kafka.server:type=ZooKeeperClientMetrics,name=ZooKeeperRequestLatencyMs") + // Latency is rounded to milliseconds, so check the count instead + val initialCount = histogram.count servers.head.zkClient.getLeaderForPartition(new TopicPartition(topic, 0)) - val newCount = yammerHistogramCount("kafka.server:type=ZooKeeperClientMetrics,name=ZooKeeperRequestLatencyMs") + val newCount = histogram.count assertTrue("ZooKeeper latency not recorded", newCount > initialCount) + val min = histogram.min + assertTrue(s"Min latency should not be negative: $min", min >= 0) + assertEquals(s"Unexpected ZK state", "CONNECTED", yammerMetricValue("SessionState")) } @@ -286,12 +290,12 @@ class MetricsTest extends IntegrationTestHarness with SaslSetup { } } - private def yammerHistogramCount(name: String): Long = { + private def yammerHistogram(name: String): Histogram = { val allMetrics = Metrics.defaultRegistry.allMetrics.asScala val (_, metric) = allMetrics.find { case (n, _) => n.getMBeanName.endsWith(name) } .getOrElse(fail(s"Unable to find broker metric $name: allMetrics: ${allMetrics.keySet.map(_.getMBeanName)}")) metric match { - case m: Histogram => m.count + case m: Histogram => m case m => fail(s"Unexpected broker metric of class ${m.getClass}") } }