From ea6b24e8a184a6fde5f543505e4436807b286865 Mon Sep 17 00:00:00 2001 From: Nikita Taranov Date: Wed, 30 Jul 2025 12:00:09 +0000 Subject: [PATCH] Merge pull request #84317 from ClickHouse/fix_metrics Report cgroup and system-wide metrics along with each other --- src/Common/AsynchronousMetrics.cpp | 53 +++++++++++++------ src/Common/AsynchronousMetrics.h | 9 ++++ .../test_async_metrics_in_cgroup/test.py | 19 +++---- 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/src/Common/AsynchronousMetrics.cpp b/src/Common/AsynchronousMetrics.cpp index eb7fcfdaced1..08a55433ecdf 100644 --- a/src/Common/AsynchronousMetrics.cpp +++ b/src/Common/AsynchronousMetrics.cpp @@ -594,6 +594,19 @@ AsynchronousMetrics::NetworkInterfaceStatValues::operator-(const AsynchronousMet #if defined(OS_LINUX) + +void AsynchronousMetrics::applyCgroupCPUMetricsUpdate( + AsynchronousMetricValues & new_values, const ProcStatValuesCPU & delta_values, double multiplier) +{ + new_values["CGroupUserTime"] + = {delta_values.user * multiplier, + "The ratio of time the CPU core was running userspace code." + " This includes also the time when the CPU was under-utilized due to the reasons internal to the CPU (memory loads, pipeline " + "stalls, branch mispredictions, running another SMT core)."}; + new_values["CGroupSystemTime"] + = {delta_values.system * multiplier, "The ratio of time the CPU core was running OS kernel (system) code."}; +} + void AsynchronousMetrics::applyCPUMetricsUpdate( AsynchronousMetricValues & new_values, const std::string & cpu_suffix, const ProcStatValuesCPU & delta_values, double multiplier) { @@ -670,6 +683,25 @@ void AsynchronousMetrics::applyCPUMetricsUpdate( "them [0..num cores]."}; } +void AsynchronousMetrics::applyCgroupNormalizedCPUMetricsUpdate( + AsynchronousMetricValues & new_values, double num_cpus_to_normalize, const ProcStatValuesCPU & delta_values_all_cpus, double multiplier) +{ + chassert(num_cpus_to_normalize); + + new_values["CGroupUserTimeNormalized"] + = {delta_values_all_cpus.user * multiplier / num_cpus_to_normalize, + "The value is similar to `CGroupUserTime` but divided by the number of available CPU cores to be measured in the [0..1] " + "interval regardless of the number of cores." + " This allows you to average the values of this metric across multiple servers in a cluster even if the number of cores is " + "non-uniform, and still get the average resource utilization metric."}; + new_values["CGroupSystemTimeNormalized"] + = {delta_values_all_cpus.system * multiplier / num_cpus_to_normalize, + "The value is similar to `CGroupSystemTime` but divided by the number of available CPU cores to be measured in the [0..1] " + "interval regardless of the number of cores." + " This allows you to average the values of this metric across multiple servers in a cluster even if the number of cores is " + "non-uniform, and still get the average resource utilization metric."}; +} + void AsynchronousMetrics::applyNormalizedCPUMetricsUpdate( AsynchronousMetricValues & new_values, double num_cpus_to_normalize, const ProcStatValuesCPU & delta_values_all_cpus, double multiplier) { @@ -1003,8 +1035,7 @@ void AsynchronousMetrics::update(TimePoint update_time, bool force_update) new_values["CGroupMaxCPU"] = { max_cpu_cgroups, "The maximum number of CPU cores according to CGroups."}; } - const bool cgroup_cpu_metrics_present = cgroupcpu_stat || cgroupcpuacct_stat; - if (cgroup_cpu_metrics_present) + if (cgroupcpu_stat || cgroupcpuacct_stat) { try { @@ -1048,13 +1079,13 @@ void AsynchronousMetrics::update(TimePoint update_time, bool force_update) const double multiplier = 1.0 / cgroup_version_specific_divisor / (std::chrono::duration_cast(time_since_previous_update).count() / 1e9); - const ProcStatValuesCPU delta_values = current_values - proc_stat_values_all_cpus; - applyCPUMetricsUpdate(new_values, /*cpu_suffix=*/"", delta_values, multiplier); + const ProcStatValuesCPU delta_values = current_values - cgroup_values_all_cpus; + applyCgroupCPUMetricsUpdate(new_values, delta_values, multiplier); if (max_cpu_cgroups > 0) - applyNormalizedCPUMetricsUpdate(new_values, max_cpu_cgroups, delta_values, multiplier); + applyCgroupNormalizedCPUMetricsUpdate(new_values, max_cpu_cgroups, delta_values, multiplier); } - proc_stat_values_all_cpus = current_values; + cgroup_values_all_cpus = current_values; } catch (...) { @@ -1088,14 +1119,6 @@ void AsynchronousMetrics::update(TimePoint update_time, bool force_update) if (name.starts_with("cpu")) { - if (cgroup_cpu_metrics_present) - { - /// Skip the CPU metrics if we already have them from cgroup - ProcStatValuesCPU current_values{}; - current_values.read(*proc_stat); - continue; - } - String cpu_num_str = name.substr(strlen("cpu")); UInt64 cpu_num = 0; if (!cpu_num_str.empty()) @@ -1182,7 +1205,7 @@ void AsynchronousMetrics::update(TimePoint update_time, bool force_update) Float64 num_cpus_to_normalize = max_cpu_cgroups > 0 ? max_cpu_cgroups : num_cpus; - if (num_cpus_to_normalize > 0 && !cgroup_cpu_metrics_present) + if (num_cpus_to_normalize > 0) applyNormalizedCPUMetricsUpdate(new_values, num_cpus_to_normalize, delta_values_all_cpus, multiplier); } diff --git a/src/Common/AsynchronousMetrics.h b/src/Common/AsynchronousMetrics.h index 4eedf5412233..9bdae8464ab1 100644 --- a/src/Common/AsynchronousMetrics.h +++ b/src/Common/AsynchronousMetrics.h @@ -190,6 +190,7 @@ class AsynchronousMetrics ProcStatValuesOther operator-(const ProcStatValuesOther & other) const; }; + ProcStatValuesCPU cgroup_values_all_cpus TSA_GUARDED_BY(data_mutex){}; ProcStatValuesCPU proc_stat_values_all_cpus TSA_GUARDED_BY(data_mutex) {}; ProcStatValuesOther proc_stat_values_other TSA_GUARDED_BY(data_mutex) {}; std::vector proc_stat_values_per_cpu TSA_GUARDED_BY(data_mutex); @@ -242,6 +243,14 @@ class AsynchronousMetrics void openSensorsChips(); void openEDAC(); + void applyCgroupCPUMetricsUpdate(AsynchronousMetricValues & new_values, const ProcStatValuesCPU & delta_values, double multiplier); + + void applyCgroupNormalizedCPUMetricsUpdate( + AsynchronousMetricValues & new_values, + double num_cpus_to_normalize, + const ProcStatValuesCPU & delta_values_all_cpus, + double multiplier); + void applyCPUMetricsUpdate( AsynchronousMetricValues & new_values, const std::string & cpu_suffix, const ProcStatValuesCPU & delta_values, double multiplier); diff --git a/tests/integration/test_async_metrics_in_cgroup/test.py b/tests/integration/test_async_metrics_in_cgroup/test.py index d23bf041a551..6a5dd8f79747 100644 --- a/tests/integration/test_async_metrics_in_cgroup/test.py +++ b/tests/integration/test_async_metrics_in_cgroup/test.py @@ -46,17 +46,12 @@ def test_user_cpu_accounting(start_cluster): # run query on the other node, its usage shouldn't be accounted by node1 run_cpu_intensive_task(node2) - node1_cpu_time = get_async_metric(node1, "OSUserTime") - assert float(node1_cpu_time) < 2 + node1_cgroup_cpu_time = get_async_metric(node1, "CGroupUserTime") + assert float(node1_cgroup_cpu_time) < 2 - # then let's test that we will account cpu time spent by the server itself - node2_cpu_time = get_async_metric(node2, "OSUserTime") + node1_os_cpu_time = get_async_metric(node1, "OSUserTime") # this check is really weak, but CI is tough place and we cannot guarantee that test process will get many cpu time - assert float(node2_cpu_time) > 2 - - # OSIdleTime is not available for cgroups, so it shouldn't be present - for node in [node1, node2]: - assert get_async_metric(node, "OSIdleTime") == "0" + assert float(node1_os_cpu_time) > 2 def test_normalized_user_cpu(start_cluster): @@ -66,10 +61,10 @@ def test_normalized_user_cpu(start_cluster): # run query on the other node, its usage shouldn't be accounted by node1 run_cpu_intensive_task(node2) - node1_cpu_time = get_async_metric(node1, "OSUserTimeNormalized") + node1_cpu_time = get_async_metric(node1, "CGroupUserTimeNormalized") assert float(node1_cpu_time) < 1.01 - node2_cpu_time = get_async_metric(node2, "OSUserTimeNormalized") + node2_cpu_time = get_async_metric(node2, "CGroupUserTimeNormalized") assert float(node2_cpu_time) < 1.01 @@ -89,6 +84,8 @@ def test_system_wide_metrics(start_cluster): "OSProcessesRunning", "OSInterrupts", "OSMemoryTotal", + "OSUserTimeNormalized", + "OSSystemTimeNormalized", ]: node2_value = get_async_metric(node2, metric) assert float(node2_value) > 0