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(©, 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);