Eliminate duplicated property constants

Summary:
Before this diff, there were duplicated constants to refer to properties (user-
facing API had strings and InternalStats had an enum). I noticed these were
inconsistent in terms of which constants are provided, names of constants, and
documentation of constants. Overall it seemed annoying/error-prone to maintain
these duplicated constants.

So, this diff gets rid of InternalStats's constants and replaces them with a map
keyed on the user-facing constant. The value in that map contains a function
pointer to get the property value, so we don't need to do string matching while
holding db->mutex_. This approach has a side benefit of making many small
handler functions rather than a giant switch-statement.

Test Plan: db_properties_test passes, running "make commit-prereq -j32"

Reviewers: sdong, yhchiang, kradhakrishnan, IslamAbdelRahman, rven, anthony

Reviewed By: anthony

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D53253

db/db_impl.cc

@@ -494,23 +494,22 @@ void DBImpl::MaybeDumpStats() {
     last_stats_dump_time_microsec_ = now_micros;
 
 #ifndef ROCKSDB_LITE
-    bool tmp1 = false;
-    bool tmp2 = false;
-    DBPropertyType cf_property_type =
-        GetPropertyType(DB::Properties::kCFStats, &tmp1, &tmp2);
-    DBPropertyType db_property_type =
-        GetPropertyType(DB::Properties::kDBStats, &tmp1, &tmp2);
+    const DBPropertyInfo* cf_property_info =
+        GetPropertyInfo(DB::Properties::kCFStats);
+    assert(cf_property_info != nullptr);
+    const DBPropertyInfo* db_property_info =
+        GetPropertyInfo(DB::Properties::kDBStats);
+    assert(db_property_info != nullptr);
+
     std::string stats;
     {
       InstrumentedMutexLock l(&mutex_);
       for (auto cfd : *versions_->GetColumnFamilySet()) {
-        cfd->internal_stats()->GetStringProperty(cf_property_type,
-                                                 DB::Properties::kCFStats,
-                                                 &stats);
+        cfd->internal_stats()->GetStringProperty(
+            *cf_property_info, DB::Properties::kCFStats, &stats);
       }
-      default_cf_internal_stats_->GetStringProperty(db_property_type,
-                                                    DB::Properties::kDBStats,
-                                                    &stats);
+      default_cf_internal_stats_->GetStringProperty(
+          *db_property_info, DB::Properties::kDBStats, &stats);
     }
     Log(InfoLogLevel::WARN_LEVEL,
         db_options_.info_log, "------- DUMPING STATS -------");
@@ -4701,53 +4700,51 @@ const DBOptions& DBImpl::GetDBOptions() const { return db_options_; }
 
 bool DBImpl::GetProperty(ColumnFamilyHandle* column_family,
                          const Slice& property, std::string* value) {
-  bool is_int_property = false;
-  bool need_out_of_mutex = false;
-  DBPropertyType property_type =
-      GetPropertyType(property, &is_int_property, &need_out_of_mutex);
-
+  const DBPropertyInfo* property_info = GetPropertyInfo(property);
   value->clear();
   auto cfd = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family)->cfd();
-  if (is_int_property) {
+  if (property_info == nullptr) {
+    return false;
+  } else if (property_info->handle_int) {
     uint64_t int_value;
-    bool ret_value = GetIntPropertyInternal(
-        cfd, property_type, need_out_of_mutex, false, &int_value);
+    bool ret_value =
+        GetIntPropertyInternal(cfd, *property_info, false, &int_value);
     if (ret_value) {
       *value = ToString(int_value);
     }
     return ret_value;
-  } else {
+  } else if (property_info->handle_string) {
     InstrumentedMutexLock l(&mutex_);
-    return cfd->internal_stats()->GetStringProperty(property_type, property,
+    return cfd->internal_stats()->GetStringProperty(*property_info, property,
                                                     value);
   }
+  // Shouldn't reach here since exactly one of handle_string and handle_int
+  // should be non-nullptr.
+  assert(false);
+  return false;
 }
 
 bool DBImpl::GetIntProperty(ColumnFamilyHandle* column_family,
                             const Slice& property, uint64_t* value) {
-  bool is_int_property = false;
-  bool need_out_of_mutex = false;
-  DBPropertyType property_type =
-      GetPropertyType(property, &is_int_property, &need_out_of_mutex);
-  if (!is_int_property) {
+  const DBPropertyInfo* property_info = GetPropertyInfo(property);
+  if (property_info == nullptr || property_info->handle_int == nullptr) {
     return false;
   }
   auto cfd = reinterpret_cast<ColumnFamilyHandleImpl*>(column_family)->cfd();
-  return GetIntPropertyInternal(cfd, property_type, need_out_of_mutex, false,
-                                value);
+  return GetIntPropertyInternal(cfd, *property_info, false, value);
 }
 
 bool DBImpl::GetIntPropertyInternal(ColumnFamilyData* cfd,
-                                    DBPropertyType property_type,
-                                    bool need_out_of_mutex, bool is_locked,
-                                    uint64_t* value) {
-  if (!need_out_of_mutex) {
+                                    const DBPropertyInfo& property_info,
+                                    bool is_locked, uint64_t* value) {
+  assert(property_info.handle_int != nullptr);
+  if (!property_info.need_out_of_mutex) {
     if (is_locked) {
       mutex_.AssertHeld();
-      return cfd->internal_stats()->GetIntProperty(property_type, value, this);
+      return cfd->internal_stats()->GetIntProperty(property_info, value, this);
     } else {
       InstrumentedMutexLock l(&mutex_);
-      return cfd->internal_stats()->GetIntProperty(property_type, value, this);
+      return cfd->internal_stats()->GetIntProperty(property_info, value, this);
     }
   } else {
     SuperVersion* sv = nullptr;
@@ -4758,7 +4755,7 @@ bool DBImpl::GetIntPropertyInternal(ColumnFamilyData* cfd,
     }
 
     bool ret = cfd->internal_stats()->GetIntPropertyOutOfMutex(
-        property_type, sv->current, value);
+        property_info, sv->current, value);
 
     if (!is_locked) {
       ReturnAndCleanupSuperVersion(cfd, sv);
@@ -4770,11 +4767,8 @@ bool DBImpl::GetIntPropertyInternal(ColumnFamilyData* cfd,
 
 bool DBImpl::GetAggregatedIntProperty(const Slice& property,
                                       uint64_t* aggregated_value) {
-  bool need_out_of_mutex;
-  bool is_int_property;
-  DBPropertyType property_type =
-      GetPropertyType(property, &is_int_property, &need_out_of_mutex);
-  if (!is_int_property) {
+  const DBPropertyInfo* property_info = GetPropertyInfo(property);
+  if (property_info == nullptr || property_info->handle_int == nullptr) {
     return false;
   }
 
@@ -4784,8 +4778,7 @@ bool DBImpl::GetAggregatedIntProperty(const Slice& property,
     InstrumentedMutexLock l(&mutex_);
     uint64_t value;
     for (auto* cfd : *versions_->GetColumnFamilySet()) {
-      if (GetIntPropertyInternal(cfd, property_type, need_out_of_mutex, true,
-                                 &value)) {
+      if (GetIntPropertyInternal(cfd, *property_info, true, &value)) {
         sum += value;
       } else {
         return false;

db/db_impl.h

@@ -893,9 +893,8 @@ class DBImpl : public DB {
                  bool* value_found = nullptr);
 
   bool GetIntPropertyInternal(ColumnFamilyData* cfd,
-                              DBPropertyType property_type,
-                              bool need_out_of_mutex, bool is_locked,
-                              uint64_t* value);
+                              const DBPropertyInfo& property_info,
+                              bool is_locked, uint64_t* value);
 
   bool HasPendingManualCompaction();
   bool HasExclusiveManualCompaction();

db/db_properties_test.cc

@@ -226,6 +226,18 @@ void GetExpectedTableProperties(TableProperties* expected_tp,
 }
 }  // anonymous namespace
 
+TEST_F(DBPropertiesTest, ValidatePropertyInfo) {
+  for (const auto& ppt_name_and_info : InternalStats::ppt_name_to_info) {
+    // If C++ gets a std::string_literal, this would be better to check at
+    // compile-time using static_assert.
+    ASSERT_TRUE(ppt_name_and_info.first.empty() ||
+                !isdigit(ppt_name_and_info.first.back()));
+
+    ASSERT_TRUE((ppt_name_and_info.second.handle_string == nullptr) !=
+                (ppt_name_and_info.second.handle_int == nullptr));
+  }
+}
+
 TEST_F(DBPropertiesTest, AggregatedTableProperties) {
   for (int kTableCount = 40; kTableCount <= 100; kTableCount += 30) {
     const int kKeysPerTable = 100;

db/internal_stats.cc

@@ -15,6 +15,7 @@
 #include <inttypes.h>
 #include <string>
 #include <algorithm>
+#include <utility>
 #include <vector>
 #include "db/column_family.h"
 
@@ -81,7 +82,21 @@ void PrintLevelStats(char* buf, size_t len, const std::string& name,
            stats.count == 0 ? 0 : stats.micros / kMicrosInSec / stats.count,
            num_input_records.c_str(), num_dropped_records.c_str());
 }
+
+// Assumes that trailing numbers represent an optional argument. This requires
+// property names to not end with numbers.
+std::pair<Slice, Slice> GetPropertyNameAndArg(const Slice& property) {
+  Slice name = property, arg = property;
+  size_t sfx_len = 0;
+  while (sfx_len < property.size() &&
+         isdigit(property[property.size() - sfx_len - 1])) {
+    ++sfx_len;
+  }
+  name.remove_suffix(sfx_len);
+  arg.remove_prefix(property.size() - sfx_len);
+  return {name, arg};
 }
+}  // anonymous namespace
 
 static const std::string rocksdb_prefix = "rocksdb.";
 
@@ -139,7 +154,7 @@ const std::string DB::Properties::kLevelStats = rocksdb_prefix + levelstats;
 const std::string DB::Properties::kNumImmutableMemTable =
                       rocksdb_prefix + num_immutable_mem_table;
 const std::string DB::Properties::kNumImmutableMemTableFlushed =
-                      rocksdb_prefix + num_immutable_mem_table_flushed;
+    rocksdb_prefix + num_immutable_mem_table_flushed;
 const std::string DB::Properties::kMemTableFlushPending =
                       rocksdb_prefix + mem_table_flush_pending;
 const std::string DB::Properties::kCompactionPending =
@@ -188,294 +203,360 @@ const std::string DB::Properties::kAggregatedTableProperties =
 const std::string DB::Properties::kAggregatedTablePropertiesAtLevel =
     rocksdb_prefix + aggregated_table_properties_at_level;
 
-DBPropertyType GetPropertyType(const Slice& property, bool* is_int_property,
-                               bool* need_out_of_mutex) {
-  assert(is_int_property != nullptr);
-  assert(need_out_of_mutex != nullptr);
-  Slice in = property;
-  Slice prefix(rocksdb_prefix);
-  *need_out_of_mutex = false;
-  *is_int_property = false;
-  if (!in.starts_with(prefix)) {
-    return kUnknown;
-  }
-  in.remove_prefix(prefix.size());
-
-  if (in.starts_with(num_files_at_level_prefix)) {
-    return kNumFilesAtLevel;
-  } else if (in == levelstats) {
-    return kLevelStats;
-  } else if (in == allstats) {
-    return kStats;
-  } else if (in == cfstats) {
-    return kCFStats;
-  } else if (in == dbstats) {
-    return kDBStats;
-  } else if (in == sstables) {
-    return kSsTables;
-  } else if (in == aggregated_table_properties) {
-    return kAggregatedTableProperties;
-  } else if (in.starts_with(aggregated_table_properties_at_level)) {
-    return kAggregatedTablePropertiesAtLevel;
+const std::unordered_map<std::string,
+                         DBPropertyInfo> InternalStats::ppt_name_to_info = {
+    {DB::Properties::kNumFilesAtLevelPrefix,
+     {false, &InternalStats::HandleNumFilesAtLevel, nullptr}},
+    {DB::Properties::kLevelStats,
+     {false, &InternalStats::HandleLevelStats, nullptr}},
+    {DB::Properties::kStats, {false, &InternalStats::HandleStats, nullptr}},
+    {DB::Properties::kCFStats, {false, &InternalStats::HandleCFStats, nullptr}},
+    {DB::Properties::kDBStats, {false, &InternalStats::HandleDBStats, nullptr}},
+    {DB::Properties::kSSTables,
+     {false, &InternalStats::HandleSsTables, nullptr}},
+    {DB::Properties::kAggregatedTableProperties,
+     {false, &InternalStats::HandleAggregatedTableProperties, nullptr}},
+    {DB::Properties::kAggregatedTablePropertiesAtLevel,
+     {false, &InternalStats::HandleAggregatedTablePropertiesAtLevel, nullptr}},
+    {DB::Properties::kNumImmutableMemTable,
+     {false, nullptr, &InternalStats::HandleNumImmutableMemTable}},
+    {DB::Properties::kNumImmutableMemTableFlushed,
+     {false, nullptr, &InternalStats::HandleNumImmutableMemTableFlushed}},
+    {DB::Properties::kMemTableFlushPending,
+     {false, nullptr, &InternalStats::HandleMemTableFlushPending}},
+    {DB::Properties::kCompactionPending,
+     {false, nullptr, &InternalStats::HandleCompactionPending}},
+    {DB::Properties::kBackgroundErrors,
+     {false, nullptr, &InternalStats::HandleBackgroundErrors}},
+    {DB::Properties::kCurSizeActiveMemTable,
+     {false, nullptr, &InternalStats::HandleCurSizeActiveMemTable}},
+    {DB::Properties::kCurSizeAllMemTables,
+     {false, nullptr, &InternalStats::HandleCurSizeAllMemTables}},
+    {DB::Properties::kSizeAllMemTables,
+     {false, nullptr, &InternalStats::HandleSizeAllMemTables}},
+    {DB::Properties::kNumEntriesActiveMemTable,
+     {false, nullptr, &InternalStats::HandleNumEntriesActiveMemTable}},
+    {DB::Properties::kNumEntriesImmMemTables,
+     {false, nullptr, &InternalStats::HandleNumEntriesImmMemTables}},
+    {DB::Properties::kNumDeletesActiveMemTable,
+     {false, nullptr, &InternalStats::HandleNumDeletesActiveMemTable}},
+    {DB::Properties::kNumDeletesImmMemTables,
+     {false, nullptr, &InternalStats::HandleNumDeletesImmMemTables}},
+    {DB::Properties::kEstimateNumKeys,
+     {false, nullptr, &InternalStats::HandleEstimateNumKeys}},
+    {DB::Properties::kEstimateTableReadersMem,
+     {true, nullptr, &InternalStats::HandleEstimateTableReadersMem}},
+    {DB::Properties::kIsFileDeletionsEnabled,
+     {false, nullptr, &InternalStats::HandleIsFileDeletionsEnabled}},
+    {DB::Properties::kNumSnapshots,
+     {false, nullptr, &InternalStats::HandleNumSnapshots}},
+    {DB::Properties::kOldestSnapshotTime,
+     {false, nullptr, &InternalStats::HandleOldestSnapshotTime}},
+    {DB::Properties::kNumLiveVersions,
+     {false, nullptr, &InternalStats::HandleNumLiveVersions}},
+    {DB::Properties::kEstimateLiveDataSize,
+     {true, nullptr, &InternalStats::HandleEstimateLiveDataSize}},
+    {DB::Properties::kBaseLevel,
+     {false, nullptr, &InternalStats::HandleBaseLevel}},
+    {DB::Properties::kTotalSstFilesSize,
+     {false, nullptr, &InternalStats::HandleTotalSstFilesSize}},
+    {DB::Properties::kEstimatePendingCompactionBytes,
+     {false, nullptr, &InternalStats::HandleEstimatePendingCompactionBytes}},
+    {DB::Properties::kNumRunningFlushes,
+     {false, nullptr, &InternalStats::HandleNumRunningFlushes}},
+    {DB::Properties::kNumRunningCompactions,
+     {false, nullptr, &InternalStats::HandleNumRunningCompactions}},
+};
+
+const DBPropertyInfo* GetPropertyInfo(const Slice& property) {
+  std::string ppt_name = GetPropertyNameAndArg(property).first.ToString();
+  auto ppt_info_iter = InternalStats::ppt_name_to_info.find(ppt_name);
+  if (ppt_info_iter == InternalStats::ppt_name_to_info.end()) {
+    return nullptr;
   }
+  return &ppt_info_iter->second;
+}
 
-  *is_int_property = true;
-  if (in == num_immutable_mem_table) {
-    return kNumImmutableMemTable;
-  } else if (in == num_immutable_mem_table_flushed) {
-    return kNumImmutableMemTableFlushed;
-  } else if (in == mem_table_flush_pending) {
-    return kMemtableFlushPending;
-  } else if (in == compaction_pending) {
-    return kCompactionPending;
-  } else if (in == background_errors) {
-    return kBackgroundErrors;
-  } else if (in == cur_size_active_mem_table) {
-    return kCurSizeActiveMemTable;
-  } else if (in == cur_size_all_mem_tables) {
-    return kCurSizeAllMemTables;
-  } else if (in == size_all_mem_tables) {
-    return kSizeAllMemTables;
-  } else if (in == num_entries_active_mem_table) {
-    return kNumEntriesInMutableMemtable;
-  } else if (in == num_entries_imm_mem_tables) {
-    return kNumEntriesInImmutableMemtable;
-  } else if (in == num_deletes_active_mem_table) {
-    return kNumDeletesInMutableMemtable;
-  } else if (in == num_deletes_imm_mem_tables) {
-    return kNumDeletesInImmutableMemtable;
-  } else if (in == estimate_num_keys) {
-    return kEstimatedNumKeys;
-  } else if (in == estimate_table_readers_mem) {
-    *need_out_of_mutex = true;
-    return kEstimatedUsageByTableReaders;
-  } else if (in == is_file_deletions_enabled) {
-    return kIsFileDeletionEnabled;
-  } else if (in == num_snapshots) {
-    return kNumSnapshots;
-  } else if (in == oldest_snapshot_time) {
-    return kOldestSnapshotTime;
-  } else if (in == num_live_versions) {
-    return kNumLiveVersions;
-  } else if (in == estimate_live_data_size) {
-    *need_out_of_mutex = true;
-    return kEstimateLiveDataSize;
-  } else if (in == base_level) {
-    return kBaseLevel;
-  } else if (in == total_sst_files_size) {
-    return kTotalSstFilesSize;
-  } else if (in == estimate_pending_comp_bytes) {
-    return kEstimatePendingCompactionBytes;
-  } else if (in == num_running_flushes) {
-    return kNumRunningFlushes;
-  } else if (in == num_running_compactions) {
-    return kNumRunningCompactions;
-  }
-  return kUnknown;
+bool InternalStats::GetStringProperty(const DBPropertyInfo& property_info,
+                                      const Slice& property,
+                                      std::string* value) {
+  assert(value != nullptr);
+  assert(property_info.handle_string != nullptr);
+  Slice arg = GetPropertyNameAndArg(property).second;
+  return (this->*(property_info.handle_string))(value, arg);
 }
 
-bool InternalStats::GetIntPropertyOutOfMutex(DBPropertyType property_type,
-                                             Version* version,
-                                             uint64_t* value) const {
+bool InternalStats::GetIntProperty(const DBPropertyInfo& property_info,
+                                   uint64_t* value, DBImpl* db) {
   assert(value != nullptr);
+  assert(property_info.handle_int != nullptr &&
+         !property_info.need_out_of_mutex);
+  db->mutex_.AssertHeld();
+  return (this->*(property_info.handle_int))(value, db, nullptr /* version */);
+}
+
+bool InternalStats::GetIntPropertyOutOfMutex(
+    const DBPropertyInfo& property_info, Version* version, uint64_t* value) {
+  assert(value != nullptr);
+  assert(property_info.handle_int != nullptr &&
+         property_info.need_out_of_mutex);
+  return (this->*(property_info.handle_int))(value, nullptr /* db */, version);
+}
+
+bool InternalStats::HandleNumFilesAtLevel(std::string* value, Slice suffix) {
+  uint64_t level;
   const auto* vstorage = cfd_->current()->storage_info();
+  bool ok = ConsumeDecimalNumber(&suffix, &level) && suffix.empty();
+  if (!ok || static_cast<int>(level) >= number_levels_) {
+    return false;
+  } else {
+    char buf[100];
+    snprintf(buf, sizeof(buf), "%d",
+             vstorage->NumLevelFiles(static_cast<int>(level)));
+    *value = buf;
+    return true;
+  }
+}
+
+bool InternalStats::HandleLevelStats(std::string* value, Slice suffix) {
+  char buf[1000];
+  const auto* vstorage = cfd_->current()->storage_info();
+  snprintf(buf, sizeof(buf),
+           "Level Files Size(MB)\n"
+           "--------------------\n");
+  value->append(buf);
 
-  switch (property_type) {
-    case kEstimatedUsageByTableReaders:
-      *value = (version == nullptr) ?
-        0 : version->GetMemoryUsageByTableReaders();
-      return true;
-    case kEstimateLiveDataSize:
-      *value = vstorage->EstimateLiveDataSize();
-      return true;
-    default:
-      return false;
+  for (int level = 0; level < number_levels_; level++) {
+    snprintf(buf, sizeof(buf), "%3d %8d %8.0f\n", level,
+             vstorage->NumLevelFiles(level),
+             vstorage->NumLevelBytes(level) / kMB);
+    value->append(buf);
   }
+  return true;
 }
 
-bool InternalStats::GetStringProperty(DBPropertyType property_type,
-                                      const Slice& property,
-                                      std::string* value) {
-  assert(value != nullptr);
+bool InternalStats::HandleStats(std::string* value, Slice suffix) {
+  if (!HandleCFStats(value, suffix)) {
+    return false;
+  }
+  if (!HandleDBStats(value, suffix)) {
+    return false;
+  }
+  return true;
+}
+
+bool InternalStats::HandleCFStats(std::string* value, Slice suffix) {
+  DumpCFStats(value);
+  return true;
+}
+
+bool InternalStats::HandleDBStats(std::string* value, Slice suffix) {
+  DumpDBStats(value);
+  return true;
+}
+
+bool InternalStats::HandleSsTables(std::string* value, Slice suffix) {
   auto* current = cfd_->current();
-  const auto* vstorage = current->storage_info();
-  Slice in = property;
-
-  switch (property_type) {
-    case kNumFilesAtLevel: {
-      in.remove_prefix(strlen("rocksdb.num-files-at-level"));
-      uint64_t level;
-      bool ok = ConsumeDecimalNumber(&in, &level) && in.empty();
-      if (!ok || (int)level >= number_levels_) {
-        return false;
-      } else {
-        char buf[100];
-        snprintf(buf, sizeof(buf), "%d",
-                 vstorage->NumLevelFiles(static_cast<int>(level)));
-        *value = buf;
-        return true;
-      }
-    }
-    case kLevelStats: {
-      char buf[1000];
-      snprintf(buf, sizeof(buf),
-               "Level Files Size(MB)\n"
-               "--------------------\n");
-      value->append(buf);
+  *value = current->DebugString();
+  return true;
+}
 
-      for (int level = 0; level < number_levels_; level++) {
-        snprintf(buf, sizeof(buf), "%3d %8d %8.0f\n", level,
-                 vstorage->NumLevelFiles(level),
-                 vstorage->NumLevelBytes(level) / kMB);
-        value->append(buf);
-      }
-      return true;
-    }
-    case kStats: {
-      if (!GetStringProperty(kCFStats, DB::Properties::kCFStats, value)) {
-        return false;
-      }
-      if (!GetStringProperty(kDBStats, DB::Properties::kDBStats, value)) {
-        return false;
-      }
-      return true;
-    }
-    case kCFStats: {
-      DumpCFStats(value);
-      return true;
-    }
-    case kDBStats: {
-      DumpDBStats(value);
-      return true;
-    }
-    case kSsTables:
-      *value = current->DebugString();
-      return true;
-    case kAggregatedTableProperties: {
-      std::shared_ptr<const TableProperties> tp;
-      auto s = cfd_->current()->GetAggregatedTableProperties(&tp);
-      if (!s.ok()) {
-        return false;
-      }
-      *value = tp->ToString();
-      return true;
-    }
-    case kAggregatedTablePropertiesAtLevel: {
-      in.remove_prefix(
-          DB::Properties::kAggregatedTablePropertiesAtLevel.length());
-      uint64_t level;
-      bool ok = ConsumeDecimalNumber(&in, &level) && in.empty();
-      if (!ok || static_cast<int>(level) >= number_levels_) {
-        return false;
-      }
-      std::shared_ptr<const TableProperties> tp;
-      auto s = cfd_->current()->GetAggregatedTableProperties(
-          &tp, static_cast<int>(level));
-      if (!s.ok()) {
-        return false;
-      }
-      *value = tp->ToString();
-      return true;
-    }
-    default:
-      return false;
+bool InternalStats::HandleAggregatedTableProperties(std::string* value,
+                                                    Slice suffix) {
+  std::shared_ptr<const TableProperties> tp;
+  auto s = cfd_->current()->GetAggregatedTableProperties(&tp);
+  if (!s.ok()) {
+    return false;
   }
+  *value = tp->ToString();
+  return true;
 }
 
-bool InternalStats::GetIntProperty(DBPropertyType property_type,
-                                   uint64_t* value, DBImpl* db) const {
-  db->mutex_.AssertHeld();
+bool InternalStats::HandleAggregatedTablePropertiesAtLevel(std::string* value,
+                                                           Slice suffix) {
+  uint64_t level;
+  bool ok = ConsumeDecimalNumber(&suffix, &level) && suffix.empty();
+  if (!ok || static_cast<int>(level) >= number_levels_) {
+    return false;
+  }
+  std::shared_ptr<const TableProperties> tp;
+  auto s = cfd_->current()->GetAggregatedTableProperties(
+      &tp, static_cast<int>(level));
+  if (!s.ok()) {
+    return false;
+  }
+  *value = tp->ToString();
+  return true;
+}
+
+bool InternalStats::HandleNumImmutableMemTable(uint64_t* value, DBImpl* db,
+                                               Version* version) {
+  *value = cfd_->imm()->NumNotFlushed();
+  return true;
+}
+
+bool InternalStats::HandleNumImmutableMemTableFlushed(uint64_t* value,
+                                                      DBImpl* db,
+                                                      Version* version) {
+  *value = cfd_->imm()->NumFlushed();
+  return true;
+}
+
+bool InternalStats::HandleMemTableFlushPending(uint64_t* value, DBImpl* db,
+                                               Version* version) {
+  // Return number of mem tables that are ready to flush (made immutable)
+  *value = (cfd_->imm()->IsFlushPending() ? 1 : 0);
+  return true;
+}
+
+bool InternalStats::HandleNumRunningFlushes(uint64_t* value, DBImpl* db,
+                                            Version* version) {
+  *value = db->num_running_flushes();
+  return true;
+}
+
+bool InternalStats::HandleCompactionPending(uint64_t* value, DBImpl* db,
+                                            Version* version) {
+  // 1 if the system already determines at least one compaction is needed.
+  // 0 otherwise,
   const auto* vstorage = cfd_->current()->storage_info();
+  *value = (cfd_->compaction_picker()->NeedsCompaction(vstorage) ? 1 : 0);
+  return true;
+}
 
-  switch (property_type) {
-    case kNumImmutableMemTable:
-      *value = cfd_->imm()->NumNotFlushed();
-      return true;
-    case kNumImmutableMemTableFlushed:
-      *value = cfd_->imm()->NumFlushed();
-      return true;
-    case kMemtableFlushPending:
-      // Return number of mem tables that are ready to flush (made immutable)
-      *value = (cfd_->imm()->IsFlushPending() ? 1 : 0);
-      return true;
-    case kNumRunningFlushes:
-      *value = db->num_running_flushes();
-      return true;
-    case kCompactionPending:
-      // 1 if the system already determines at least one compaction is needed.
-      // 0 otherwise,
-      *value = (cfd_->compaction_picker()->NeedsCompaction(vstorage) ? 1 : 0);
-      return true;
-    case kNumRunningCompactions:
-      *value = db->num_running_compactions_;
-      return true;
-    case kBackgroundErrors:
-      // Accumulated number of  errors in background flushes or compactions.
-      *value = GetBackgroundErrorCount();
-      return true;
-    case kCurSizeActiveMemTable:
-      // Current size of the active memtable
-      *value = cfd_->mem()->ApproximateMemoryUsage();
-      return true;
-    case kCurSizeAllMemTables:
-      // Current size of the active memtable + immutable memtables
-      *value = cfd_->mem()->ApproximateMemoryUsage() +
-               cfd_->imm()->ApproximateUnflushedMemTablesMemoryUsage();
-      return true;
-    case kSizeAllMemTables:
-      *value = cfd_->mem()->ApproximateMemoryUsage() +
-               cfd_->imm()->ApproximateMemoryUsage();
-      return true;
-    case kNumEntriesInMutableMemtable:
-      // Current number of entires in the active memtable
-      *value = cfd_->mem()->num_entries();
-      return true;
-    case kNumEntriesInImmutableMemtable:
-      // Current number of entries in the immutable memtables
-      *value = cfd_->imm()->current()->GetTotalNumEntries();
-      return true;
-    case kNumDeletesInMutableMemtable:
-      // Current number of entires in the active memtable
-      *value = cfd_->mem()->num_deletes();
-      return true;
-    case kNumDeletesInImmutableMemtable:
-      // Current number of entries in the immutable memtables
-      *value = cfd_->imm()->current()->GetTotalNumDeletes();
-      return true;
-    case kEstimatedNumKeys:
-      // Estimate number of entries in the column family:
-      // Use estimated entries in tables + total entries in memtables.
-      *value = cfd_->mem()->num_entries() +
-               cfd_->imm()->current()->GetTotalNumEntries() -
-               (cfd_->mem()->num_deletes() +
-                cfd_->imm()->current()->GetTotalNumDeletes()) *
-                   2 +
-               vstorage->GetEstimatedActiveKeys();
-      return true;
-    case kNumSnapshots:
-      *value = db->snapshots().count();
-      return true;
-    case kOldestSnapshotTime:
-      *value = static_cast<uint64_t>(db->snapshots().GetOldestSnapshotTime());
-      return true;
-    case kNumLiveVersions:
-      *value = cfd_->GetNumLiveVersions();
-      return true;
-    case kIsFileDeletionEnabled:
-      *value = db->IsFileDeletionsEnabled();
-      return true;
-    case kBaseLevel:
-      *value = vstorage->base_level();
-      return true;
-    case kTotalSstFilesSize:
-      *value = cfd_->GetTotalSstFilesSize();
-      return true;
-    case kEstimatePendingCompactionBytes:
-      *value = vstorage->estimated_compaction_needed_bytes();
-      return true;
-    default:
-      return false;
-  }
+bool InternalStats::HandleNumRunningCompactions(uint64_t* value, DBImpl* db,
+                                                Version* version) {
+  *value = db->num_running_compactions_;
+  return true;
+}
+
+bool InternalStats::HandleBackgroundErrors(uint64_t* value, DBImpl* db,
+                                           Version* version) {
+  // Accumulated number of  errors in background flushes or compactions.
+  *value = GetBackgroundErrorCount();
+  return true;
+}
+
+bool InternalStats::HandleCurSizeActiveMemTable(uint64_t* value, DBImpl* db,
+                                                Version* version) {
+  // Current size of the active memtable
+  *value = cfd_->mem()->ApproximateMemoryUsage();
+  return true;
+}
+
+bool InternalStats::HandleCurSizeAllMemTables(uint64_t* value, DBImpl* db,
+                                              Version* version) {
+  // Current size of the active memtable + immutable memtables
+  *value = cfd_->mem()->ApproximateMemoryUsage() +
+           cfd_->imm()->ApproximateUnflushedMemTablesMemoryUsage();
+  return true;
+}
+
+bool InternalStats::HandleSizeAllMemTables(uint64_t* value, DBImpl* db,
+                                           Version* version) {
+  *value = cfd_->mem()->ApproximateMemoryUsage() +
+           cfd_->imm()->ApproximateMemoryUsage();
+  return true;
+}
+
+bool InternalStats::HandleNumEntriesActiveMemTable(uint64_t* value, DBImpl* db,
+                                                   Version* version) {
+  // Current number of entires in the active memtable
+  *value = cfd_->mem()->num_entries();
+  return true;
+}
+
+bool InternalStats::HandleNumEntriesImmMemTables(uint64_t* value, DBImpl* db,
+                                                 Version* version) {
+  // Current number of entries in the immutable memtables
+  *value = cfd_->imm()->current()->GetTotalNumEntries();
+  return true;
+}
+
+bool InternalStats::HandleNumDeletesActiveMemTable(uint64_t* value, DBImpl* db,
+                                                   Version* version) {
+  // Current number of entires in the active memtable
+  *value = cfd_->mem()->num_deletes();
+  return true;
+}
+
+bool InternalStats::HandleNumDeletesImmMemTables(uint64_t* value, DBImpl* db,
+                                                 Version* version) {
+  // Current number of entries in the immutable memtables
+  *value = cfd_->imm()->current()->GetTotalNumDeletes();
+  return true;
+}
+
+bool InternalStats::HandleEstimateNumKeys(uint64_t* value, DBImpl* db,
+                                          Version* version) {
+  // Estimate number of entries in the column family:
+  // Use estimated entries in tables + total entries in memtables.
+  const auto* vstorage = cfd_->current()->storage_info();
+  *value = cfd_->mem()->num_entries() +
+           cfd_->imm()->current()->GetTotalNumEntries() -
+           (cfd_->mem()->num_deletes() +
+            cfd_->imm()->current()->GetTotalNumDeletes()) *
+               2 +
+           vstorage->GetEstimatedActiveKeys();
+  return true;
+}
+
+bool InternalStats::HandleNumSnapshots(uint64_t* value, DBImpl* db,
+                                       Version* version) {
+  *value = db->snapshots().count();
+  return true;
+}
+
+bool InternalStats::HandleOldestSnapshotTime(uint64_t* value, DBImpl* db,
+                                             Version* version) {
+  *value = static_cast<uint64_t>(db->snapshots().GetOldestSnapshotTime());
+  return true;
+}
+
+bool InternalStats::HandleNumLiveVersions(uint64_t* value, DBImpl* db,
+                                          Version* version) {
+  *value = cfd_->GetNumLiveVersions();
+  return true;
+}
+
+bool InternalStats::HandleIsFileDeletionsEnabled(uint64_t* value, DBImpl* db,
+                                                 Version* version) {
+  *value = db->IsFileDeletionsEnabled();
+  return true;
+}
+
+bool InternalStats::HandleBaseLevel(uint64_t* value, DBImpl* db,
+                                    Version* version) {
+  const auto* vstorage = cfd_->current()->storage_info();
+  *value = vstorage->base_level();
+  return true;
+}
+
+bool InternalStats::HandleTotalSstFilesSize(uint64_t* value, DBImpl* db,
+                                            Version* version) {
+  *value = cfd_->GetTotalSstFilesSize();
+  return true;
+}
+
+bool InternalStats::HandleEstimatePendingCompactionBytes(uint64_t* value,
+                                                         DBImpl* db,
+                                                         Version* version) {
+  const auto* vstorage = cfd_->current()->storage_info();
+  *value = vstorage->estimated_compaction_needed_bytes();
+  return true;
+}
+
+bool InternalStats::HandleEstimateTableReadersMem(uint64_t* value, DBImpl* db,
+                                                  Version* version) {
+  *value = (version == nullptr) ? 0 : version->GetMemoryUsageByTableReaders();
+  return true;
+}
+
+bool InternalStats::HandleEstimateLiveDataSize(uint64_t* value, DBImpl* db,
+                                               Version* version) {
+  const auto* vstorage = cfd_->current()->storage_info();
+  *value = vstorage->EstimateLiveDataSize();
+  return true;
 }
 
 void InternalStats::DumpDBStats(std::string* value) {
@@ -760,10 +841,7 @@ void InternalStats::DumpCFStats(std::string* value) {
 
 #else
 
-DBPropertyType GetPropertyType(const Slice& property, bool* is_int_property,
-                               bool* need_out_of_mutex) {
-  return kUnknown;
-}
+const DBPropertyInfo* GetPropertyInfo(const Slice& property) { return nullptr; }
 
 #endif  // !ROCKSDB_LITE
 

db/internal_stats.h

@@ -21,63 +21,29 @@ namespace rocksdb {
 class MemTableList;
 class DBImpl;
 
-// IMPORTANT: If you add a new property here, also add it to the list in
-//            include/rocksdb/db.h
-enum DBPropertyType : uint32_t {
-  kUnknown,
-  kNumFilesAtLevel,  // Number of files at a specific level
-  kLevelStats,       // Return number of files and total sizes of each level
-  kCFStats,          // Return general statitistics of CF
-  kDBStats,          // Return general statitistics of DB
-  kStats,            // Return general statitistics of both DB and CF
-  kSsTables,         // Return a human readable string of current SST files
-  kStartIntTypes,    // ---- Dummy value to indicate the start of integer values
-  kNumImmutableMemTable,         // Return number of immutable mem tables that
-                                 // have not been flushed.
-  kNumImmutableMemTableFlushed,  // Return number of immutable mem tables
-                                 // in memory that have already been flushed
-  kMemtableFlushPending,         // Return 1 if mem table flushing is pending,
-                                 // otherwise 0.
-  kNumRunningFlushes,      // Return the number of currently running flushes.
-  kCompactionPending,      // Return 1 if a compaction is pending. Otherwise 0.
-  kNumRunningCompactions,  // Return the number of currently running
-                           // compactions.
-  kBackgroundErrors,       // Return accumulated background errors encountered.
-  kCurSizeActiveMemTable,  // Return current size of the active memtable
-  kCurSizeAllMemTables,    // Return current size of unflushed
-                           // (active + immutable) memtables
-  kSizeAllMemTables,       // Return current size of all (active + immutable
-                           // + pinned) memtables
-  kNumEntriesInMutableMemtable,    // Return number of deletes in the mutable
-                                   // memtable.
-  kNumEntriesInImmutableMemtable,  // Return sum of number of entries in all
-                                   // the immutable mem tables.
-  kNumDeletesInMutableMemtable,    // Return number of deletion entries in the
-                                   // mutable memtable.
-  kNumDeletesInImmutableMemtable,  // Return the total number of deletion
-                                   // entries in all the immutable mem tables.
-  kEstimatedNumKeys,  // Estimated total number of keys in the database.
-  kEstimatedUsageByTableReaders,  // Estimated memory by table readers.
-  kIsFileDeletionEnabled,         // Equals disable_delete_obsolete_files_,
-                                  // 0 means file deletions enabled
-  kNumSnapshots,                  // Number of snapshots in the system
-  kOldestSnapshotTime,            // Unix timestamp of the first snapshot
-  kNumLiveVersions,
-  kEstimateLiveDataSize,            // Estimated amount of live data in bytes
-  kTotalSstFilesSize,               // Total size of all sst files.
-  kBaseLevel,                       // The level that L0 data is compacted to
-  kEstimatePendingCompactionBytes,  // Estimated bytes to compaction
-  kAggregatedTableProperties,  // Return a string that contains the aggregated
-                               // table properties.
-  kAggregatedTablePropertiesAtLevel,  // Return a string that contains the
-                                      // aggregated
-  // table properties at the specified level.
+// Config for retrieving a property's value.
+struct DBPropertyInfo {
+  bool need_out_of_mutex;
+
+  // gcc had an internal error for initializing union of pointer-to-member-
+  // functions. Workaround is to populate exactly one of the following function
+  // pointers with a non-nullptr value.
+
+  // @param value Value-result argument for storing the property's string value
+  // @param suffix Argument portion of the property. For example, suffix would
+  //      be "5" for the property "rocksdb.num-files-at-level5". So far, only
+  //      certain string properties take an argument.
+  bool (InternalStats::*handle_string)(std::string* value, Slice suffix);
+
+  // @param value Value-result argument for storing the property's uint64 value
+  // @param db Many of the int properties rely on DBImpl methods.
+  // @param version Version is needed in case the property is retrieved without
+  //      holding db mutex, which is only supported for int properties.
+  bool (InternalStats::*handle_int)(uint64_t* value, DBImpl* db,
+                                    Version* version);
 };
 
-extern DBPropertyType GetPropertyType(const Slice& property,
-                                      bool* is_int_property,
-                                      bool* need_out_of_mutex);
-
+extern const DBPropertyInfo* GetPropertyInfo(const Slice& property);
 
 #ifndef ROCKSDB_LITE
 class InternalStats {
@@ -248,14 +214,18 @@ class InternalStats {
 
   uint64_t BumpAndGetBackgroundErrorCount() { return ++bg_error_count_; }
 
-  bool GetStringProperty(DBPropertyType property_type, const Slice& property,
-                         std::string* value);
+  bool GetStringProperty(const DBPropertyInfo& property_info,
+                         const Slice& property, std::string* value);
+
+  bool GetIntProperty(const DBPropertyInfo& property_info, uint64_t* value,
+                      DBImpl* db);
 
-  bool GetIntProperty(DBPropertyType property_type, uint64_t* value,
-                      DBImpl* db) const;
+  bool GetIntPropertyOutOfMutex(const DBPropertyInfo& property_info,
+                                Version* version, uint64_t* value);
 
-  bool GetIntPropertyOutOfMutex(DBPropertyType property_type, Version* version,
-                                uint64_t* value) const;
+  // Store a mapping from the user-facing DB::Properties string to our
+  // DBPropertyInfo struct used internally for retrieving properties.
+  static const std::unordered_map<std::string, DBPropertyInfo> ppt_name_to_info;
 
  private:
   void DumpDBStats(std::string* value);
@@ -321,6 +291,54 @@ class InternalStats {
           seconds_up(0) {}
   } db_stats_snapshot_;
 
+  // Handler functions for getting property values. They use "value" as a value-
+  // result argument, and return true upon successfully setting "value".
+  bool HandleNumFilesAtLevel(std::string* value, Slice suffix);
+  bool HandleLevelStats(std::string* value, Slice suffix);
+  bool HandleStats(std::string* value, Slice suffix);
+  bool HandleCFStats(std::string* value, Slice suffix);
+  bool HandleDBStats(std::string* value, Slice suffix);
+  bool HandleSsTables(std::string* value, Slice suffix);
+  bool HandleAggregatedTableProperties(std::string* value, Slice suffix);
+  bool HandleAggregatedTablePropertiesAtLevel(std::string* value, Slice suffix);
+  bool HandleNumImmutableMemTable(uint64_t* value, DBImpl* db,
+                                  Version* version);
+  bool HandleNumImmutableMemTableFlushed(uint64_t* value, DBImpl* db,
+                                         Version* version);
+  bool HandleMemTableFlushPending(uint64_t* value, DBImpl* db,
+                                  Version* version);
+  bool HandleNumRunningFlushes(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleCompactionPending(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleNumRunningCompactions(uint64_t* value, DBImpl* db,
+                                   Version* version);
+  bool HandleBackgroundErrors(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleCurSizeActiveMemTable(uint64_t* value, DBImpl* db,
+                                   Version* version);
+  bool HandleCurSizeAllMemTables(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleSizeAllMemTables(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleNumEntriesActiveMemTable(uint64_t* value, DBImpl* db,
+                                      Version* version);
+  bool HandleNumEntriesImmMemTables(uint64_t* value, DBImpl* db,
+                                    Version* version);
+  bool HandleNumDeletesActiveMemTable(uint64_t* value, DBImpl* db,
+                                      Version* version);
+  bool HandleNumDeletesImmMemTables(uint64_t* value, DBImpl* db,
+                                    Version* version);
+  bool HandleEstimateNumKeys(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleNumSnapshots(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleOldestSnapshotTime(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleNumLiveVersions(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleIsFileDeletionsEnabled(uint64_t* value, DBImpl* db,
+                                    Version* version);
+  bool HandleBaseLevel(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleTotalSstFilesSize(uint64_t* value, DBImpl* db, Version* version);
+  bool HandleEstimatePendingCompactionBytes(uint64_t* value, DBImpl* db,
+                                            Version* version);
+  bool HandleEstimateTableReadersMem(uint64_t* value, DBImpl* db,
+                                     Version* version);
+  bool HandleEstimateLiveDataSize(uint64_t* value, DBImpl* db,
+                                  Version* version);
+
   // Total number of background errors encountered. Every time a flush task
   // or compaction task fails, this counter is incremented. The failure can
   // be caused by any possible reason, including file system errors, out of
@@ -402,14 +420,20 @@ class InternalStats {
 
   uint64_t BumpAndGetBackgroundErrorCount() { return 0; }
 
-  bool GetStringProperty(DBPropertyType property_type, const Slice& property,
-                         std::string* value) { return false; }
+  bool GetStringProperty(const DBPropertyInfo& property_info,
+                         const Slice& property, std::string* value) {
+    return false;
+  }
 
-  bool GetIntProperty(DBPropertyType property_type, uint64_t* value,
-                      DBImpl* db) const { return false; }
+  bool GetIntProperty(const DBPropertyInfo& property_info, uint64_t* value,
+                      DBImpl* db) const {
+    return false;
+  }
 
-  bool GetIntPropertyOutOfMutex(DBPropertyType property_type, Version* version,
-                                uint64_t* value) const { return false; }
+  bool GetIntPropertyOutOfMutex(const DBPropertyInfo& property_info,
+                                Version* version, uint64_t* value) const {
+    return false;
+  }
 };
 #endif  // !ROCKSDB_LITE
 

include/rocksdb/db.h

@@ -329,6 +329,9 @@ class DB {
 
 #ifndef ROCKSDB_LITE
   // Contains all valid property arguments for GetProperty().
+  //
+  // NOTE: Property names cannot end in numbers since those are interpreted as
+  //       arguments, e.g., see kNumFilesAtLevelPrefix.
   struct Properties {
     //  "rocksdb.num-files-at-level<N>" - returns string containing the number
     //      of files at level <N>, where <N> is an ASCII representation of a