diff --git a/CMakeLists.txt b/CMakeLists.txt index 984c6197c..123356965 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -326,12 +326,23 @@ CHECK_CXX_SOURCE_COMPILES(" #endif int main() { static __thread int tls; + (void)tls; } " HAVE_THREAD_LOCAL) if(HAVE_THREAD_LOCAL) add_definitions(-DROCKSDB_SUPPORT_THREAD_LOCAL) endif() +option(WITH_IOSTATS_CONTEXT "Enable IO stats context" ON) +if (NOT WITH_IOSTATS_CONTEXT) + add_definitions(-DNIOSTATS_CONTEXT) +endif() + +option(WITH_PERF_CONTEXT "Enable perf context" ON) +if (NOT WITH_PERF_CONTEXT) + add_definitions(-DNPERF_CONTEXT) +endif() + option(FAIL_ON_WARNINGS "Treat compile warnings as errors" ON) if(FAIL_ON_WARNINGS) if(MSVC) diff --git a/HISTORY.md b/HISTORY.md index 5c57fc02e..02b1c4af9 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -4,6 +4,7 @@ * `ColumnFamilyOptions::sample_for_compression` now takes effect for creation of all block-based tables. Previously it only took effect for block-based tables created by flush. * `CompactFiles()` can no longer compact files from lower level to up level, which has the risk to corrupt DB (details: #8063). The validation is also added to all compactions. * Fixed some cases in which DB::OpenForReadOnly() could write to the filesystem. If you want a Logger with a read-only DB, you must now set DBOptions::info_log yourself, such as using CreateLoggerFromOptions(). +* get_iostats_context() will never return nullptr. If thread-local support is not available, and user does not opt-out iostats context, then compilation will fail. The same applies to perf context as well. ### Bug Fixes * Use thread-safe `strerror_r()` to get error messages. diff --git a/include/rocksdb/iostats_context.h b/include/rocksdb/iostats_context.h index b31b6d70a..0f6ab692e 100644 --- a/include/rocksdb/iostats_context.h +++ b/include/rocksdb/iostats_context.h @@ -50,7 +50,15 @@ struct IOStatsContext { uint64_t cpu_read_nanos; }; -// Get Thread-local IOStatsContext object pointer +// If RocksDB is compiled with -DNIOSTATS_CONTEXT, then a pointer to a global, +// non-thread-local IOStatsContext object will be returned. Attempts to update +// this object will be ignored, and reading from it will also be no-op. +// Otherwise, +// a) if thread-local is supported on the platform, then a pointer to +// a thread-local IOStatsContext object will be returned. +// b) if thread-local is NOT supported, then compilation will fail. +// +// This function never returns nullptr. IOStatsContext* get_iostats_context(); } // namespace ROCKSDB_NAMESPACE diff --git a/include/rocksdb/perf_context.h b/include/rocksdb/perf_context.h index 01eff575e..699f57344 100644 --- a/include/rocksdb/perf_context.h +++ b/include/rocksdb/perf_context.h @@ -230,8 +230,15 @@ struct PerfContext { bool per_level_perf_context_enabled = false; }; -// Get Thread-local PerfContext object pointer -// if defined(NPERF_CONTEXT), then the pointer is not thread-local +// If RocksDB is compiled with -DNPERF_CONTEXT, then a pointer to a global, +// non-thread-local PerfContext object will be returned. Attempts to update +// this object will be ignored, and reading from it will also be no-op. +// Otherwise, +// a) if thread-local is supported on the platform, then a pointer to +// a thread-local PerfContext object will be returned. +// b) if thread-local is NOT supported, then compilation will fail. +// +// This function never returns nullptr. PerfContext* get_perf_context(); } // namespace ROCKSDB_NAMESPACE diff --git a/monitoring/iostats_context.cc b/monitoring/iostats_context.cc index 6be681b6e..23bf3a694 100644 --- a/monitoring/iostats_context.cc +++ b/monitoring/iostats_context.cc @@ -9,19 +9,23 @@ namespace ROCKSDB_NAMESPACE { -#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL +#ifdef NIOSTATS_CONTEXT +// Should not be used because the counters are not thread-safe. +// Put here just to make get_iostats_context() simple without ifdef. +static IOStatsContext iostats_context; +#elif defined(ROCKSDB_SUPPORT_THREAD_LOCAL) __thread IOStatsContext iostats_context; +#else +#error \ + "No thread-local support. Disable iostats context with -DNIOSTATS_CONTEXT." #endif IOStatsContext* get_iostats_context() { -#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL return &iostats_context; -#else - return nullptr; -#endif } void IOStatsContext::Reset() { +#ifndef NIOSTATS_CONTEXT thread_pool_id = Env::Priority::TOTAL; bytes_read = 0; bytes_written = 0; @@ -35,6 +39,7 @@ void IOStatsContext::Reset() { logger_nanos = 0; cpu_write_nanos = 0; cpu_read_nanos = 0; +#endif //! NIOSTATS_CONTEXT } #define IOSTATS_CONTEXT_OUTPUT(counter) \ @@ -43,6 +48,10 @@ void IOStatsContext::Reset() { } std::string IOStatsContext::ToString(bool exclude_zero_counters) const { +#ifdef NIOSTATS_CONTEXT + (void)exclude_zero_counters; + return ""; +#else std::ostringstream ss; IOSTATS_CONTEXT_OUTPUT(thread_pool_id); IOSTATS_CONTEXT_OUTPUT(bytes_read); @@ -61,6 +70,7 @@ std::string IOStatsContext::ToString(bool exclude_zero_counters) const { std::string str = ss.str(); str.erase(str.find_last_not_of(", ") + 1); return str; +#endif //! NIOSTATS_CONTEXT } } // namespace ROCKSDB_NAMESPACE diff --git a/monitoring/iostats_context_imp.h b/monitoring/iostats_context_imp.h index 92a3f7126..69b0c6590 100644 --- a/monitoring/iostats_context_imp.h +++ b/monitoring/iostats_context_imp.h @@ -7,7 +7,7 @@ #include "monitoring/perf_step_timer.h" #include "rocksdb/iostats_context.h" -#ifdef ROCKSDB_SUPPORT_THREAD_LOCAL +#if defined(ROCKSDB_SUPPORT_THREAD_LOCAL) && !defined(NIOSTATS_CONTEXT) namespace ROCKSDB_NAMESPACE { extern __thread IOStatsContext iostats_context; } // namespace ROCKSDB_NAMESPACE @@ -44,7 +44,7 @@ extern __thread IOStatsContext iostats_context; PerfLevel::kEnableTimeAndCPUTimeExceptForMutex); \ iostats_step_timer_##metric.Start(); -#else // ROCKSDB_SUPPORT_THREAD_LOCAL +#else // ROCKSDB_SUPPORT_THREAD_LOCAL && !NIOSTATS_CONTEXT #define IOSTATS_ADD(metric, value) #define IOSTATS_ADD_IF_POSITIVE(metric, value) @@ -57,4 +57,4 @@ extern __thread IOStatsContext iostats_context; #define IOSTATS_TIMER_GUARD(metric) #define IOSTATS_CPU_TIMER_GUARD(metric, clock) static_cast(clock) -#endif // ROCKSDB_SUPPORT_THREAD_LOCAL +#endif // ROCKSDB_SUPPORT_THREAD_LOCAL && !NIOSTATS_CONTEXT diff --git a/monitoring/perf_context.cc b/monitoring/perf_context.cc index 53f502405..d45d84fb6 100644 --- a/monitoring/perf_context.cc +++ b/monitoring/perf_context.cc @@ -9,26 +9,22 @@ namespace ROCKSDB_NAMESPACE { -#if defined(NPERF_CONTEXT) || !defined(ROCKSDB_SUPPORT_THREAD_LOCAL) +#if defined(NPERF_CONTEXT) +// Should not be used because the counters are not thread-safe. +// Put here just to make get_perf_context() simple without ifdef. PerfContext perf_context; -#else +#elif defined(ROCKSDB_SUPPORT_THREAD_LOCAL) #if defined(OS_SOLARIS) -__thread PerfContext perf_context_; -#else +__thread PerfContext perf_context; +#else // OS_SOLARIS thread_local PerfContext perf_context; -#endif +#endif // OS_SOLARIS +#else +#error "No thread-local support. Disable perf context with -DNPERF_CONTEXT." #endif PerfContext* get_perf_context() { -#if defined(NPERF_CONTEXT) || !defined(ROCKSDB_SUPPORT_THREAD_LOCAL) return &perf_context; -#else -#if defined(OS_SOLARIS) - return &perf_context_; -#else - return &perf_context; -#endif -#endif } PerfContext::~PerfContext() {