diff --git a/src/ray/common/cgroup2/BUILD.bazel b/src/ray/common/cgroup2/BUILD.bazel index 5eaf9e515088..a3810ff70f58 100644 --- a/src/ray/common/cgroup2/BUILD.bazel +++ b/src/ray/common/cgroup2/BUILD.bazel @@ -5,26 +5,34 @@ config_setting( constraint_values = ["@platforms//os:linux"], ) -# Public targets. +# The module exposes only two public targets. +# "cgroup_manager_factory" to create a CgroupManager +# "cgroup_manager_interface" to use the public API of CgroupManager. ray_cc_library( - name = "cgroup_manager", + name = "cgroup_manager_factory", srcs = select({ - ":is_linux": ["cgroup_manager.cc"], - "//conditions:default": ["noop_cgroup_manager.cc"], + ":is_linux": [ + "linux_cgroup_manager_factory.cc", + ], + "//conditions:default": [ + "noop_cgroup_manager_factory.cc", + ], }), hdrs = [ - "cgroup_manager.h", - "scoped_cgroup_operation.h", + "cgroup_manager_factory.h", ], visibility = ["//visibility:public"], deps = [ - ":cgroup_driver_interface", ":cgroup_manager_interface", - "//src/ray/common:status", - "//src/ray/common:status_or", + ":noop_cgroup_manager", + "//src/ray/util:logging", ] + select({ ":is_linux": [ - "//src/ray/util:logging", + ":cgroup_driver_interface", + ":cgroup_manager", + ":sysfs_cgroup_driver", + "//src/ray/common:status", + "//src/ray/common:status_or", "@com_google_absl//absl/strings", ], "//conditions:default": [], @@ -32,77 +40,75 @@ ray_cc_library( ) ray_cc_library( - name = "cgroup_driver_interface", + name = "cgroup_manager_interface", hdrs = [ - "cgroup_driver_interface.h", + "cgroup_manager_interface.h", ], visibility = ["//visibility:public"], deps = [ + ":cgroup_driver_interface", "//src/ray/common:status", "//src/ray/common:status_or", ], ) +# Private targets ray_cc_library( - name = "cgroup_manager_interface", + name = "cgroup_manager", + srcs = [ + "cgroup_manager.cc", + ], hdrs = [ - "cgroup_manager_interface.h", + "cgroup_manager.h", + "scoped_cgroup_operation.h", ], - visibility = ["//visibility:public"], + visibility = [":__subpackages__"], deps = [ ":cgroup_driver_interface", + ":cgroup_manager_interface", "//src/ray/common:status", "//src/ray/common:status_or", + "//src/ray/util:logging", ], ) ray_cc_library( - name = "sysfs_cgroup_driver", - srcs = select({ - ":is_linux": ["sysfs_cgroup_driver.cc"], - "//conditions:default": ["noop_sysfs_cgroup_driver.cc"], - }), + name = "noop_cgroup_manager", hdrs = [ - "sysfs_cgroup_driver.h", + "noop_cgroup_manager.h", ], - visibility = ["//visibility:public"], + visibility = [":__subpackages__"], deps = [ ":cgroup_driver_interface", + ":cgroup_manager_interface", "//src/ray/common:status", "//src/ray/common:status_or", - ] + select({ - ":is_linux": [ - "//src/ray/util:logging", - "@com_google_absl//absl/strings", - ], - "//conditions:default": [], - }), + ], ) -# Private targets -# -# TODO(#54703): This target builds the noop implementations. -# There's a corressponding test that runs on Linux and Non-Linux -# CI so breakages are caught in premerge before these targets are -# cleaned up at the end of the resource isolation milestone 1 -# project. ray_cc_library( - name = "noop_cgroup_targets", - srcs = [ - "noop_cgroup_manager.cc", - "noop_sysfs_cgroup_driver.cc", - ], + name = "cgroup_driver_interface", hdrs = [ - "cgroup_manager.h", - "scoped_cgroup_operation.h", - "sysfs_cgroup_driver.h", + "cgroup_driver_interface.h", ], visibility = [":__subpackages__"], + deps = [ + "//src/ray/common:status", + "//src/ray/common:status_or", + ], +) + +ray_cc_library( + name = "sysfs_cgroup_driver", + srcs = ["sysfs_cgroup_driver.cc"], + hdrs = ["sysfs_cgroup_driver.h"], + visibility = [":__subpackages__"], deps = [ ":cgroup_driver_interface", - ":cgroup_manager_interface", "//src/ray/common:status", "//src/ray/common:status_or", + "//src/ray/util:logging", + "@com_google_absl//absl/strings", ], ) diff --git a/src/ray/common/cgroup2/cgroup_manager.h b/src/ray/common/cgroup2/cgroup_manager.h index 3693d3fcfa08..21250850ba46 100644 --- a/src/ray/common/cgroup2/cgroup_manager.h +++ b/src/ray/common/cgroup2/cgroup_manager.h @@ -78,10 +78,7 @@ class CgroupManager : public CgroupManagerInterface { 2) the system leaf cgroup i.e. the destination cgroup. 3) the lowest common ancestor of the source and destination cgroups. - TODO(#54703): There currently is not a good way to signal to the caller that - the method can cause a FATAL error. Revisit this once we've settled on a pattern. - - NOTE: If the process does not have adequate cgroup permissions or the application leaf + @note If the process does not have adequate cgroup permissions or the application leaf cgroup does not exist, this will fail a RAY_CHECK. @param pid of the process to move into the application leaf cgroup. diff --git a/src/ray/common/cgroup2/cgroup_manager_factory.h b/src/ray/common/cgroup2/cgroup_manager_factory.h new file mode 100644 index 000000000000..5f4a6df56c8d --- /dev/null +++ b/src/ray/common/cgroup2/cgroup_manager_factory.h @@ -0,0 +1,69 @@ +// Copyright 2025 The Ray Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#pragma once + +#include +#include + +#include "ray/common/cgroup2/cgroup_manager_interface.h" + +namespace ray { + +// TODO(54703): Refactor the configs into a struct called CgroupManagerConfig +// and delegate input validation and error messages to it. +class CgroupManagerFactory { + public: + /** + + This feature is only enabled in Linux. If using Linux, validates inputs, creates the + ray cgroup heirarchy, enables constraints, and moves all system processes into the + system cgroup. + + On non-Linux platforms, this will return a noop implementation. + + @param enable_resource_isolation if true, will create process isolation with using + cgroups (@see CgroupManager::Create for more information). + @param cgroup_path the cgroup that the process will take ownership of. + @param node_id used to create a unique cgroup subtree per running ray node. + @param system_reserved_cpu_weight a value between [1,10000] to assign to the cgroup + for system processes. The cgroup for application processes gets 10000 - + system_reserved_cpu_weight. + @param system_reserved_memory_bytes used to reserve memory for the system cgroup. + @param system_pids a comma-separated list of pids of ray system processes to move into + the system cgroup. + + For more information about the parameters, see @ref CgroupManager::Create. + + @note any of the following is undefined behavior and will cause a RAY_CHECK to fail + 1. enable_resource_isolation is true and either + a. cgroup_path is empty + b. system_reserved_cpu_weight or system_reserved_memory_bytes are -1. + 2. The CgroupManager's precondition checks fail + a. cgroupv2 is not mounted correctly in unified mode (see @ref + CgroupDriverInterface::CheckCgroupv2Enabled). + b. the current process does not adequate permissions (see @ref + CgroupManager::Create). + c. supported cgroup controllers are not available (see @ref + CgroupManager::supported_controllers_). + 3. if a process in system_pids cannot be moved into the system cgroup. + */ + static std::unique_ptr Create( + bool enable_resource_isolation, + std::string cgroup_path, + const std::string &node_id, + const int64_t system_reserved_cpu_weight, + const int64_t system_reserved_memory_bytes, + const std::string &system_pids); +}; +} // namespace ray diff --git a/src/ray/common/cgroup2/cgroup_manager_interface.h b/src/ray/common/cgroup2/cgroup_manager_interface.h index 096979d436a4..c007dacdfc34 100644 --- a/src/ray/common/cgroup2/cgroup_manager_interface.h +++ b/src/ray/common/cgroup2/cgroup_manager_interface.h @@ -13,8 +13,6 @@ // limitations under the License. #pragma once -#include - #include #include #include @@ -50,10 +48,7 @@ class CgroupManagerInterface { 2) the system leaf cgroup i.e. the destination cgroup. 3) the lowest common ancestor of the source and destination cgroups. - TODO(#54703): There currently is not a good way to signal to the caller that - the method can cause a FATAL error. Revisit this once we've settled on a pattern. - - NOTE: If the process does not have adequate cgroup permissions or the application leaf + @note If the process does not have adequate cgroup permissions or the application leaf cgroup does not exist, this will fail a RAY_CHECK. @param pid of the process to move into the system leaf cgroup. diff --git a/src/ray/common/cgroup2/linux_cgroup_manager_factory.cc b/src/ray/common/cgroup2/linux_cgroup_manager_factory.cc new file mode 100644 index 000000000000..cce5e0551c43 --- /dev/null +++ b/src/ray/common/cgroup2/linux_cgroup_manager_factory.cc @@ -0,0 +1,83 @@ +// Copyright 2025 The Ray Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include +#include + +#include +#include +#include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_split.h" +#include "ray/common/cgroup2/cgroup_driver_interface.h" +#include "ray/common/cgroup2/cgroup_manager.h" +#include "ray/common/cgroup2/cgroup_manager_factory.h" +#include "ray/common/cgroup2/cgroup_manager_interface.h" +#include "ray/common/cgroup2/noop_cgroup_manager.h" +#include "ray/common/cgroup2/sysfs_cgroup_driver.h" + +namespace ray { + +std::unique_ptr CgroupManagerFactory::Create( + bool enable_resource_isolation, + std::string cgroup_path, + const std::string &node_id, + const int64_t system_reserved_cpu_weight, + const int64_t system_reserved_memory_bytes, + const std::string &system_pids) { + if (!enable_resource_isolation) { + return std::make_unique(); + } + + RAY_CHECK(!cgroup_path.empty()) + << "Failed to start CgroupManager. If enable_resource_isolation is set to true, " + "cgroup_path cannot be empty."; + + RAY_CHECK_NE(system_reserved_cpu_weight, -1) + << "Failed to start CgroupManager. If enable_resource_isolation is set to true, " + "system_reserved_cpu_weight must be set to a value between [1,10000]"; + + RAY_CHECK_NE(system_reserved_memory_bytes, -1) + << "Failed to start CgroupManager. If enable_resource_isolation is set to true, " + "system_reserved_memory_bytes must be set to a value > 0"; + + StatusOr> cgroup_manager_s = + CgroupManager::Create(cgroup_path, + node_id, + system_reserved_cpu_weight, + system_reserved_memory_bytes, + std::make_unique()); + + RAY_CHECK(cgroup_manager_s.ok()) << absl::StrFormat( + "Failed to start CgroupManager due to %s.", cgroup_manager_s.ToString()); + + std::unique_ptr cgroup_manager = + std::move(cgroup_manager_s.value()); + + std::vector system_pids_to_move; + if (!system_pids.empty()) { + system_pids_to_move = std::move(absl::StrSplit(system_pids, ",")); + } + + system_pids_to_move.emplace_back(std::to_string(getpid())); + + for (const auto &pid : system_pids_to_move) { + RAY_CHECK_OK(cgroup_manager->AddProcessToSystemCgroup(pid)) + << absl::StrFormat("Failed to move process with pid %s into system cgroup.", pid); + } + + return cgroup_manager; +} +} // namespace ray diff --git a/src/ray/common/cgroup2/noop_cgroup_manager.cc b/src/ray/common/cgroup2/noop_cgroup_manager.cc deleted file mode 100644 index 6caf59092acb..000000000000 --- a/src/ray/common/cgroup2/noop_cgroup_manager.cc +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2025 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -#include -#include -#include - -#include "ray/common/cgroup2/cgroup_driver_interface.h" -#include "ray/common/cgroup2/cgroup_manager.h" -#include "ray/common/status_or.h" - -namespace ray { - -CgroupManager::CgroupManager(std::string base_cgroup_path, - const std::string &node_id, - std::unique_ptr cgroup_driver) {} - -CgroupManager::~CgroupManager() {} - -StatusOr> CgroupManager::Create( - std::string base_cgroup_path, - const std::string &node_id, - const int64_t system_reserved_cpu_weight, - const int64_t system_reserved_memory_bytes, - std::unique_ptr cgroup_driver) { - return std::unique_ptr( - new CgroupManager(base_cgroup_path, node_id, std::move(cgroup_driver))); -} - -Status CgroupManager::AddProcessToSystemCgroup(const std::string &pid) { - return Status::OK(); -} - -Status CgroupManager::AddProcessToApplicationCgroup(const std::string &pid) { - return Status::OK(); -} - -} // namespace ray diff --git a/src/ray/common/cgroup2/noop_cgroup_manager.h b/src/ray/common/cgroup2/noop_cgroup_manager.h new file mode 100644 index 000000000000..423de472be8d --- /dev/null +++ b/src/ray/common/cgroup2/noop_cgroup_manager.h @@ -0,0 +1,42 @@ +// Copyright 2025 The Ray Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#pragma once + +#include +#include + +#include "ray/common/cgroup2/cgroup_manager_interface.h" +#include "ray/common/status.h" +#include "ray/common/status_or.h" + +namespace ray { +class NoopCgroupManager : public CgroupManagerInterface { + public: + // Uncopyable type. + NoopCgroupManager() = default; + explicit NoopCgroupManager(const NoopCgroupManager &) = delete; + NoopCgroupManager &operator=(const NoopCgroupManager &) = delete; + explicit NoopCgroupManager(NoopCgroupManager &&) {} + NoopCgroupManager &operator=(NoopCgroupManager &&) { return *this; } + ~NoopCgroupManager() = default; + + Status AddProcessToApplicationCgroup(const std::string &pid) override { + return Status::OK(); + } + + Status AddProcessToSystemCgroup(const std::string &pid) override { + return Status::OK(); + } +}; // namespace ray +} // namespace ray diff --git a/src/ray/common/cgroup2/noop_cgroup_manager_factory.cc b/src/ray/common/cgroup2/noop_cgroup_manager_factory.cc new file mode 100644 index 000000000000..d418a911f36a --- /dev/null +++ b/src/ray/common/cgroup2/noop_cgroup_manager_factory.cc @@ -0,0 +1,38 @@ +// Copyright 2025 The Ray Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#include +#include + +#include "ray/common/cgroup2/cgroup_manager_factory.h" +#include "ray/common/cgroup2/cgroup_manager_interface.h" +#include "ray/common/cgroup2/noop_cgroup_manager.h" + +namespace ray { + +std::unique_ptr CgroupManagerFactory::Create( + bool enable_resource_isolation, + std::string cgroup_path, + const std::string &node_id, + const int64_t system_reserved_cpu_weight, + const int64_t system_reserved_memory_bytes, + const std::string &system_pids) { + if (enable_resource_isolation) { + // TODO(54703): Add link to OSS documentation when ready. + RAY_LOG(WARNING) + << "Raylet started with --enable_resource_isolation. Resource isolation is only " + "supported on Linux. This is likey a misconfiguration."; + } + return std::make_unique(); +} +} // namespace ray diff --git a/src/ray/common/cgroup2/noop_sysfs_cgroup_driver.cc b/src/ray/common/cgroup2/noop_sysfs_cgroup_driver.cc deleted file mode 100644 index 40eb13e12cf5..000000000000 --- a/src/ray/common/cgroup2/noop_sysfs_cgroup_driver.cc +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright 2025 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include -#include - -#include "ray/common/cgroup2/sysfs_cgroup_driver.h" -#include "ray/common/status.h" -#include "ray/common/status_or.h" - -namespace ray { -Status SysFsCgroupDriver::CheckCgroupv2Enabled() { return Status::OK(); } - -Status SysFsCgroupDriver::CheckCgroup(const std::string &cgroup_path) { - return Status::OK(); -} - -Status SysFsCgroupDriver::CreateCgroup(const std::string &cgroup_path) { - return Status::OK(); -} - -Status SysFsCgroupDriver::DeleteCgroup(const std::string &cgroup_path) { - return Status::OK(); -} - -StatusOr> SysFsCgroupDriver::GetAvailableControllers( - const std::string &cgroup_dir) { - return std::unordered_set{}; -} - -StatusOr> SysFsCgroupDriver::GetEnabledControllers( - const std::string &cgroup_dir) { - return std::unordered_set{}; -} - -Status SysFsCgroupDriver::MoveAllProcesses(const std::string &from, - const std::string &to) { - return Status::OK(); -} - -Status SysFsCgroupDriver::EnableController(const std::string &cgroup_path, - const std::string &controller) { - return Status::OK(); -} - -Status SysFsCgroupDriver::DisableController(const std::string &cgroup_path, - const std::string &controller) { - return Status::OK(); -} - -Status SysFsCgroupDriver::AddConstraint(const std::string &cgroup_path, - const std::string &controller, - const std::string &constraint, - const std::string &constraint_value) { - return Status::OK(); -} - -StatusOr> SysFsCgroupDriver::ReadControllerFile( - const std::string &controller_file_path) { - return std::unordered_set{}; -} - -Status SysFsCgroupDriver::AddProcessToCgroup(const std::string &cgroup, - const std::string &process) { - return Status::OK(); -} - -} // namespace ray diff --git a/src/ray/common/cgroup2/sysfs_cgroup_driver.h b/src/ray/common/cgroup2/sysfs_cgroup_driver.h index 1d0be4904230..90b060d7476e 100644 --- a/src/ray/common/cgroup2/sysfs_cgroup_driver.h +++ b/src/ray/common/cgroup2/sysfs_cgroup_driver.h @@ -13,10 +13,7 @@ // limitations under the License. #pragma once -// TODO(#54703): SysFsCgroupDriver should not be a public target. -// It will be hidden behind a CgroupManagerFactory which will create -// an appropriate depending on configuration and platform. -// #include +#include #include #include @@ -44,7 +41,7 @@ class SysFsCgroupDriver : public CgroupDriverInterface { /** * @param mount_file_path only used for testing. */ - explicit SysFsCgroupDriver(std::string mount_file_path = kMountFilePath) + explicit SysFsCgroupDriver(std::string mount_file_path = MOUNTED) : mount_file_path_(std::move(mount_file_path)) {} ~SysFsCgroupDriver() override = default; @@ -296,6 +293,5 @@ class SysFsCgroupDriver : public CgroupDriverInterface { static constexpr std::string_view kCgroupSubtreeControlFilename = "cgroup.subtree_control"; static constexpr std::string_view kCgroupControllersFilename = "cgroup.controllers"; - static inline std::string kMountFilePath = "/etc/mtab"; }; } // namespace ray diff --git a/src/ray/common/cgroup2/tests/BUILD.bazel b/src/ray/common/cgroup2/tests/BUILD.bazel index a910059d6adf..06d0ca6d1221 100644 --- a/src/ray/common/cgroup2/tests/BUILD.bazel +++ b/src/ray/common/cgroup2/tests/BUILD.bazel @@ -39,20 +39,3 @@ ray_cc_test( "@com_google_googletest//:gtest_main", ], ) - -# TODO(#54703): This target builds the noop implementations. -# There's a corressponding test that runs on Linux and Non-Linux -# CI so breakages are caught in premerge before these targets are -# cleaned up at the end of the resource isolation milestone 1 -# project. -ray_cc_test( - name = "noop_cgroup_test", - srcs = ["noop_cgroup_test.cc"], - tags = [ - "team:core", - ], - deps = [ - "//src/ray/common/cgroup2:noop_cgroup_targets", - "@com_google_googletest//:gtest_main", - ], -) diff --git a/src/ray/common/cgroup2/tests/noop_cgroup_test.cc b/src/ray/common/cgroup2/tests/noop_cgroup_test.cc deleted file mode 100644 index 705c6d6e8349..000000000000 --- a/src/ray/common/cgroup2/tests/noop_cgroup_test.cc +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2025 The Ray Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include -#include - -#include "gtest/gtest.h" -#include "ray/common/cgroup2/cgroup_manager.h" -#include "ray/common/cgroup2/sysfs_cgroup_driver.h" -#include "ray/common/status.h" -namespace ray { - -TEST(NoopCgroupTest, NoopCgroupDriverAndManagerBuildSuccessfullyOnAllPlatforms) { - std::unique_ptr sysfs_cgroup_driver = - std::make_unique(); - auto cgroup_manager = - CgroupManager::Create("", "", 1, 1, std::move(sysfs_cgroup_driver)); -} - -} // namespace ray diff --git a/src/ray/raylet/BUILD.bazel b/src/ray/raylet/BUILD.bazel index d038150f15c4..a95b3c6693d3 100644 --- a/src/ray/raylet/BUILD.bazel +++ b/src/ray/raylet/BUILD.bazel @@ -221,6 +221,7 @@ ray_cc_library( "//src/ray/common:flatbuf_utils", "//src/ray/common:lease", "//src/ray/common:memory_monitor", + "//src/ray/common/cgroup2:cgroup_manager_interface", "//src/ray/core_worker:experimental_mutable_object_provider", "//src/ray/core_worker_rpc_client:core_worker_client_pool", "//src/ray/flatbuffers:node_manager_generated", @@ -276,8 +277,8 @@ ray_cc_binary( "//src/ray/common:lease", "//src/ray/common:ray_config", "//src/ray/common:status", - "//src/ray/common/cgroup2:cgroup_manager", - "//src/ray/common/cgroup2:sysfs_cgroup_driver", + "//src/ray/common/cgroup2:cgroup_manager_factory", + "//src/ray/common/cgroup2:cgroup_manager_interface", "//src/ray/core_worker:metrics", "//src/ray/core_worker_rpc_client:core_worker_client", "//src/ray/core_worker_rpc_client:core_worker_client_pool", diff --git a/src/ray/raylet/main.cc b/src/ray/raylet/main.cc index e35f1322b561..b99ab969c0e8 100644 --- a/src/ray/raylet/main.cc +++ b/src/ray/raylet/main.cc @@ -21,12 +21,12 @@ #include #include -#include "absl/strings/str_split.h" +#include "absl/strings/str_format.h" #include "gflags/gflags.h" #include "nlohmann/json.hpp" #include "ray/common/asio/instrumented_io_context.h" -#include "ray/common/cgroup2/cgroup_manager.h" -#include "ray/common/cgroup2/sysfs_cgroup_driver.h" +#include "ray/common/cgroup2/cgroup_manager_factory.h" +#include "ray/common/cgroup2/cgroup_manager_interface.h" #include "ray/common/constants.h" #include "ray/common/id.h" #include "ray/common/lease/lease.h" @@ -268,88 +268,27 @@ int main(int argc, char *argv[]) { RAY_LOG(INFO) << "Setting cluster ID to: " << cluster_id; gflags::ShutDownCommandLineFlags(); - std::unique_ptr cgroup_manager; - AddProcessToCgroupHook add_process_to_cgroup_hook = [](const std::string &) {}; + // Setting up resource isolation with cgroups. + // The lifecycle of CgroupManager will be controlled by NodeManager. + std::unique_ptr cgroup_manager = + ray::CgroupManagerFactory::Create(enable_resource_isolation, + std::move(cgroup_path), + node_id, + system_reserved_cpu_weight, + system_reserved_memory_bytes, + system_pids); + AddProcessToCgroupHook add_process_to_application_cgroup_hook = - [](const std::string &) {}; - AddProcessToCgroupHook add_process_to_system_cgroup_hook = [](const std::string &) {}; - - // TODO(#54703): Link OSS documentation once it's available in the error messages. - if (enable_resource_isolation) { - RAY_CHECK(!cgroup_path.empty()) - << "Failed to start up raylet. If enable_resource_isolation is set to true, " - "cgroup_path cannot be empty."; - RAY_CHECK_NE(system_reserved_cpu_weight, -1) - << "Failed to start up raylet. If enable_resource_isolation is set to true, " - "system_reserved_cpu_weight must be set to a value between [1,10000]"; - RAY_CHECK_NE(system_reserved_memory_bytes, -1) - << "Failed to start up raylet. If enable_resource_isolation is set to true, " - "system_reserved_memory_bytes must be set to a value > 0"; - - std::unique_ptr cgroup_driver = - std::make_unique(); - - ray::StatusOr> cgroup_manager_s = - ray::CgroupManager::Create(std::move(cgroup_path), - node_id, - system_reserved_cpu_weight, - system_reserved_memory_bytes, - std::move(cgroup_driver)); - - // TODO(#54703) - Link to OSS documentation once available. - RAY_CHECK(cgroup_manager_s.ok()) - << "Failed to start raylet. Could not create CgroupManager because of " - << cgroup_manager_s.ToString(); - - cgroup_manager = std::move(cgroup_manager_s.value()); - -#ifndef __linux__ - RAY_LOG(WARNING) - << "Resource isolation with cgroups is only supported in linux. Please set " - "enable_resource_isolation to false. This is likely a misconfiguration."; -#endif + [&cgroup_mgr = *cgroup_manager](const std::string &pid) { + RAY_CHECK_OK(cgroup_mgr.AddProcessToApplicationCgroup(pid)) << absl::StrFormat( + "Failed to move process %s into the application cgroup.", pid); + }; - // Move system processes into the system cgroup. - // TODO(#54703): This logic needs to be hardened and moved out of main.cc. E.g. - // if system_pids is ",,,,,,", this will log an error for each empty - // string. - std::vector system_pids_to_move; - if (!system_pids.empty()) { - system_pids_to_move = std::move(absl::StrSplit(system_pids, ",")); - } - system_pids_to_move.emplace_back(std::to_string(ray::GetPID())); - for (const auto &pid : system_pids_to_move) { - ray::Status s = cgroup_manager->AddProcessToSystemCgroup(pid); - // TODO(#54703): This could be upgraded to a RAY_CHECK. - if (!s.ok()) { - RAY_LOG(WARNING) << absl::StrFormat( - "Failed to move process %s into system cgroup with error %s", - pid, - s.ToString()); - } - } - add_process_to_application_cgroup_hook = - [&cgroup_mgr = *cgroup_manager](const std::string &pid) { - ray::Status s = cgroup_mgr.AddProcessToApplicationCgroup(pid); - if (!s.ok()) { - RAY_LOG(WARNING) << absl::StrFormat( - "Failed to move process %s into the application cgroup with error %s.", - pid, - s.ToString()); - } - }; - - add_process_to_system_cgroup_hook = [&cgroup_mgr = - *cgroup_manager](const std::string &pid) { - ray::Status s = cgroup_mgr.AddProcessToSystemCgroup(pid); - if (!s.ok()) { - RAY_LOG(WARNING) << absl::StrFormat( - "Failed to move process %s into the system cgroup with error %s.", - pid, - s.ToString()); - } - }; - } + AddProcessToCgroupHook add_process_to_system_cgroup_hook = + [&cgroup_mgr = *cgroup_manager](const std::string &pid) { + RAY_CHECK_OK(cgroup_mgr.AddProcessToSystemCgroup(pid)) << absl::StrFormat( + "Failed to move process %s into the system cgroup with error.", pid); + }; // Configuration for the node manager. ray::raylet::NodeManagerConfig node_manager_config; @@ -999,7 +938,8 @@ int main(int argc, char *argv[]) { std::move(raylet_client_factory), /*check_signals=*/nullptr), shutdown_raylet_gracefully, - std::move(add_process_to_system_cgroup_hook)); + std::move(add_process_to_system_cgroup_hook), + std::move(cgroup_manager)); // Initialize the node manager. raylet = std::make_unique(main_service, diff --git a/src/ray/raylet/node_manager.cc b/src/ray/raylet/node_manager.cc index 441ab68b10a8..4ef68f212a28 100644 --- a/src/ray/raylet/node_manager.cc +++ b/src/ray/raylet/node_manager.cc @@ -32,6 +32,7 @@ #include "ray/common/asio/asio_util.h" #include "ray/common/asio/instrumented_io_context.h" #include "ray/common/buffer.h" +#include "ray/common/cgroup2/cgroup_manager_interface.h" #include "ray/common/constants.h" #include "ray/common/flatbuf_utils.h" #include "ray/common/grpc_util.h" @@ -144,7 +145,8 @@ NodeManager::NodeManager( std::unique_ptr mutable_object_provider, std::function shutdown_raylet_gracefully, - AddProcessToCgroupHook add_process_to_system_cgroup_hook) + AddProcessToCgroupHook add_process_to_system_cgroup_hook, + std::unique_ptr cgroup_manager) : self_node_id_(self_node_id), self_node_name_(std::move(self_node_name)), io_service_(io_service), @@ -196,7 +198,8 @@ NodeManager::NodeManager( RayConfig::instance().min_memory_free_bytes(), RayConfig::instance().memory_monitor_refresh_ms(), CreateMemoryUsageRefreshCallback())), - add_process_to_system_cgroup_hook_(std::move(add_process_to_system_cgroup_hook)) { + add_process_to_system_cgroup_hook_(std::move(add_process_to_system_cgroup_hook)), + cgroup_manager_(std::move(cgroup_manager)) { RAY_LOG(INFO).WithField(kLogKeyNodeID, self_node_id_) << "Initializing NodeManager"; placement_group_resource_manager_ = @@ -2575,7 +2578,6 @@ void NodeManager::HandleGetNodeStats(rpc::GetNodeStatsRequest node_stats_request } namespace { - rpc::ObjectStoreStats AccumulateStoreStats( const std::vector &node_stats) { rpc::ObjectStoreStats store_stats; diff --git a/src/ray/raylet/node_manager.h b/src/ray/raylet/node_manager.h index 232fe56ca3e9..6498219b24a8 100644 --- a/src/ray/raylet/node_manager.h +++ b/src/ray/raylet/node_manager.h @@ -23,6 +23,7 @@ #include "ray/common/asio/instrumented_io_context.h" #include "ray/common/bundle_spec.h" +#include "ray/common/cgroup2/cgroup_manager_interface.h" #include "ray/common/id.h" #include "ray/common/lease/lease.h" #include "ray/common/memory_monitor.h" @@ -153,7 +154,8 @@ class NodeManager : public rpc::NodeManagerServiceHandler, std::unique_ptr mutable_object_provider, std::function shutdown_raylet_gracefully, - AddProcessToCgroupHook add_process_to_system_cgroup_hook); + AddProcessToCgroupHook add_process_to_system_cgroup_hook, + std::unique_ptr cgroup_manager); /// Handle an unexpected error that occurred on a client connection. /// The client will be disconnected and no more messages will be processed. @@ -883,6 +885,9 @@ class NodeManager : public rpc::NodeManagerServiceHandler, /// Used to move the dashboard and runtime_env agents into the system cgroup. AddProcessToCgroupHook add_process_to_system_cgroup_hook_; + + // Controls the lifecycle of the CgroupManager. + std::unique_ptr cgroup_manager_; }; } // namespace ray::raylet diff --git a/src/ray/raylet/tests/node_manager_test.cc b/src/ray/raylet/tests/node_manager_test.cc index 3900eebdc967..c93710b817ab 100644 --- a/src/ray/raylet/tests/node_manager_test.cc +++ b/src/ray/raylet/tests/node_manager_test.cc @@ -418,7 +418,8 @@ class NodeManagerTest : public ::testing::Test { std::move(mutable_object_provider), /*shutdown_raylet_gracefully=*/ [](const auto &) {}, - [](const std::string &) {}); + [](const std::string &) {}, + nullptr); } instrumented_io_context io_service_;