Add min / max to histograms

Even 99.9th %-ile measurements can hide a single huge outlier. Add the
min and max measurements to histogram metrics.

Add a test to metrics-test to sanity check histogram measurements.

Change-Id: Ibffd1250a4c9cda91e0cbb1b9b3a9c53df1668fe
Reviewed-on: http://gerrit.cloudera.org:8080/6257
Reviewed-by: Matthew Jacobs <[email protected]>
Tested-by: Impala Public Jenkins


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/13837262
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/13837262
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/13837262

Branch: refs/heads/master
Commit: 13837262b7733b7ae51f7dc134a6037b978629e8
Parents: 5214765
Author: Henry Robinson <[email protected]>
Authored: Fri Mar 3 11:37:21 2017 -0800
Committer: Impala Public Jenkins <[email protected]>
Committed: Sun Mar 5 02:58:25 2017 +0000

----------------------------------------------------------------------
 be/src/util/histogram-metric.h |  4 ++++
 be/src/util/metrics-test.cc    | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/13837262/be/src/util/histogram-metric.h
----------------------------------------------------------------------
diff --git a/be/src/util/histogram-metric.h b/be/src/util/histogram-metric.h
index 5a61648..a520947 100644
--- a/be/src/util/histogram-metric.h
+++ b/be/src/util/histogram-metric.h
@@ -58,6 +58,8 @@ class HistogramMetric : public Metric {
           "95th %-ile", histogram_->ValueAtPercentile(95), 
document->GetAllocator());
       container.AddMember(
           "99.9th %-ile", histogram_->ValueAtPercentile(99.9), 
document->GetAllocator());
+      container.AddMember("max", histogram_->MaxValue(), 
document->GetAllocator());
+      container.AddMember("min", histogram_->MinValue(), 
document->GetAllocator());
       container.AddMember("count", histogram_->TotalCount(), 
document->GetAllocator());
     }
     rapidjson::Value 
type_value(PrintTMetricKind(TMetricKind::HISTOGRAM).c_str(),
@@ -90,6 +92,8 @@ class HistogramMetric : public Metric {
     boost::lock_guard<SpinLock> l(lock_);
     std::stringstream out;
     out << "Count: " << histogram_->TotalCount() << ", "
+        << "min / max: " << PrettyPrinter::Print(histogram_->MinValue(), unit_)
+        << " / " << PrettyPrinter::Print(histogram_->MaxValue(), unit_) << ", "
         << "25th %-ile: "
         << PrettyPrinter::Print(histogram_->ValueAtPercentile(25), unit_) << 
", "
         << "50th %-ile: "

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/13837262/be/src/util/metrics-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/metrics-test.cc b/be/src/util/metrics-test.cc
index 592382e..10ecb64 100644
--- a/be/src/util/metrics-test.cc
+++ b/be/src/util/metrics-test.cc
@@ -24,6 +24,7 @@
 #include "util/collection-metrics.h"
 #include "util/memory-metrics.h"
 #include "util/metrics.h"
+#include "util/histogram-metric.h"
 #include "util/thread.h"
 
 #include "common/names.h"
@@ -326,6 +327,37 @@ TEST_F(MetricsTest, StatsMetricsJson) {
   EXPECT_EQ(stats_val["stddev"].GetDouble(), 5.0);
 }
 
+TEST_F(MetricsTest, HistogramMetrics) {
+  MetricGroup metrics("HistoMetrics");
+  TMetricDef metric_def =
+      MakeTMetricDef("histogram-metric", TMetricKind::HISTOGRAM, 
TUnit::TIME_MS);
+  constexpr int MAX_VALUE = 10000;
+  HistogramMetric* metric = metrics.RegisterMetric(new HistogramMetric(
+          metric_def, MAX_VALUE, 3));
+
+  // Add value beyond limit to make sure it's recorded accurately.
+  for (int i = 0; i <= MAX_VALUE + 1; ++i) metric->Update(i);
+
+  Document document;
+  Value val;
+  metrics.ToJson(true, &document, &val);
+  const Value& histo_val = val["metrics"][0u];
+  EXPECT_EQ(histo_val["min"].GetInt(), 0);
+  EXPECT_EQ(histo_val["max"].GetInt(), MAX_VALUE + 1);
+  EXPECT_EQ(histo_val["25th %-ile"].GetInt(), 2500);
+  EXPECT_EQ(histo_val["50th %-ile"].GetInt(), 5000);
+  EXPECT_EQ(histo_val["75th %-ile"].GetInt(), 7500);
+  EXPECT_EQ(histo_val["90th %-ile"].GetInt(), 9000);
+  EXPECT_EQ(histo_val["95th %-ile"].GetInt(), 9496);
+  EXPECT_EQ(histo_val["99.9th %-ile"].GetInt(), 9984);
+  EXPECT_EQ(histo_val["min"].GetInt(), 0);
+  EXPECT_EQ(histo_val["max"].GetInt(), MAX_VALUE + 1);
+
+  EXPECT_EQ(metric->ToHumanReadable(), "Count: 10002, min / max: 0 / 10s001ms, 
"
+      "25th %-ile: 2s500ms, 50th %-ile: 5s000ms, 75th %-ile: 7s500ms, "
+      "90th %-ile: 9s000ms, 95th %-ile: 9s496ms, 99.9th %-ile: 9s984ms");
+}
+
 TEST_F(MetricsTest, UnitsAndDescriptionJson) {
   MetricGroup metrics("Units");
   AddMetricDef("counter", TMetricKind::COUNTER, TUnit::BYTES, "description");

Reply via email to