From 85b2bde3dd9a3b30def867eb2cda656ebd352b0c Mon Sep 17 00:00:00 2001 From: Siying Dong Date: Fri, 12 Apr 2019 10:55:14 -0700 Subject: [PATCH] Still implement StatisticsImpl::measureTime() (#5181) Summary: Since Statistics::measureTime() is deprecated, StatisticsImpl::measureTime() is not implemented. We realized that users might have a wrapped Statistics implementation in which measureTime() is implemented as forwarded to StatisticsImpl, and causes assert failure. In order to make the change less intrusive, we implement StatisticsImpl::measureTime(). We will revisit whether we need to remove it after several releases. Also, add a test to make sure that a Statistics implementation using the old interface still works. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5181 Differential Revision: D14907089 Pulled By: siying fbshipit-source-id: 29b6202fd04e30ed6f6adcaeb1000e87f10d1e1a --- db/db_test2.cc | 42 +++++++++++++++++++++++++++++++++++++++++ monitoring/statistics.h | 7 +++++++ 2 files changed, 49 insertions(+) diff --git a/db/db_test2.cc b/db/db_test2.cc index 37382b2b8..388d3ccf5 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -3696,6 +3696,48 @@ TEST_F(DBTest2, MultiDBParallelOpenTest) { } #endif // OS_WIN +namespace { +class DummyOldStats : public Statistics { + public: + uint64_t getTickerCount(uint32_t /*ticker_type*/) const override { return 0; } + void recordTick(uint32_t /* ticker_type */, uint64_t /* count */) override { + num_rt++; + } + void setTickerCount(uint32_t /*ticker_type*/, uint64_t /*count*/) override {} + uint64_t getAndResetTickerCount(uint32_t /*ticker_type*/) override { + return 0; + } + void measureTime(uint32_t /*histogram_type*/, uint64_t /*count*/) override { + num_mt++; + } + void histogramData(uint32_t /*histogram_type*/, + rocksdb::HistogramData* const /*data*/) const override {} + std::string getHistogramString(uint32_t /*type*/) const override { + return ""; + } + bool HistEnabledForType(uint32_t /*type*/) const override { return false; } + std::string ToString() const override { return ""; } + int num_rt = 0; + int num_mt = 0; +}; +} // namespace + +TEST_F(DBTest2, OldStatsInterface) { + DummyOldStats* dos = new DummyOldStats(); + std::shared_ptr stats(dos); + Options options = CurrentOptions(); + options.create_if_missing = true; + options.statistics = stats; + Reopen(options); + + Put("foo", "bar"); + ASSERT_EQ("bar", Get("foo")); + ASSERT_OK(Flush()); + ASSERT_EQ("bar", Get("foo")); + + ASSERT_GT(dos->num_rt, 0); + ASSERT_GT(dos->num_mt, 0); +} } // namespace rocksdb int main(int argc, char** argv) { diff --git a/monitoring/statistics.h b/monitoring/statistics.h index fac01205f..952bf8cb4 100644 --- a/monitoring/statistics.h +++ b/monitoring/statistics.h @@ -53,6 +53,13 @@ class StatisticsImpl : public Statistics { virtual void setTickerCount(uint32_t ticker_type, uint64_t count) override; virtual uint64_t getAndResetTickerCount(uint32_t ticker_type) override; virtual void recordTick(uint32_t ticker_type, uint64_t count) override; + // The function is implemented for now for backward compatibility reason. + // In case a user explictly calls it, for example, they may have a wrapped + // Statistics object, passing the call to recordTick() into here, nothing + // will break. + void measureTime(uint32_t histogramType, uint64_t time) override { + recordInHistogram(histogramType, time); + } virtual void recordInHistogram(uint32_t histogram_type, uint64_t value) override;