From 059a2704d05be56da5771ada04111cc498859029 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Thu, 25 Jun 2020 23:36:50 +0300 Subject: [PATCH 1/4] Add CPU frequencies to system.asynchronous_metrics --- src/Core/Settings.h | 1 + src/IO/ReadHelpers.h | 2 + src/Interpreters/AsynchronousMetrics.cpp | 50 ++++++++++++++++++++---- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/Core/Settings.h b/src/Core/Settings.h index f434132eccd..52add89b009 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -230,6 +230,7 @@ struct Settings : public SettingsCollection M(SettingUInt64, query_profiler_cpu_time_period_ns, 1000000000, "Period for CPU clock timer of query profiler (in nanoseconds). Set 0 value to turn off the CPU clock query profiler. Recommended value is at least 10000000 (100 times a second) for single queries or 1000000000 (once a second) for cluster-wide profiling.", 0) \ M(SettingBool, metrics_perf_events_enabled, false, "If enabled, some of the perf events will be measured throughout queries' execution.", 0) \ M(SettingString, metrics_perf_events_list, "", "Comma separated list of perf metrics that will be measured throughout queries' execution. Empty means all events. See PerfEventInfo in sources for the available events.", 0) \ + M(SettingUInt64, asynchronous_metrics_update_period_s, 60, "Period for updating asynchronous metrics, in seconds.", 0) \ \ \ /** Limits during query execution are part of the settings. \ diff --git a/src/IO/ReadHelpers.h b/src/IO/ReadHelpers.h index 7254ae43c17..52601ecb25b 100644 --- a/src/IO/ReadHelpers.h +++ b/src/IO/ReadHelpers.h @@ -454,6 +454,8 @@ void readBackQuotedString(String & s, ReadBuffer & buf); void readBackQuotedStringWithSQLStyle(String & s, ReadBuffer & buf); void readStringUntilEOF(String & s, ReadBuffer & buf); + +// Doesn't read the EOL itself. void readEscapedStringUntilEOL(String & s, ReadBuffer & buf); diff --git a/src/Interpreters/AsynchronousMetrics.cpp b/src/Interpreters/AsynchronousMetrics.cpp index babe63f8522..1dfdaa298b7 100644 --- a/src/Interpreters/AsynchronousMetrics.cpp +++ b/src/Interpreters/AsynchronousMetrics.cpp @@ -63,16 +63,13 @@ void AsynchronousMetrics::run() { setThreadName("AsyncMetrics"); - const auto get_next_update_time = [] + const auto period = std::chrono::seconds( + context.getSettingsRef().asynchronous_metrics_update_period_s); + + const auto get_next_update_time = [period] { using namespace std::chrono; - // The period doesn't really have to be configurable, but sometimes you - // need to change it by recompilation to debug something. The generic - // code is left here so that you don't have to ruin your mood by touching - // std::chrono. - const seconds period(60); - const auto now = time_point_cast(system_clock::now()); // Use seconds since the start of the hour, because we don't know when @@ -329,6 +326,45 @@ void AsynchronousMetrics::update() saveAllArenasMetric(new_values, "muzzy_purged"); #endif + // Try to add processor frequencies, ignoring errors. + try + { + ReadBufferFromFile buf("/proc/cpuinfo"); + + // We need the following lines: + // core id : 4 + // cpu MHz : 4052.941 + // They contain tabs and are interspersed with other info. + int core_id = 0; + while (!buf.eof()) + { + std::string s; + readEscapedStringUntilEOL(s, buf); + // It doesn't read the EOL itself. + ++buf.position(); + + if (s.rfind("core id", 0) == 0) + { + if (auto colon = s.find_first_of(':')) + { + core_id = std::stoi(s.substr(colon + 2)); + } + } + else if (s.rfind("cpu MHz", 0) == 0) + { + if (auto colon = s.find_first_of(':')) + { + auto mhz = std::stod(s.substr(colon + 2)); + new_values[fmt::format("CPUFrequencyMHz_{}", core_id)] = mhz; + } + } + } + } + catch (...) + { + tryLogCurrentException(__PRETTY_FUNCTION__); + } + /// Add more metrics as you wish. // Log the new metrics. From 477fbc70f8e2e93c960f4ce28d32ef872787c125 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Thu, 25 Jun 2020 23:46:18 +0300 Subject: [PATCH 2/4] Update AsynchronousMetrics.cpp --- src/Interpreters/AsynchronousMetrics.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Interpreters/AsynchronousMetrics.cpp b/src/Interpreters/AsynchronousMetrics.cpp index 1dfdaa298b7..57cc9bce2b8 100644 --- a/src/Interpreters/AsynchronousMetrics.cpp +++ b/src/Interpreters/AsynchronousMetrics.cpp @@ -332,8 +332,8 @@ void AsynchronousMetrics::update() ReadBufferFromFile buf("/proc/cpuinfo"); // We need the following lines: - // core id : 4 - // cpu MHz : 4052.941 + // core id : 4 + // cpu MHz : 4052.941 // They contain tabs and are interspersed with other info. int core_id = 0; while (!buf.eof()) From d77f397b384cbd73e7f18882942dd2900cbe7785 Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov Date: Fri, 26 Jun 2020 03:16:58 +0300 Subject: [PATCH 3/4] review fixes --- programs/server/Server.cpp | 3 +- src/Core/Settings.h | 1 - src/IO/ReadHelpers.h | 3 +- src/Interpreters/AsynchronousMetrics.cpp | 60 ++++++++++++------------ src/Interpreters/AsynchronousMetrics.h | 9 ++-- 5 files changed, 41 insertions(+), 35 deletions(-) diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index c99543ff631..a0dfb80b037 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -861,7 +861,8 @@ int Server::main(const std::vector & /*args*/) }; /// This object will periodically calculate some metrics. - AsynchronousMetrics async_metrics(*global_context); + AsynchronousMetrics async_metrics(*global_context, + config().getUInt("asynchronous_metrics_update_period_s", 60)); attachSystemTablesAsync(*DatabaseCatalog::instance().getSystemDatabase(), async_metrics); for (const auto & listen_host : listen_hosts) diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 52add89b009..f434132eccd 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -230,7 +230,6 @@ struct Settings : public SettingsCollection M(SettingUInt64, query_profiler_cpu_time_period_ns, 1000000000, "Period for CPU clock timer of query profiler (in nanoseconds). Set 0 value to turn off the CPU clock query profiler. Recommended value is at least 10000000 (100 times a second) for single queries or 1000000000 (once a second) for cluster-wide profiling.", 0) \ M(SettingBool, metrics_perf_events_enabled, false, "If enabled, some of the perf events will be measured throughout queries' execution.", 0) \ M(SettingString, metrics_perf_events_list, "", "Comma separated list of perf metrics that will be measured throughout queries' execution. Empty means all events. See PerfEventInfo in sources for the available events.", 0) \ - M(SettingUInt64, asynchronous_metrics_update_period_s, 60, "Period for updating asynchronous metrics, in seconds.", 0) \ \ \ /** Limits during query execution are part of the settings. \ diff --git a/src/IO/ReadHelpers.h b/src/IO/ReadHelpers.h index 52601ecb25b..f299ab2286f 100644 --- a/src/IO/ReadHelpers.h +++ b/src/IO/ReadHelpers.h @@ -455,7 +455,8 @@ void readBackQuotedStringWithSQLStyle(String & s, ReadBuffer & buf); void readStringUntilEOF(String & s, ReadBuffer & buf); -// Doesn't read the EOL itself. +// Reads the line until EOL, unescaping backslash escape sequences. +// Buffer pointer is left at EOL, don't forget to advance it. void readEscapedStringUntilEOL(String & s, ReadBuffer & buf); diff --git a/src/Interpreters/AsynchronousMetrics.cpp b/src/Interpreters/AsynchronousMetrics.cpp index 57cc9bce2b8..eb93ac4412b 100644 --- a/src/Interpreters/AsynchronousMetrics.cpp +++ b/src/Interpreters/AsynchronousMetrics.cpp @@ -58,45 +58,44 @@ AsynchronousMetricValues AsynchronousMetrics::getValues() const return values; } +static auto get_next_update_time(std::chrono::seconds update_period) +{ + using namespace std::chrono; + + const auto now = time_point_cast(system_clock::now()); + + // Use seconds since the start of the hour, because we don't know when + // the epoch started, maybe on some weird fractional time. + const auto start_of_hour = time_point_cast(time_point_cast(now)); + const auto seconds_passed = now - start_of_hour; + + // Rotate time forward by half a period -- e.g. if a period is a minute, + // we'll collect metrics on start of minute + 30 seconds. This is to + // achieve temporal separation with MetricTransmitter. Don't forget to + // rotate it back. + const auto rotation = update_period / 2; + + const auto periods_passed = (seconds_passed + rotation) / update_period; + const auto seconds_next = (periods_passed + 1) * update_period - rotation; + const auto time_next = start_of_hour + seconds_next; + + return time_next; +} void AsynchronousMetrics::run() { setThreadName("AsyncMetrics"); - const auto period = std::chrono::seconds( - context.getSettingsRef().asynchronous_metrics_update_period_s); - - const auto get_next_update_time = [period] - { - using namespace std::chrono; - - const auto now = time_point_cast(system_clock::now()); - - // Use seconds since the start of the hour, because we don't know when - // the epoch started, maybe on some weird fractional time. - const auto start_of_hour = time_point_cast(time_point_cast(now)); - const auto seconds_passed = now - start_of_hour; - - // Rotate time forward by half a period -- e.g. if a period is a minute, - // we'll collect metrics on start of minute + 30 seconds. This is to - // achieve temporal separation with MetricTransmitter. Don't forget to - // rotate it back. - const auto rotation = period / 2; - - const auto periods_passed = (seconds_passed + rotation) / period; - const auto seconds_next = (periods_passed + 1) * period - rotation; - const auto time_next = start_of_hour + seconds_next; - - return time_next; - }; - while (true) { { // Wait first, so that the first metric collection is also on even time. std::unique_lock lock{mutex}; - if (wait_cond.wait_until(lock, get_next_update_time(), [this] { return quit; })) + if (wait_cond.wait_until(lock, get_next_update_time(update_period), + [this] { return quit; })) + { break; + } } try @@ -329,7 +328,7 @@ void AsynchronousMetrics::update() // Try to add processor frequencies, ignoring errors. try { - ReadBufferFromFile buf("/proc/cpuinfo"); + ReadBufferFromFile buf("/proc/cpuinfo", 32768 /* buf_size */); // We need the following lines: // core id : 4 @@ -339,6 +338,9 @@ void AsynchronousMetrics::update() while (!buf.eof()) { std::string s; + // We don't have any backslash escape sequences in /proc/cpuinfo, so + // this function will read the line until EOL, which is exactly what + // we need. readEscapedStringUntilEOL(s, buf); // It doesn't read the EOL itself. ++buf.position(); diff --git a/src/Interpreters/AsynchronousMetrics.h b/src/Interpreters/AsynchronousMetrics.h index 6817f545c8f..dc892b9d40c 100644 --- a/src/Interpreters/AsynchronousMetrics.h +++ b/src/Interpreters/AsynchronousMetrics.h @@ -18,15 +18,17 @@ typedef double AsynchronousMetricValue; typedef std::unordered_map AsynchronousMetricValues; -/** Periodically (each minute, starting at 30 seconds offset) +/** Periodically (by default, each minute, starting at 30 seconds offset) * calculates and updates some metrics, * that are not updated automatically (so, need to be asynchronously calculated). */ class AsynchronousMetrics { public: - AsynchronousMetrics(Context & context_) - : context(context_), thread([this] { run(); }) + AsynchronousMetrics(Context & context_, int update_period_seconds) + : context(context_), + update_period(update_period_seconds), + thread([this] { run(); }) { } @@ -38,6 +40,7 @@ public: private: Context & context; + const std::chrono::seconds update_period; mutable std::mutex mutex; std::condition_variable wait_cond; From 808713471c8a31a856b46b8d8be60489af27587e Mon Sep 17 00:00:00 2001 From: Alexander Kuzmenkov <36882414+akuzm@users.noreply.github.com> Date: Fri, 26 Jun 2020 05:18:57 +0300 Subject: [PATCH 4/4] Update AsynchronousMetrics.h --- src/Interpreters/AsynchronousMetrics.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Interpreters/AsynchronousMetrics.h b/src/Interpreters/AsynchronousMetrics.h index dc892b9d40c..6ab32ff9ab6 100644 --- a/src/Interpreters/AsynchronousMetrics.h +++ b/src/Interpreters/AsynchronousMetrics.h @@ -25,7 +25,10 @@ typedef std::unordered_map AsynchronousMet class AsynchronousMetrics { public: - AsynchronousMetrics(Context & context_, int update_period_seconds) + // The default value of update_period_seconds is for ClickHouse-over-YT + // in Arcadia -- it uses its own server implementation that also uses these + // metrics. + AsynchronousMetrics(Context & context_, int update_period_seconds = 60) : context(context_), update_period(update_period_seconds), thread([this] { run(); })