From 272cc7775186f9462ac076da99f701e976d13b0e Mon Sep 17 00:00:00 2001 From: mrambacher Date: Fri, 17 Sep 2021 06:54:40 -0700 Subject: [PATCH] Added a default Name method to Statistics (#8918) Summary: This keeps the implementations/API backward compatible. Implementations of Statistics will need to override this method (and be registered with the ObjectRegistry) in order to be created via CreateFromString. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8918 Reviewed By: pdillinger Differential Revision: D30958916 Pulled By: mrambacher fbshipit-source-id: 75b99a84e9e11fda2a9e8eff9ee1ef69a17517b2 --- HISTORY.md | 1 + include/rocksdb/statistics.h | 7 +++++ monitoring/statistics_test.cc | 49 +++++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index eccbe3c97..18ff661b0 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -27,6 +27,7 @@ ### Public API change * Remove obsolete implementation details FullKey and ParseFullKey from public API * Add a public API RateLimiter::GetTotalPendingRequests() for the total number of requests that are pending for bytes in the rate limiter. +* Made Statistics extend the Customizable class and added a CreateFromString method. Implementations of Statistics need to be registered with the ObjectRegistry and to implement a Name() method in order to be created via this method. * Extended `FlushJobInfo` and `CompactionJobInfo` in listener.h to provide information about the blob files generated by a flush/compaction and garbage collected during compaction in Integrated BlobDB. Added struct members `blob_file_addition_infos` and `blob_file_garbage_infos` that contain this information. * Extended parameter `output_file_names` of `CompactFiles` API to also include paths of the blob files generated by the compaction in Integrated BlobDB. diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h index 57d173cd6..c4f2d5154 100644 --- a/include/rocksdb/statistics.h +++ b/include/rocksdb/statistics.h @@ -576,6 +576,10 @@ class Statistics : public Customizable { static Status CreateFromString(const ConfigOptions& opts, const std::string& value, std::shared_ptr* result); + // Default name of empty, for backwards compatibility. Derived classes should + // override this method. + // This default implementation will likely be removed in a future release + const char* Name() const override { return ""; } virtual uint64_t getTickerCount(uint32_t tickerType) const = 0; virtual void histogramData(uint32_t type, HistogramData* const data) const = 0; @@ -607,6 +611,9 @@ class Statistics : public Customizable { // Resets all ticker and histogram stats virtual Status Reset() { return Status::NotSupported("Not implemented"); } +#ifndef ROCKSDB_LITE + using Customizable::ToString; +#endif // ROCKSDB_LITE // String representation of the statistic object. Must be thread-safe. virtual std::string ToString() const { // Do nothing by default diff --git a/monitoring/statistics_test.cc b/monitoring/statistics_test.cc index e497afcbe..cffa5054a 100644 --- a/monitoring/statistics_test.cc +++ b/monitoring/statistics_test.cc @@ -4,12 +4,14 @@ // (found in the LICENSE.Apache file in the root directory). // +#include "rocksdb/statistics.h" + #include "port/stack_trace.h" +#include "rocksdb/convenience.h" +#include "rocksdb/utilities/options_type.h" #include "test_util/testharness.h" #include "test_util/testutil.h" -#include "rocksdb/statistics.h" - namespace ROCKSDB_NAMESPACE { class StatisticsTest : public testing::Test {}; @@ -38,6 +40,49 @@ TEST_F(StatisticsTest, SanityHistograms) { } } +TEST_F(StatisticsTest, NoNameStats) { + static std::unordered_map no_name_opt_info = { +#ifndef ROCKSDB_LITE + {"inner", + OptionTypeInfo::AsCustomSharedPtr( + 0, OptionVerificationType::kByName, + OptionTypeFlags::kAllowNull | OptionTypeFlags::kCompareNever)}, +#endif // ROCKSDB_LITE + }; + + class DefaultNameStatistics : public Statistics { + public: + DefaultNameStatistics(const std::shared_ptr& stats = nullptr) + : inner(stats) { + RegisterOptions("", &inner, &no_name_opt_info); + } + + uint64_t getTickerCount(uint32_t /*tickerType*/) const override { + return 0; + } + void histogramData(uint32_t /*type*/, + HistogramData* const /*data*/) const override {} + void recordTick(uint32_t /*tickerType*/, uint64_t /*count*/) override {} + void setTickerCount(uint32_t /*tickerType*/, uint64_t /*count*/) override {} + uint64_t getAndResetTickerCount(uint32_t /*tickerType*/) override { + return 0; + } + std::shared_ptr inner; + }; + ConfigOptions options; + options.ignore_unsupported_options = false; + auto stats = std::make_shared(); + ASSERT_STREQ(stats->Name(), ""); +#ifndef ROCKSDB_LITE + ASSERT_EQ("", stats->ToString( + options)); // A stats with no name with have no options... + ASSERT_OK(stats->ConfigureFromString(options, "inner=")); + ASSERT_EQ("", stats->ToString( + options)); // A stats with no name with have no options... + ASSERT_NE(stats->inner, nullptr); + ASSERT_NE("", stats->inner->ToString(options)); // ... even if it does... +#endif // ROCKSDB_LITE +} } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) {