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 <[email protected]>
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 <[email protected]>
Reviewed-by: Wang Xixu <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
---
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);
