address review comments

base/base/cgroupsv2.cpp

@@ -71,3 +71,27 @@ fs::path cgroupV2PathOfProcess()
     return {};
 #endif
 }
+
+std::optional<std::string> getCgroupsV2PathContainingFile(std::string_view file_name)
+{
+    if (!cgroupsV2Enabled())
+        return {};
+
+    if (!cgroupsV2MemoryControllerEnabled())
+        return {};
+
+    fs::path current_cgroup = cgroupV2PathOfProcess();
+    if (current_cgroup.empty())
+        return {};
+
+    /// Return the bottom-most nested current memory file. If there is no such file at the current
+    /// level, try again at the parent level as memory settings are inherited.
+    while (current_cgroup != default_cgroups_mount.parent_path())
+    {
+        const auto path = current_cgroup / file_name;
+        if (fs::exists(path))
+            return {current_cgroup};
+        current_cgroup = current_cgroup.parent_path();
+    }
+    return {};
+}

base/base/cgroupsv2.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <filesystem>
+#include <string_view>
 
 #if defined(OS_LINUX)
 /// I think it is possible to mount the cgroups hierarchy somewhere else (e.g. when in containers).
@@ -19,3 +20,7 @@ bool cgroupsV2MemoryControllerEnabled();
 /// Returns an empty path the cgroup cannot be determined.
 /// Assumes that cgroupsV2Enabled() is enabled.
 std::filesystem::path cgroupV2PathOfProcess();
+
+/// Returns the most nested cgroup dir containing the specified file.
+/// If cgroups v2 is not enabled - returns an empty optional.
+std::optional<std::string> getCgroupsV2PathContainingFile(std::string_view file_name);

src/Common/AsynchronousMetrics.cpp

@@ -1,13 +1,13 @@
-#include <chrono>
+#include <Common/AsynchronousMetrics.h>
+
 #include <IO/MMappedFileCache.h>
 #include <IO/ReadHelpers.h>
 #include <IO/UncompressedCache.h>
+#include <base/cgroupsv2.h>
 #include <base/errnoToString.h>
 #include <base/find_symbols.h>
 #include <base/getPageSize.h>
-#include <boost/locale/date_time_facet.hpp>
 #include <sys/resource.h>
-#include <Common/AsynchronousMetrics.h>
 #include <Common/CurrentMetrics.h>
 #include <Common/Exception.h>
 #include <Common/filesystemHelpers.h>
@@ -15,6 +15,11 @@
 #include <Common/logger_useful.h>
 #include <Common/setThreadName.h>
 
+#include <boost/locale/date_time_facet.hpp>
+
+#include <chrono>
+#include <string_view>
+
 #include "config.h"
 
 #if USE_JEMALLOC
@@ -53,6 +58,12 @@ static std::unique_ptr<ReadBufferFromFilePRead> openFileIfExists(const std::stri
     return {};
 }
 
+static void openCgroupv2MetricFile(const std::string & filename, std::optional<ReadBufferFromFilePRead> & out)
+{
+    if (auto path = getCgroupsV2PathContainingFile(filename))
+        openFileIfExists((path.value() + filename).c_str(), out);
+};
+
 #endif
 
 
@@ -73,13 +84,10 @@ AsynchronousMetrics::AsynchronousMetrics(
     openFileIfExists("/proc/net/dev", net_dev);
 
     /// CGroups v2
-    openFileIfExists("/sys/fs/cgroup/memory.max", cgroupmem_limit_in_bytes);
-    if (cgroupmem_limit_in_bytes)
-    {
-        openFileIfExists("/sys/fs/cgroup/memory.current", cgroupmem_usage_in_bytes);
-    }
-    openFileIfExists("/sys/fs/cgroup/cpu.max", cgroupcpu_max);
-    openFileIfExists("/sys/fs/cgroup/cpu.stat", cgroupcpu_stat);
+    openCgroupv2MetricFile("memory.max", cgroupmem_limit_in_bytes);
+    openCgroupv2MetricFile("memory.current", cgroupmem_usage_in_bytes);
+    openCgroupv2MetricFile("cpu.max", cgroupcpu_max);
+    openCgroupv2MetricFile("cpu.stat", cgroupcpu_stat);
 
     /// CGroups v1
     if (!cgroupmem_limit_in_bytes)
@@ -1014,10 +1022,14 @@ void AsynchronousMetrics::update(TimePoint update_time, bool force_update)
 
             if (!first_run)
             {
-                int64_t hz = sysconf(_SC_CLK_TCK);
-                if (-1 == hz)
-                    throw ErrnoException(ErrorCodes::CANNOT_SYSCONF, "Cannot call 'sysconf' to obtain system HZ");
-                const auto cgroup_version_specific_divisor = cgroupcpu_stat ? 1e6 : hz;
+                auto get_clock_ticks = [&]()
+                {
+                    if (auto hz = sysconf(_SC_CLK_TCK); hz != -1)
+                        return hz;
+                    else
+                        throw ErrnoException(ErrorCodes::CANNOT_SYSCONF, "Cannot call 'sysconf' to obtain system HZ");
+                };
+                const auto cgroup_version_specific_divisor = cgroupcpu_stat ? 1e6 : get_clock_ticks();
                 const double multiplier = 1.0 / cgroup_version_specific_divisor
                     / (std::chrono::duration_cast<std::chrono::nanoseconds>(time_since_previous_update).count() / 1e9);
 
@@ -1032,7 +1044,7 @@ void AsynchronousMetrics::update(TimePoint update_time, bool force_update)
         catch (...)
         {
             tryLogCurrentException(__PRETTY_FUNCTION__);
-            openFileIfExists("/sys/fs/cgroup/cpu.stat", cgroupcpu_stat);
+            openCgroupv2MetricFile("cpu.stat", cgroupcpu_stat);
             if (!cgroupcpu_stat)
                 openFileIfExists("/sys/fs/cgroup/cpuacct/cpuacct.stat", cgroupcpuacct_stat);
         }

src/Common/CgroupsMemoryUsageObserver.cpp

@@ -144,31 +144,6 @@ private:
 /// - I did not test what happens if a host has v1 and v2 simultaneously enabled. I believe such
 ///   systems existed only for a short transition period.
 
-std::optional<std::string> getCgroupsV2Path()
-{
-    if (!cgroupsV2Enabled())
-        return {};
-
-    if (!cgroupsV2MemoryControllerEnabled())
-        return {};
-
-    fs::path current_cgroup = cgroupV2PathOfProcess();
-    if (current_cgroup.empty())
-        return {};
-
-    /// Return the bottom-most nested current memory file. If there is no such file at the current
-    /// level, try again at the parent level as memory settings are inherited.
-    while (current_cgroup != default_cgroups_mount.parent_path())
-    {
-        const auto current_path = current_cgroup / "memory.current";
-        const auto stat_path = current_cgroup / "memory.stat";
-        if (fs::exists(current_path) && fs::exists(stat_path))
-            return {current_cgroup};
-        current_cgroup = current_cgroup.parent_path();
-    }
-    return {};
-}
-
 std::optional<std::string> getCgroupsV1Path()
 {
     auto path = default_cgroups_mount / "memory/memory.stat";
@@ -179,7 +154,7 @@ std::optional<std::string> getCgroupsV1Path()
 
 std::pair<std::string, CgroupsMemoryUsageObserver::CgroupsVersion> getCgroupsPath()
 {
-    auto v2_path = getCgroupsV2Path();
+    auto v2_path = getCgroupsV2PathContainingFile("memory.current");
     if (v2_path.has_value())
         return {*v2_path, CgroupsMemoryUsageObserver::CgroupsVersion::V2};