-
Notifications
You must be signed in to change notification settings - Fork 260
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
Implement cgroup v2 #2588
Implement cgroup v2 #2588
Conversation
3793aa6
to
3c426ec
Compare
f65f320
to
ce67e8b
Compare
There are three TODOs that I need to think a little bit more about, but in general it is ready for review (and works both on my laptop, a desktop and on CI). |
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.
We lose some of the other integration tests, can you leave the gitlab configs for now so we can add those back in the future?
You checked if the cgroupv1 also still worked?
Happy to revert what needs to be reverted but I fail to see what we are losing.
Locally: yes. |
The GitLab version tested:
I think not all were copied over and would require a matrix job. |
Since judging was basically disabled on gitlab we didn't get a lot of use from the matrix |
I agree, but I think those tests found errors in the past. But if you think they're not needed let's leave it like this. |
The most important part is testing this PR to figure out where it works and where it doesn't. @nickygerritsen already tested on Mac plus docker and we have other people who committed to testing on Debian stable, fedora and Arch. |
I tried to run this on my Arch Linux laptop using
However, I do have When I change `exit 1` to `exit 0` in `judge/create_cgroups.in`, the webapp starts, but the judgedaemons fail to start.diff --git a/judge/create_cgroups.in b/judge/create_cgroups.in
index 56d1338a3..a36eb8c00 100755
--- a/judge/create_cgroups.in
+++ b/judge/create_cgroups.in
@@ -16,8 +16,9 @@ cgroup_error_and_usage () {
On modern distros (e.g. Debian bullseye and Ubuntu Jammy Jellyfish) which have cgroup v2 enabled by default,
you need to add 'systemd.unified_cgroup_hierarchy=0' as well.
2. Run update-grub
- 3. Reboot" >&2
- exit 1
+ 3. Reboot
+ (ignoring for now, hehe)" >&2
+ exit 0
}
for i in cpuset memory; do It looks like I cannot run cgroups v2 without also having support for v1, unless I'm missing some configuration setting somewhere 😛 I'm guessing that |
The create_cgroups script should not be needed. I commented out the whole thing on my docker setup and that worked. What error is the judgedaemon giving you? |
In my
However, when looking in However, when I try to upload a contest on this fresh DB state, I get three times the following judgehost error, on the first submission it tries to run for each of the languages C++, Python, and Java:
|
@mpsijm thanks for testing, can you add |
There you go, some debug output:
|
I did some initial testing and can reproduce the debug output in a container. As far as I understand, moving the process out of the container slice is not permitted. When mounting the complete cgroupfs, the processes are already running in a cgroup:
Manually creating a Processes in the container think that they are in the cgroup:
If we do not mount |
If you are running docker, you might need to pass |
The (podman in my setup) container is already running privileged, and I tried using the host cgroupns but that did not solve it. |
I've added I've checked for some submissions that should time out that they do, and for submissions that use too much memory that they are terminated, looks like it works 😄 I haven't tested the scenario where the compiler uses too much memory or times out, because I wouldn't know how to 😂 |
So we need to test if |
bce07d9
to
dddd091
Compare
Apparently podman doesn't pick up the correct value from the compose file. Forcing values seems to work on a system with cgroup v2 (did some basic judging): sudo docker compose --podman-args "--cgroup-manager cgroupfs" --podman-run-args "--cgroupns host" up
sudo docker compose --podman-args "--cgroup-manager cgroupfs" down Tested on Arch with kernel Linux 6.6.52-1-lts, systemd 256.6, podman 5.2.3. |
I'm probably doing something stupid, but on my Ubuntu 24.04, I get (with debug logging):
My
|
@nickygerritsen Because you get I checked on my system, and it looks like I don't even have
|
which seems good to me. I generated this this morning and don't have access to my machine right now, so can't check |
So after some debugging, it turns out I needed: echo '+cpuset' >> /sys/fs/cgroup/cgroup.subtree_control as root. After that it all works, as in I can judge succesfully. I have no clue why this is not enabled by default on my Ubuntu 24.04 machine.... Or how to detect this. |
It seems Ubuntu doesn't enable the ❯ cat /usr/lib/systemd/system/[email protected] | grep Delegate
Delegate=pids memory cpu I pushed a fix that will enable it using the existing @mpsijm can you verify this doesn't break your machine? @Kevinjil can you do the same? I tested on macOS with Docker and that still works. |
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 tested on my local Docker development using the hello
problem (which should contain a bunch of TLE/OOM-submissions) and some actual contest problems, and the results of the Judging Verifier are as I would expect 😄 Glad you found and fixed the issue!
Besides a reminder that g++
is still giving warnings, I have some final comments:
0ed11cc
to
065c999
Compare
Fixes DOMjudge#1072. Also move our integration tests from broken gitlab to working github actions.
…hout cgroupv1 fallback.
065c999
to
5f6ee8b
Compare
@meisterT You resolved my threads, but I don't see any changes in the related code 🤔 |
I already pinged him, he will fix it tonight. Some wrong force push it seems. |
5f6ee8b
to
59afcf0
Compare
Now pushed with more force :-), please take a look |
Looks good to me now. Anyone has any comments? Otherwise let's merge it. |
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.
Still works on my machine! 😄
Thanks everybody for making it possible to run DOMjudge on my machine again! ❤️
This is just to test whether it works on CI, it needs a lot of cleanup.