-
Notifications
You must be signed in to change notification settings - Fork 8k
[Core] (Resource Isolation 13/n) Introduce Multi Memory Monitor Factory to create new memory monitoring system #62705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a17f625
9c7a9ad
64efec0
6fd9952
603d98a
7fdb882
9ef8c05
db6dd99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,11 +48,9 @@ struct SystemMemorySnapshot { | |
| using ProcessesMemorySnapshot = absl::flat_hash_map<pid_t, int64_t>; | ||
|
|
||
| /** | ||
| * @brief Callback that runs at each monitoring interval. | ||
| * | ||
| * \param system_memory snapshot of system memory information. | ||
| * @brief Callback to trigger worker oom killing when under memory pressure. | ||
| */ | ||
| using KillWorkersCallback = std::function<void(SystemMemorySnapshot system_memory)>; | ||
| using KillWorkersCallback = std::function<void()>; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are moving taking the memory snapshot logic to decide how many workers to kill to the node manager callback. This is because there could be a delay between when the memory monitor triggers the callback to when the node manager actually execute the callback. We might to make sure the decision for how many workers to kill is made at the time of the kill. |
||
|
|
||
| /** | ||
| * @brief implementations of this interface monitors the memory usage of the node | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -318,30 +318,26 @@ int64_t MemoryMonitorUtils::GetMemoryThreshold( | |
| } | ||
|
|
||
| if (resource_isolation_enabled) { | ||
| StatusOr<std::string> user_memory_max_bytes_or = | ||
| cgroup_manager.GetUserCgroupConstraintValue("memory.max"); | ||
| RAY_CHECK(user_memory_max_bytes_or.ok()) << absl::StrFormat( | ||
| "Failed to get user cgroup memory limit when setting up memory monitor: %s", | ||
| user_memory_max_bytes_or.ToString()); | ||
| std::string user_memory_max_bytes_str = user_memory_max_bytes_or.value(); | ||
|
|
||
| if (!user_memory_max_bytes_str.empty() && | ||
| std::all_of(user_memory_max_bytes_str.begin(), | ||
| user_memory_max_bytes_str.end(), | ||
| StatusOr<std::string> user_slice_upper_bound_bytes_or = | ||
| cgroup_manager.GetUserCgroupConstraintValue("memory.high"); | ||
| RAY_CHECK(user_slice_upper_bound_bytes_or.ok()) << absl::StrFormat( | ||
| "Failed to get user cgroup memory limit from user cgroup %s " | ||
| "when setting up memory monitor: %s. " | ||
| "Does the cgroup path exist and/or matches the resource isolation hierarchy?", | ||
| cgroup_manager.GetUserCgroupPath(), | ||
| user_slice_upper_bound_bytes_or.ToString()); | ||
| std::string user_slice_upper_bound_bytes_str = | ||
| user_slice_upper_bound_bytes_or.value(); | ||
| RAY_CHECK(!user_slice_upper_bound_bytes_str.empty()) << absl::StrFormat( | ||
| "Failed to get upper bound memory constraints from user cgroup %s. " | ||
| "Does the cgroup path exist and/or matches the resource isolation hierarchy?", | ||
| cgroup_manager.GetUserCgroupPath()); | ||
|
Kunchd marked this conversation as resolved.
|
||
|
|
||
| if (!user_slice_upper_bound_bytes_str.empty() && | ||
| std::all_of(user_slice_upper_bound_bytes_str.begin(), | ||
| user_slice_upper_bound_bytes_str.end(), | ||
| ::isdigit)) { | ||
| int64_t user_memory_max_bytes = std::stoll(user_memory_max_bytes_str); | ||
| int64_t reaction_buffer_bytes = | ||
| std::min(static_cast<int64_t>(total_memory_bytes * | ||
| kDefaultThresholdMonitorReactionBufferProportion), | ||
| RayConfig::instance().max_threshold_monitor_reaction_buffer_bytes()); | ||
| resolved_memory_threshold_bytes = user_memory_max_bytes - reaction_buffer_bytes; | ||
| RAY_CHECK_GE(resolved_memory_threshold_bytes, 0) << absl::StrFormat( | ||
| "Available user task memory is less than the kill memory buffer bytes: " | ||
| "%d < %d. This means the available memory for user proceses is likely " | ||
| "less than 5%% of total memory. Please consider decreasing the proportion " | ||
| "of reserved system memory if it was custom set.", | ||
| user_memory_max_bytes, | ||
| reaction_buffer_bytes); | ||
|
Kunchd marked this conversation as resolved.
|
||
| resolved_memory_threshold_bytes = std::stoll(user_slice_upper_bound_bytes_str); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that we have switched to using |
||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: The event memory monitor will be wired in in a future PR. This PR will simply wire in the threshold monitor to maintain similar behavior with the original default_mode monitoring behavior.