From 508a09fd62bf058a2b6bd9ea9bcd8e66e5b240cc Mon Sep 17 00:00:00 2001
From: Andrew Kryczka <andrewkr@fb.com>
Date: Mon, 21 May 2018 11:09:00 -0700
Subject: [PATCH] Print histogram count and sum in statistics string

Summary:
Previously it only printed percentiles, even though our histogram keeps track of count and sum (and more). There have been many times we want to know more than the percentiles. For example, we currently want sum of "rocksdb.compression.times.nanos" and sum of "rocksdb.decompression.times.nanos", which would allow us to know the relative cost of compression vs decompression.

This PR adds count and sum to the string printed by `StatisticsImpl::ToString`. This is a bit risky as there are definitely parsers assuming the old format. I will mention it in HISTORY.md and hope for the best...
Closes https://github.com/facebook/rocksdb/pull/3863

Differential Revision: D8038831

Pulled By: ajkr

fbshipit-source-id: 0465b72e4b0cbf18ef965f4efe402601d16d5b5c
---
 HISTORY.md                   |  2 ++
 include/rocksdb/statistics.h |  2 ++
 monitoring/histogram.cc      |  2 ++
 monitoring/statistics.cc     | 14 ++++++++++----
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/HISTORY.md b/HISTORY.md
index e9698ab3b..c299a0505 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -1,5 +1,7 @@
 # Rocksdb Change Log
 ## Unreleased
+### Public API Change
+* For users of `Statistics` objects created via `CreateDBStatistics()`, the format of the string returned by its `ToString()` method has changed.
 
 ## 5.14.0 (5/16/2018)
 ### Public API Change
diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h
index 1a4f699db..30e79b099 100644
--- a/include/rocksdb/statistics.h
+++ b/include/rocksdb/statistics.h
@@ -619,6 +619,8 @@ struct HistogramData {
   // zero-initialize new members since old Statistics::histogramData()
   // implementations won't write them.
   double max = 0.0;
+  uint64_t count = 0;
+  uint64_t sum = 0;
 };
 
 enum StatsLevel {
diff --git a/monitoring/histogram.cc b/monitoring/histogram.cc
index 4970df7bd..d5ac07c90 100644
--- a/monitoring/histogram.cc
+++ b/monitoring/histogram.cc
@@ -235,6 +235,8 @@ void HistogramStat::Data(HistogramData * const data) const {
   data->max = static_cast<double>(max());
   data->average = Average();
   data->standard_deviation = StandardDeviation();
+  data->count = num();
+  data->sum = sum();
 }
 
 void HistogramImpl::Clear() {
diff --git a/monitoring/statistics.cc b/monitoring/statistics.cc
index 938704312..59ce3d9e0 100644
--- a/monitoring/statistics.cc
+++ b/monitoring/statistics.cc
@@ -169,11 +169,17 @@ std::string StatisticsImpl::ToString() const {
       char buffer[kTmpStrBufferSize];
       HistogramData hData;
       getHistogramImplLocked(h.first)->Data(&hData);
-      snprintf(
+      // don't handle failures - buffer should always be big enough and arguments
+      // should be provided correctly
+      int ret = snprintf(
           buffer, kTmpStrBufferSize,
-          "%s statistics Percentiles :=> 50 : %f 95 : %f 99 : %f 100 : %f\n",
-          h.second.c_str(), hData.median, hData.percentile95,
-          hData.percentile99, hData.max);
+          "%s P50 : %f P95 : %f P99 : %f P100 : %f COUNT : %" PRIu64 " SUM : %"
+          PRIu64 "\n", h.second.c_str(), hData.median, hData.percentile95,
+          hData.percentile99, hData.max, hData.count, hData.sum);
+      if (ret < 0 || ret >= kTmpStrBufferSize) {
+        assert(false);
+        continue;
+      }
       res.append(buffer);
     }
   }
-- 
GitLab