Support cgroups v1 and v2 for memory limits #1286
Draft
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
colincasey
requested changes
Jun 27, 2024
1888864 to
2771b81
Compare
Uses memory.high if available (recommended way of setting a functioning soft limit), then first falls back to memory.max (e.g. from 'docker run -m'), then to memory.low (e.g. from 'docker run --memory-reservation'), and finally to memory.min. Falls back to direct reading of '/sys/fs/cgroup/memory/memory.limit_in_bytes' for cases where that exists, but no full cgroupfs is mounted (e.g. on Heroku). Limit enforcement (now to 8 TB) is still in place this way - a Docker v1 value will be read, and run into the limit for unrestricted containers, without hitting the fallback and getting returned. GUS-W-16052317
From https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idcontainer: > Note: The default shell for run steps inside a container is sh instead of bash. This can be overridden with jobs.<job_id>.defaults.run or jobs.<job_id>.steps[*].shell.
colincasey
reviewed
Jul 2, 2024
| fi | ||
| } | ||
|
|
||
| dne_memory() { |
There was a problem hiding this comment.
what does dne stand for?
| return | ||
| } | ||
|
|
||
| if (($? == 99)); then |
There was a problem hiding this comment.
a brief comment about exit code 99 would be nice
colincasey
reviewed
Jul 2, 2024
There was a problem hiding this comment.
@dzuelke this file contains all the "shared" logic between buildpacks, right? if changes occur will we be manually keeping these in sync? is the PHP version the canonical one since it contains most of the underlying tests?
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.
For v2, it uses
memory.highif available (recommended way of setting a functioning soft limit), then first falls back tomemory.max(e.g. fromdocker run -m), then tomemory.low(e.g. fromdocker run --memory-reservation), and finally tomemory.min.Falls back to direct reading of
/sys/fs/cgroup/memory/memory.limit_in_bytesfor cases where that exists, but no full cgroupfs is mounted (e.g. on Heroku).Limit enforcement (now to 8 TB) is still in place this way - a Docker v1 value will be read, and run into the limit for unrestricted containers, without hitting the fallback and getting returned.
GUS-W-16052317