diff --git a/monitoring/histogram.cc b/monitoring/histogram.cc index 331796e5b..4d00219c9 100644 --- a/monitoring/histogram.cc +++ b/monitoring/histogram.cc @@ -168,10 +168,12 @@ double HistogramStat::StandardDeviation() const { static_cast(num()); // Use double to avoid integer overflow double cur_sum = static_cast(sum()); double cur_sum_squares = static_cast(sum_squares()); - if (cur_num == 0) return 0; + if (cur_num == 0.0) { + return 0.0; + } double variance = (cur_sum_squares * cur_num - cur_sum * cur_sum) / (cur_num * cur_num); - return std::sqrt(variance); + return std::sqrt(std::max(variance, 0.0)); } std::string HistogramStat::ToString() const { diff --git a/monitoring/histogram.h b/monitoring/histogram.h index 6d72b0651..41af01442 100644 --- a/monitoring/histogram.h +++ b/monitoring/histogram.h @@ -140,6 +140,8 @@ class HistogramImpl : public Histogram { virtual ~HistogramImpl() {} + inline HistogramStat& TEST_GetStats() { return stats_; } + private: HistogramStat stats_; std::mutex mutex_; diff --git a/monitoring/histogram_test.cc b/monitoring/histogram_test.cc index 7b63dc93a..1ed561d44 100644 --- a/monitoring/histogram_test.cc +++ b/monitoring/histogram_test.cc @@ -147,7 +147,8 @@ TEST_F(HistogramTest, HistogramWindowingExpire) { ASSERT_EQ(histogramWindowing.num(), 100); ASSERT_EQ(histogramWindowing.min(), 1); ASSERT_EQ(histogramWindowing.max(), 1); - ASSERT_EQ(histogramWindowing.Average(), 1); + ASSERT_EQ(histogramWindowing.Average(), 1.0); + ASSERT_EQ(histogramWindowing.StandardDeviation(), 0.0); PopulateHistogram(histogramWindowing, 2, 2, 100); clock->SleepForMicroseconds(micros_per_window); @@ -155,6 +156,7 @@ TEST_F(HistogramTest, HistogramWindowingExpire) { ASSERT_EQ(histogramWindowing.min(), 1); ASSERT_EQ(histogramWindowing.max(), 2); ASSERT_EQ(histogramWindowing.Average(), 1.5); + ASSERT_GT(histogramWindowing.StandardDeviation(), 0.0); PopulateHistogram(histogramWindowing, 3, 3, 100); clock->SleepForMicroseconds(micros_per_window); @@ -162,6 +164,7 @@ TEST_F(HistogramTest, HistogramWindowingExpire) { ASSERT_EQ(histogramWindowing.min(), 1); ASSERT_EQ(histogramWindowing.max(), 3); ASSERT_EQ(histogramWindowing.Average(), 2.0); + ASSERT_GT(histogramWindowing.StandardDeviation(), 0.0); // dropping oldest window with value 1, remaining 2 ~ 4 PopulateHistogram(histogramWindowing, 4, 4, 100); @@ -170,6 +173,7 @@ TEST_F(HistogramTest, HistogramWindowingExpire) { ASSERT_EQ(histogramWindowing.min(), 2); ASSERT_EQ(histogramWindowing.max(), 4); ASSERT_EQ(histogramWindowing.Average(), 3.0); + ASSERT_GT(histogramWindowing.StandardDeviation(), 0.0); // dropping oldest window with value 2, remaining 3 ~ 5 PopulateHistogram(histogramWindowing, 5, 5, 100); @@ -178,6 +182,7 @@ TEST_F(HistogramTest, HistogramWindowingExpire) { ASSERT_EQ(histogramWindowing.min(), 3); ASSERT_EQ(histogramWindowing.max(), 5); ASSERT_EQ(histogramWindowing.Average(), 4.0); + ASSERT_GT(histogramWindowing.StandardDeviation(), 0.0); } TEST_F(HistogramTest, HistogramWindowingMerge) { @@ -231,6 +236,15 @@ TEST_F(HistogramTest, LargeStandardDeviation) { ASSERT_LT(fabs(histogram.StandardDeviation() - 288675), 1); } +TEST_F(HistogramTest, LostUpdateStandardDeviation) { + HistogramImpl histogram; + PopulateHistogram(histogram, 100, 100, 100); + // Simulate a possible lost update (since they are not atomic) + histogram.TEST_GetStats().sum_squares_ -= 10000; + // Ideally zero, but should never be negative or NaN + ASSERT_GE(histogram.StandardDeviation(), 0.0); +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) {