[FLINK-12984][metrics] only call Histogram#getStatistics() once where…

… possible In some occasions, Histogram#getStatistics() was called multiple times to retrieve different statistics. However, at least the dropwizard implementation has some constant overhead per call and we should maybe rather interpret this method as returning a point-in-time snapshot of the histogram in order to get consistent values when querying them.
deepfield · Aug 15, 2019 · d9f0127 · d9f0127
1 parent fd9ef60
commit d9f0127
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 11 deletions.
diff --git a/...metrics/flink-metrics-jmx/src/test/java/org/apache/flink/metrics/jmx/JMXReporterTest.java b/...metrics/flink-metrics-jmx/src/test/java/org/apache/flink/metrics/jmx/JMXReporterTest.java
@@ -19,6 +19,7 @@
 package org.apache.flink.metrics.jmx;
 
 import org.apache.flink.metrics.Gauge;
+import org.apache.flink.metrics.HistogramStatistics;
 import org.apache.flink.metrics.reporter.MetricReporter;
 import org.apache.flink.metrics.util.TestHistogram;
 import org.apache.flink.metrics.util.TestMeter;
@@ -245,16 +246,17 @@ public void testHistogramReporting() throws Exception {
  assertEquals(11, attributeInfos.length);
 
  assertEquals(histogram.getCount(), mBeanServer.getAttribute(objectName, "Count"));
- assertEquals(histogram.getStatistics().getMean(), mBeanServer.getAttribute(objectName, "Mean"));
- assertEquals(histogram.getStatistics().getStdDev(), mBeanServer.getAttribute(objectName, "StdDev"));
- assertEquals(histogram.getStatistics().getMax(), mBeanServer.getAttribute(objectName, "Max"));
- assertEquals(histogram.getStatistics().getMin(), mBeanServer.getAttribute(objectName, "Min"));
- assertEquals(histogram.getStatistics().getQuantile(0.5), mBeanServer.getAttribute(objectName, "Median"));
- assertEquals(histogram.getStatistics().getQuantile(0.75), mBeanServer.getAttribute(objectName, "75thPercentile"));
- assertEquals(histogram.getStatistics().getQuantile(0.95), mBeanServer.getAttribute(objectName, "95thPercentile"));
- assertEquals(histogram.getStatistics().getQuantile(0.98), mBeanServer.getAttribute(objectName, "98thPercentile"));
- assertEquals(histogram.getStatistics().getQuantile(0.99), mBeanServer.getAttribute(objectName, "99thPercentile"));
- assertEquals(histogram.getStatistics().getQuantile(0.999), mBeanServer.getAttribute(objectName, "999thPercentile"));
+ HistogramStatistics statistics = histogram.getStatistics();
+ assertEquals(statistics.getMean(), mBeanServer.getAttribute(objectName, "Mean"));
+ assertEquals(statistics.getStdDev(), mBeanServer.getAttribute(objectName, "StdDev"));
+ assertEquals(statistics.getMax(), mBeanServer.getAttribute(objectName, "Max"));
+ assertEquals(statistics.getMin(), mBeanServer.getAttribute(objectName, "Min"));
+ assertEquals(statistics.getQuantile(0.5), mBeanServer.getAttribute(objectName, "Median"));
+ assertEquals(statistics.getQuantile(0.75), mBeanServer.getAttribute(objectName, "75thPercentile"));
+ assertEquals(statistics.getQuantile(0.95), mBeanServer.getAttribute(objectName, "95thPercentile"));
+ assertEquals(statistics.getQuantile(0.98), mBeanServer.getAttribute(objectName, "98thPercentile"));
+ assertEquals(statistics.getQuantile(0.99), mBeanServer.getAttribute(objectName, "99thPercentile"));
+ assertEquals(statistics.getQuantile(0.999), mBeanServer.getAttribute(objectName, "999thPercentile"));
 
  } finally {
  if (registry != null) {

diff --git a/...metheus/src/main/java/org/apache/flink/metrics/prometheus/AbstractPrometheusReporter.java b/...metheus/src/main/java/org/apache/flink/metrics/prometheus/AbstractPrometheusReporter.java
@@ -24,6 +24,7 @@
 import org.apache.flink.metrics.Counter;
 import org.apache.flink.metrics.Gauge;
 import org.apache.flink.metrics.Histogram;
+import org.apache.flink.metrics.HistogramStatistics;
 import org.apache.flink.metrics.Meter;
 import org.apache.flink.metrics.Metric;
 import org.apache.flink.metrics.MetricConfig;
@@ -300,10 +301,11 @@ void remove(final List<String> labelValues) {
  private void addSamples(final List<String> labelValues, final Histogram histogram, final List<MetricFamilySamples.Sample> samples) {
  samples.add(new MetricFamilySamples.Sample(metricName + "_count",
  labelNamesWithQuantile.subList(0, labelNamesWithQuantile.size() - 1), labelValues, histogram.getCount()));
+ final HistogramStatistics statistics = histogram.getStatistics();
  for (final Double quantile : QUANTILES) {
  samples.add(new MetricFamilySamples.Sample(metricName, labelNamesWithQuantile,
  addToList(labelValues, quantile.toString()),
- histogram.getStatistics().getQuantile(quantile)));
+ statistics.getQuantile(quantile)));
  }
  }
  }