[Core] Fix in-container memory limit fetching for cgroups v2 #23922
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ericl
reviewed
Apr 14, 2022
| @@ -389,7 +389,12 @@ def get_system_memory(): | |||
| docker_limit = int(f.read()) | |||
| elif os.path.exists(memory_limit_filename_v2): | |||
| with open(memory_limit_filename_v2, "r") as f: | |||
| docker_limit = int(f.read()) | |||
| max_file = f.read() | |||
| if max_file.isnumeric(): | |||
There was a problem hiding this comment.
Huh never know this existed.
ericl
approved these changes
Apr 14, 2022
ericl
added
the
@author-action-required
DmitriGekhtman
previously requested changes
Apr 14, 2022
There was a problem hiding this comment.
Thanks for the fix!
Looks like I neglected to test memory in #21688 and only covered CPU.
Would you mind adding a test for the fix in this PR? test_advanced_3 is as good a place as any to put it.
|
LGTM, adding test would be great. |
|
@DmitriGekhtman @richardliaw Tests added. |
|
seems like test failures are unrelated, merging |
richardliaw
deleted the
core/hotfix/fix-container-memory-limit-cgroups-v2
branch
amogkam
pushed a commit
that referenced
this pull request
Apr 19, 2022
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When trying to fetch the in-container memory limit while running Ray in a container using cgroups v2, the limit parsing will fail if the limit is not set, which will prevent Ray from being able to start. This currently happens when trying to run Ray in a Google Colab notebook.
It looks like this memory limit fetching path might be untested? cc @DmitriGekhtman @wuisawesome
Checks
scripts/format.shto lint the changes in this PR.