This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 78b785b78 [util] expose recently seen value in a histogram
78b785b78 is described below

commit 78b785b78ed7e85bcaca11233acdecb6a134c173
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Wed Apr 17 17:40:16 2024 -0700

    [util] expose recently seen value in a histogram
    
    When troubleshooting a performance issue with a Kudu cluster, I found it
    would be useful if each of the existing histogram metrics exposed most
    recently seen value (in particular, I was interested in knowing
    the most recently captured length of a tablet's prepare queue).
    
    This does not align with the semantics of a histogram since collected
    statistics on the distribution of observed values aren't supposed to
    include any notion of recency.  However, I think this is a useful
    improvement from the observability standpoint because it allows for
    collection of valuable information on monitored parameters
    without introducing additional metrics.
    
    NOTE: even if chromium-based Atomics are obsolete and STL atomics
          should be used in new code instead, I opted to use the former
          because it would look much uglier otherwise.  I think
          it's a better option to switch to using STL atomics in
          hdr_histogram.{h,cc} altogether in a separate changelist.
    
    Change-Id: Ia4547faba050e09e31c83372105a9fe97b77ccbc
    Reviewed-on: http://gerrit.cloudera.org:8080/21321
    Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
    Reviewed-by: Wang Xixu <1450306...@qq.com>
    Tested-by: Alexey Serbin <ale...@apache.org>
---
 src/kudu/util/hdr_histogram-test.cc | 11 +++++++++++
 src/kudu/util/hdr_histogram.cc      | 22 +++++++++++++++++++---
 src/kudu/util/hdr_histogram.h       |  5 ++++-
 src/kudu/util/histogram.proto       |  1 +
 src/kudu/util/metrics-test.cc       |  2 ++
 src/kudu/util/metrics.cc            |  2 ++
 6 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/src/kudu/util/hdr_histogram-test.cc 
b/src/kudu/util/hdr_histogram-test.cc
index 22a9ab606..065c6fca3 100644
--- a/src/kudu/util/hdr_histogram-test.cc
+++ b/src/kudu/util/hdr_histogram-test.cc
@@ -35,21 +35,27 @@ TEST_F(HdrHistogramTest, SimpleTest) {
 
   HdrHistogram hist(highest_val, kSigDigits);
   ASSERT_EQ(0, hist.CountInBucketForValue(1));
+  ASSERT_EQ(0, hist.LastValue());
   hist.Increment(1);
   ASSERT_EQ(1, hist.CountInBucketForValue(1));
+  ASSERT_EQ(1, hist.LastValue());
   hist.IncrementBy(1, 3);
   ASSERT_EQ(4, hist.CountInBucketForValue(1));
+  ASSERT_EQ(1, hist.LastValue());
   hist.Increment(10);
   ASSERT_EQ(1, hist.CountInBucketForValue(10));
+  ASSERT_EQ(10, hist.LastValue());
   hist.Increment(20);
   ASSERT_EQ(1, hist.CountInBucketForValue(20));
   ASSERT_EQ(0, hist.CountInBucketForValue(1000));
+  ASSERT_EQ(20, hist.LastValue());
   hist.Increment(1000);
   hist.Increment(1001);
   ASSERT_EQ(2, hist.CountInBucketForValue(1000));
 
   ASSERT_EQ(1 + 1 * 3 + 10 + 20 + 1000 + 1001,
             hist.TotalSum());
+  ASSERT_EQ(1001, hist.LastValue());
 }
 
 TEST_F(HdrHistogramTest, TestCoordinatedOmission) {
@@ -111,6 +117,7 @@ TEST_F(HdrHistogramTest, PercentileAndCopyTest) {
   NO_FATALS(validate_percentiles(&copy, specified_max));
 
   ASSERT_EQ(hist.TotalSum(), copy.TotalSum());
+  ASSERT_EQ(hist.LastValue(), copy.LastValue());
 }
 
 void PopulateHistogram(HdrHistogram* histogram, uint64_t low, uint64_t high) {
@@ -126,7 +133,9 @@ TEST_F(HdrHistogramTest, MergeTest) {
   HdrHistogram other(highest_val, kSigDigits);
 
   PopulateHistogram(&hist, 1, 100);
+  ASSERT_EQ(100, hist.LastValue());
   PopulateHistogram(&other, 101, 250);
+  ASSERT_EQ(250, other.LastValue());
   HdrHistogram old(hist);
   hist.MergeFrom(other);
 
@@ -134,6 +143,8 @@ TEST_F(HdrHistogramTest, MergeTest) {
   ASSERT_EQ(hist.TotalSum(), old.TotalSum() + other.TotalSum());
   ASSERT_EQ(hist.MinValue(), 1);
   ASSERT_EQ(hist.MaxValue(), 250);
+  ASSERT_EQ(100, old.LastValue());
+  ASSERT_EQ(other.LastValue(), hist.LastValue());
   ASSERT_NEAR(hist.MeanValue(), (1 + 250) / 2.0, 1e3);
   ASSERT_EQ(hist.ValueAtPercentile(100.0), 250);
   ASSERT_NEAR(hist.ValueAtPercentile(99.0), 250 * 99.0 / 100, 1e3);
diff --git a/src/kudu/util/hdr_histogram.cc b/src/kudu/util/hdr_histogram.cc
index f8d2bd222..82571ef89 100644
--- a/src/kudu/util/hdr_histogram.cc
+++ b/src/kudu/util/hdr_histogram.cc
@@ -32,6 +32,7 @@
 
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/bits.h"
+#include "kudu/gutil/port.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/status.h"
 
@@ -57,7 +58,8 @@ HdrHistogram::HdrHistogram(uint64_t highest_trackable_value, 
int num_significant
     total_count_(0),
     total_sum_(0),
     min_value_(std::numeric_limits<Atomic64>::max()),
-    max_value_(0) {
+    max_value_(0),
+    last_value_(0) {
   Init();
 }
 
@@ -73,7 +75,8 @@ HdrHistogram::HdrHistogram(const HdrHistogram& other)
     total_count_(0),
     total_sum_(0),
     min_value_(std::numeric_limits<Atomic64>::max()),
-    max_value_(0) {
+    max_value_(0),
+    last_value_(0) {
   Init();
 
   // Not a consistent snapshot but we try to roughly keep it close.
@@ -92,6 +95,8 @@ HdrHistogram::HdrHistogram(const HdrHistogram& other)
   NoBarrier_Store(&max_value_, NoBarrier_Load(&other.max_value_));
   // We must ensure the total is consistent with the copied counts.
   NoBarrier_Store(&total_count_, total_copied_count);
+  // At last, copy the most recent value.
+  NoBarrier_Store(&last_value_, NoBarrier_Load(&other.last_value_));
 }
 
 bool HdrHistogram::IsValidHighestTrackableValue(uint64_t 
highest_trackable_value) {
@@ -187,6 +192,8 @@ void HdrHistogram::IncrementBy(int64_t value, int64_t 
count) {
   NoBarrier_AtomicIncrement(&total_count_, count);
   NoBarrier_AtomicIncrement(&total_sum_, value * count);
 
+  NoBarrier_Store(&last_value_, value);
+
   UpdateMinMax(value, value);
 }
 
@@ -294,6 +301,10 @@ uint64_t HdrHistogram::MaxValue() const {
   return NoBarrier_Load(&max_value_);
 }
 
+uint64_t HdrHistogram::LastValue() const {
+  return NoBarrier_Load(&last_value_);
+}
+
 double HdrHistogram::MeanValue() const {
   uint64_t count = TotalCount();
   if (PREDICT_FALSE(count == 0)) return 0.0;
@@ -329,6 +340,7 @@ uint64_t HdrHistogram::ValueAtPercentile(double percentile) 
const {
 void HdrHistogram::DumpHumanReadable(std::ostream* out) const {
   *out << "Count: " << TotalCount() << endl;
   *out << "Mean: " << MeanValue() << endl;
+  *out << "Last: " << LastValue() << endl;
   *out << "Percentiles:" << endl;
   *out << "   0%  (min) = " << MinValue() << endl;
   *out << "  25%        = " << ValueAtPercentile(25) << endl;
@@ -339,7 +351,7 @@ void HdrHistogram::DumpHumanReadable(std::ostream* out) 
const {
   *out << "  99.9%      = " << ValueAtPercentile(99.9) << endl;
   *out << "  99.99%     = " << ValueAtPercentile(99.99) << endl;
   *out << "  100% (max) = " << MaxValue() << endl;
-  if (MaxValue() >= highest_trackable_value()) {
+  if (PREDICT_FALSE(MaxValue() >= highest_trackable_value())) {
     *out << "*NOTE: some values were greater than highest trackable value" << 
endl;
   }
 }
@@ -365,6 +377,10 @@ void HdrHistogram::MergeFrom(const HdrHistogram& other) {
       NoBarrier_AtomicIncrement(&counts_[i], count);
     }
   }
+  // Just a convention: the other histogram provides the most recent value.
+  if (other.TotalCount() != 0) {
+    NoBarrier_Store(&last_value_, other.last_value_);
+  }
 }
 
 ///////////////////////////////////////////////////////////////////////
diff --git a/src/kudu/util/hdr_histogram.h b/src/kudu/util/hdr_histogram.h
index 212d58353..3be0d4533 100644
--- a/src/kudu/util/hdr_histogram.h
+++ b/src/kudu/util/hdr_histogram.h
@@ -54,7 +54,6 @@
 
 #include "kudu/gutil/atomicops.h"
 #include "kudu/gutil/macros.h"
-#include "kudu/gutil/port.h"
 
 namespace kudu {
 
@@ -152,6 +151,9 @@ class HdrHistogram {
   // Get the exact maximum value (may lie outside the histogram).
   uint64_t MaxValue() const;
 
+  // Get the most recent (last seen) value.
+  uint64_t LastValue() const;
+
   // Get the exact mean value of all recorded values in the histogram.
   double MeanValue() const;
 
@@ -203,6 +205,7 @@ class HdrHistogram {
   base::subtle::Atomic64 total_sum_;
   base::subtle::Atomic64 min_value_;
   base::subtle::Atomic64 max_value_;
+  base::subtle::Atomic64 last_value_;
   std::unique_ptr<base::subtle::Atomic64[]> counts_;
 
   HdrHistogram& operator=(const HdrHistogram& other); // Disable assignment 
operator.
diff --git a/src/kudu/util/histogram.proto b/src/kudu/util/histogram.proto
index e4526e7cc..34e4a4a81 100644
--- a/src/kudu/util/histogram.proto
+++ b/src/kudu/util/histogram.proto
@@ -39,6 +39,7 @@ message HistogramSnapshotPB {
   required uint64 percentile_99_9 = 13;
   required uint64 percentile_99_99 = 14;
   required uint64 max = 15;
+  optional uint64 last = 20;
   repeated uint64 values = 16 [packed = true];
   repeated uint64 counts = 17 [packed = true];
 }
diff --git a/src/kudu/util/metrics-test.cc b/src/kudu/util/metrics-test.cc
index 8d598ebc5..95a6dcf76 100644
--- a/src/kudu/util/metrics-test.cc
+++ b/src/kudu/util/metrics-test.cc
@@ -631,11 +631,13 @@ TEST_F(MetricsTest, SimpleHistogramMergeTest) {
   ASSERT_EQ(2, hist->histogram()->MinValue());
   ASSERT_EQ(4, hist->histogram()->MeanValue());
   ASSERT_EQ(6, hist->histogram()->MaxValue());
+  ASSERT_EQ(6, hist->histogram()->LastValue());
   ASSERT_EQ(2, hist->histogram()->TotalCount());
   ASSERT_EQ(8, hist->histogram()->TotalSum());
   ASSERT_EQ(1, hist_for_merge->histogram()->MinValue());
   ASSERT_EQ(3, hist_for_merge->histogram()->MeanValue());
   ASSERT_EQ(6, hist_for_merge->histogram()->MaxValue());
+  ASSERT_EQ(6, hist_for_merge->histogram()->LastValue());
   ASSERT_EQ(6, hist_for_merge->histogram()->TotalCount());
   ASSERT_EQ(18, hist_for_merge->histogram()->TotalSum());
   ASSERT_EQ(1, hist_for_merge->histogram()->ValueAtPercentile(20.0));
diff --git a/src/kudu/util/metrics.cc b/src/kudu/util/metrics.cc
index 0f1590de5..b24aeb135 100644
--- a/src/kudu/util/metrics.cc
+++ b/src/kudu/util/metrics.cc
@@ -1183,6 +1183,7 @@ Status 
Histogram::GetHistogramSnapshotPB(HistogramSnapshotPB* snapshot_pb,
     snapshot_pb->set_percentile_99_9(0);
     snapshot_pb->set_percentile_99_99(0);
     snapshot_pb->set_max(0);
+    snapshot_pb->set_last(0);
   } else {
     HdrHistogram snapshot(*histogram_);
     snapshot_pb->set_total_count(snapshot.TotalCount());
@@ -1195,6 +1196,7 @@ Status 
Histogram::GetHistogramSnapshotPB(HistogramSnapshotPB* snapshot_pb,
     snapshot_pb->set_percentile_99_9(snapshot.ValueAtPercentile(99.9));
     snapshot_pb->set_percentile_99_99(snapshot.ValueAtPercentile(99.99));
     snapshot_pb->set_max(snapshot.MaxValue());
+    snapshot_pb->set_last(snapshot.LastValue());
 
     if (opts.include_raw_histograms) {
       RecordedValuesIterator iter(&snapshot);

Reply via email to