database/rocksdb
1
0
Fork 0
mirror of https://github.com/facebook/rocksdb.git synced 2026-10-24 19:59:35 +03:00
Code Activity

Fix overflow bug in standard deviation computation. (#10100)

Browse Source 
Summary:
There was an overflow bug when computing the variance in the HistogramStat class.

This manifests, for instance, when running cache_bench with default arguments. This executes 32M lookups/inserts/deletes in a block cache, and then computes (among other things) the variance of the latencies. The variance is computed as ``variance = (cur_sum_squares * cur_num - cur_sum * cur_sum) / (cur_num * cur_num)``, where ``cum_sum_squares`` is the sum of the squares of the samples, ``cur_num`` is the number of samples, and ``cur_sum`` is the sum of the samples. Because the median latency in a typical run is around 3800 nanoseconds, both the ``cur_sum_squares * cur_num`` and ``cur_sum * cur_sum`` terms overflow as uint64_t.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10100

Test Plan: Added a unit test. Run ``make -j24 histogram_test && ./histogram_test``.

Reviewed By: pdillinger

Differential Revision: D36942738

Pulled By: guidotag

fbshipit-source-id: 0af5fb9e2a297a284e8e74c24e604d302906006e
This commit is contained in:
Guido Tagliavini Ponce
2022-06-06 13:53:47 -07:00
committed by Facebook GitHub Bot
parent 4f78f9699b
commit 2af132c341
2 changed files with 12 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
monitoring/histogram.cc
View File

@@ -164,15 +164,16 @@ double HistogramStat::Average() const {
}
double HistogramStat::StandardDeviation() const {
uint64_t cur_num = num();
uint64_t cur_sum = sum();
uint64_t cur_sum_squares = sum_squares();
double cur_num =
static_cast<double>(num()); // Use double to avoid integer overflow
double cur_sum = static_cast<double>(sum());
double cur_sum_squares = static_cast<double>(sum_squares());
if (cur_num == 0) return 0;
double variance =
static_cast<double>(cur_sum_squares * cur_num - cur_sum * cur_sum) /
static_cast<double>(cur_num * cur_num);
(cur_sum_squares * cur_num - cur_sum * cur_sum) / (cur_num * cur_num);
return std::sqrt(variance);
}
std::string HistogramStat::ToString() const {
uint64_t cur_num = num();
std::string r;

6
monitoring/histogram_test.cc
View File

@@ -225,6 +225,12 @@ TEST_F(HistogramTest, HistogramWindowingMerge) {
ASSERT_EQ(histogramWindowing.max(), 5);
}
TEST_F(HistogramTest, LargeStandardDeviation) {
HistogramImpl histogram;
PopulateHistogram(histogram, 1, 1000000);
ASSERT_LT(fabs(histogram.StandardDeviation() - 288675), 1);
}
} // namespace ROCKSDB_NAMESPACE
int main(int argc, char** argv) {