From 7f5b9f40cb9a888447b7097cc5b834417d9fad66 Mon Sep 17 00:00:00 2001 From: Hui Xiao Date: Wed, 5 Apr 2023 14:42:31 -0700 Subject: [PATCH] Fix initialization-order-fiasco in write_stall_stats.cc (#11355) Summary: **Context/Summary:** As title. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11355 Test Plan: - Ran previously failed tests and they succeed - Perf `./db_bench -seed=1679014417652004 -db=/dev/shm/testdb/ -statistics=false -benchmarks="fillseq[-X60]" -key_size=32 -value_size=512 -num=100000 -db_write_buffer_size=655 -target_file_size_base=655 -disable_auto_compactions=false -compression_type=none -bloom_bits=3` Reviewed By: ajkr Differential Revision: D44719333 Pulled By: hx235 fbshipit-source-id: 23d22f314144071d97f7106ff1241c31c0bdf08b --- db/db_properties_test.cc | 15 +++++----- db/write_stall_stats.cc | 60 ++++++++++++++++++++++++++-------------- db/write_stall_stats.h | 11 +++----- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/db/db_properties_test.cc b/db/db_properties_test.cc index 2c843a9749..074f4e9a86 100644 --- a/db/db_properties_test.cc +++ b/db/db_properties_test.cc @@ -2109,25 +2109,26 @@ TEST_F(DBPropertiesTest, GetMapPropertyBlockCacheEntryStats) { TEST_F(DBPropertiesTest, WriteStallStatsSanityCheck) { for (uint32_t i = 0; i < static_cast(WriteStallCause::kNone); ++i) { - std::string str = kWriteStallCauseToHyphenString[i]; + WriteStallCause cause = static_cast(i); + const std::string& str = WriteStallCauseToHyphenString(cause); ASSERT_TRUE(!str.empty()) << "Please ensure mapping from `WriteStallCause` to " - "`kWriteStallCauseToHyphenString` is complete"; - WriteStallCause cause = static_cast(i); + "`WriteStallCauseToHyphenString` is complete"; if (cause == WriteStallCause::kCFScopeWriteStallCauseEnumMax || cause == WriteStallCause::kDBScopeWriteStallCauseEnumMax) { - ASSERT_EQ(str, kInvalidWriteStallCauseHyphenString) - << "Please ensure order in `kWriteStallCauseToHyphenString` is " + ASSERT_EQ(str, InvalidWriteStallHyphenString()) + << "Please ensure order in `WriteStallCauseToHyphenString` is " "consistent with `WriteStallCause`"; } } for (uint32_t i = 0; i < static_cast(WriteStallCondition::kNormal); ++i) { - std::string str = kWriteStallConditionToHyphenString[i]; + WriteStallCondition condition = static_cast(i); + const std::string& str = WriteStallConditionToHyphenString(condition); ASSERT_TRUE(!str.empty()) << "Please ensure mapping from `WriteStallCondition` to " - "`kWriteStallConditionToHyphenString` is complete"; + "`WriteStallConditionToHyphenString` is complete"; } for (uint32_t i = 0; i < static_cast(WriteStallCause::kNone); ++i) { diff --git a/db/write_stall_stats.cc b/db/write_stall_stats.cc index 3143531e72..3973df7685 100644 --- a/db/write_stall_stats.cc +++ b/db/write_stall_stats.cc @@ -6,26 +6,46 @@ #include "db/write_stall_stats.h" namespace ROCKSDB_NAMESPACE { -const std::string kInvalidWriteStallCauseHyphenString = "invalid"; +const std::string& InvalidWriteStallHyphenString() { + static const std::string kInvalidWriteStallHyphenString = "invalid"; + return kInvalidWriteStallHyphenString; +} -const std::array(WriteStallCause::kNone)> - kWriteStallCauseToHyphenString{{ - "memtable-limit", - "l0-file-count-limit", - "pending-compaction-bytes", - // WriteStallCause::kCFScopeWriteStallCauseEnumMax - kInvalidWriteStallCauseHyphenString, - "write-buffer-manager-limit", - // WriteStallCause::kDBScopeWriteStallCauseEnumMax - kInvalidWriteStallCauseHyphenString, - }}; +const std::string& WriteStallCauseToHyphenString(WriteStallCause cause) { + static const std::string kMemtableLimit = "memtable-limit"; + static const std::string kL0FileCountLimit = "l0-file-count-limit"; + static const std::string kPendingCompactionBytes = "pending-compaction-bytes"; + static const std::string kWriteBufferManagerLimit = + "write-buffer-manager-limit"; + switch (cause) { + case WriteStallCause::kMemtableLimit: + return kMemtableLimit; + case WriteStallCause::kL0FileCountLimit: + return kL0FileCountLimit; + case WriteStallCause::kPendingCompactionBytes: + return kPendingCompactionBytes; + case WriteStallCause::kWriteBufferManagerLimit: + return kWriteBufferManagerLimit; + default: + break; + } + return InvalidWriteStallHyphenString(); +} -const std::array(WriteStallCondition::kNormal)> - kWriteStallConditionToHyphenString{{ - "delays", - "stops", - }}; +const std::string& WriteStallConditionToHyphenString( + WriteStallCondition condition) { + static const std::string kDelayed = "delays"; + static const std::string kStopped = "stops"; + switch (condition) { + case WriteStallCondition::kDelayed: + return kDelayed; + case WriteStallCondition::kStopped: + return kStopped; + default: + break; + } + return InvalidWriteStallHyphenString(); +} InternalStats::InternalCFStatsType InternalCFStat( WriteStallCause cause, WriteStallCondition condition) { @@ -139,14 +159,14 @@ std::string WriteStallStatsMapKeys::CauseConditionCount( std::string cause_name; if (isCFScopeWriteStallCause(cause) || isDBScopeWriteStallCause(cause)) { - cause_name = kWriteStallCauseToHyphenString[static_cast(cause)]; + cause_name = WriteStallCauseToHyphenString(cause); } else { assert(false); return ""; } const std::string& condition_name = - kWriteStallConditionToHyphenString[static_cast(condition)]; + WriteStallConditionToHyphenString(condition); cause_condition_count_name.reserve(cause_name.size() + 1 + condition_name.size()); diff --git a/db/write_stall_stats.h b/db/write_stall_stats.h index 9ae518a079..6394abb0a8 100644 --- a/db/write_stall_stats.h +++ b/db/write_stall_stats.h @@ -11,15 +11,12 @@ #include "rocksdb/types.h" namespace ROCKSDB_NAMESPACE { -extern const std::string kInvalidWriteStallCauseHyphenString; +extern const std::string& InvalidWriteStallHyphenString(); -extern const std::array(WriteStallCause::kNone)> - kWriteStallCauseToHyphenString; +extern const std::string& WriteStallCauseToHyphenString(WriteStallCause cause); -extern const std::array(WriteStallCondition::kNormal)> - kWriteStallConditionToHyphenString; +extern const std::string& WriteStallConditionToHyphenString( + WriteStallCondition condition); // REQUIRES: // cause` is CF-scope `WriteStallCause`, see `WriteStallCause` for more