Skip to content

Commit

Permalink
Still implement StatisticsImpl::measureTime() (#5181)
Browse files Browse the repository at this point in the history
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: #5181

Differential Revision: D14907089

Pulled By: siying

fbshipit-source-id: 29b6202fd04e30ed6f6adcaeb1000e87f10d1e1a
  • Loading branch information
siying authored and facebook-github-bot committed Apr 12, 2019
1 parent 3189398 commit 85b2bde
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
42 changes: 42 additions & 0 deletions db/db_test2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Statistics> 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) {
Expand Down
7 changes: 7 additions & 0 deletions monitoring/statistics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 85b2bde

Please sign in to comment.