-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Container Awareness #25635
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
base: master
Are you sure you want to change the base?
Container Awareness #25635
Conversation
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.
I am not a zig team member, so feel free to ignore me if you want.
Some of your code is the same in totalSystemMemory
and getCpuCount
. Maybe it makes sense to introduce some cgroup-specific code to avoid this code duplication and enable access to other cgroup files from user code.
I compared this against gnulib nproc.c. Here are some differences:
-
gnulib lets you decide which method(s) to use to get the cpu count.
-
gnulib calls and checks the return value
sched_getscheduler
before using cgroup. I don't know enough about schedulers to know if this should also be done here. -
gnulib also looks at parent cgroups.
-
gnulib looks at
/proc/mounts
to find acgroup2
mount if it is not mounted at/sys/fs/cgroup
.
I would also like to get rid of the code duplication but I don't really know where a good place would be to place something like this, since these helper functions should probably not be available publicly. Can you think of something were some helper functions could be placed?
Since the interface should be the same across different OSes I did not know how one could specify some custom options which are only valid on linux unless one introduces something like an option parameter leading to a breaking change.
In most containerization scenarios this won't be possible due to namespacing but in those other scenarios it is possible so adding it here might not hurt either.
My thought was that it might be too much of an overhead to open another file and proccess it for just getting the cpu count but it might be better than relying on the convetion to mount the cgroup2 fs to /sys/fs/cgroup
It would make sense to check if the process is scheduled 'fairly' otherwise the cpu.max file will be ignored. On the other hand I feel like that someone who is cares about the scheduling of their process in that detail would not use a generic getCpuCount() function. Also this adds another call which for most people won't really do much. @rpkak thanks for taking the time. |
The CPU controller can also be used in threaded mode so theoretically different threads of a process can have different cpu.limits but taking this into account seems overkill. |
Content
This implements the cgroup awareness mentioned in #25491.
Things I'm unsure about:
std.fs.openFileAbsolute
instead ofposix.open
other than that everything is implemented as stated in the issue.
Testing It
This outlines how one can test the changes without playing around with the cgroup fs manually.
Building The Binary
Building The OCI Image
docker build -t testimage --build-arg BINARY_NAME=test .
Testing It