Skip to content
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

Merged
merged 5 commits into from
Oct 23, 2024
Merged

Implement cgroup v2 #2588

merged 5 commits into from
Oct 23, 2024

Conversation

meisterT
Copy link
Member

@meisterT meisterT commented Jun 3, 2024

This is just to test whether it works on CI, it needs a lot of cleanup.

@meisterT meisterT force-pushed the cgroupv2_exp branch 3 times, most recently from 3793aa6 to 3c426ec Compare June 7, 2024 20:02
@meisterT meisterT force-pushed the cgroupv2_exp branch 3 times, most recently from f65f320 to ce67e8b Compare August 27, 2024 19:35
@meisterT meisterT changed the title cgroupv2 experimental impl Implement cgroup v2 Aug 27, 2024
@meisterT meisterT marked this pull request as ready for review August 27, 2024 19:35
@meisterT
Copy link
Member Author

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).

Copy link
Member

@vmcj vmcj left a 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?

.github/workflows/integration.yml Outdated Show resolved Hide resolved
@meisterT
Copy link
Member Author

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?

Happy to revert what needs to be reverted but I fail to see what we are losing.

You checked if the cgroupv1 also still worked?

Locally: yes.

@vmcj
Copy link
Member

vmcj commented Aug 27, 2024

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?

Happy to revert what needs to be reverted but I fail to see what we are losing.

The GitLab version tested:

  • Setup with MySQL
  • Setup with MariaDB
  • Unpinned judgehost
  • Pinned judgehost to a specific CPU

I think not all were copied over and would require a matrix job.

@meisterT
Copy link
Member Author

Since judging was basically disabled on gitlab we didn't get a lot of use from the matrix

@vmcj
Copy link
Member

vmcj commented Aug 27, 2024

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.

@meisterT
Copy link
Member Author

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).

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.

@mpsijm
Copy link
Contributor

mpsijm commented Sep 25, 2024

I tried to run this on my Arch Linux laptop using docker compose up, but I got the same error as when running on the main branch:

domjudge-1  | Error: cgroup support missing memory features in running kernel. Unable to continue.
domjudge-1  | To fix this, please make the following changes:
domjudge-1  |     1. In /etc/default/grub, add 'cgroup_enable=memory swapaccount=1' to GRUB_CMDLINE_LINUX_DEFAULT.
domjudge-1  |        On modern distros (e.g. Debian bullseye and Ubuntu Jammy Jellyfish) which have cgroup v2 enabled by default,
domjudge-1  |        you need to add 'systemd.unified_cgroup_hierarchy=0' as well.
domjudge-1  |     2. Run update-grub
domjudge-1  |     3. Reboot

However, I do have cgroup_enable=memory swapaccount=1 currently in my /proc/cmdline.

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 judge/create_cgroups.in should be updated as well? The error message also appears to be outdated now regarding the "modern distros" part 😄

@nickygerritsen
Copy link
Member

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?

@mpsijm
Copy link
Contributor

mpsijm commented Sep 26, 2024

In my docker compose up log, I got:

domjudge-1  | 2024-09-26 08:09:29,624 INFO spawned: 'judgedaemon0' with pid 1956
domjudge-1  | 2024-09-26 08:09:29,628 INFO spawned: 'judgedaemon1' with pid 1957
domjudge-1  | 2024-09-26 08:09:30,217 WARN exited: judgedaemon0 (exit status 1; not expected)
domjudge-1  | 2024-09-26 08:09:30,217 WARN exited: judgedaemon1 (exit status 1; not expected)
    [... repeats three times, followed by ...]
domjudge-1  | 2024-09-26 08:09:36,815 INFO gave up: judgedaemon0 entered FATAL state, too many start retries too quickly
domjudge-1  | 2024-09-26 08:09:36,816 INFO gave up: judgedaemon1 entered FATAL state, too many start retries too quickly

However, when looking in ./output/log/judge.domjudge-contributor-0.log, it appeared that my judgehost password was incorrect, because I had a DB dump loaded from a previous contest. 😇 So I backed up my current DB volume did a docker compose pull and docker compose rm -v (removing volumes) to re-initialize the database. Now, the judgehosts do start (indeed, with commenting out judge/create_cgroups.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:
[Sep 26 08:54:31.892] judgedaemon[14487]: API request GET judgehosts/get_files/compile/9
[Sep 26 08:54:31.940] judgedaemon[14487]: Building executable in /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/executable/compile/9/62531e780378bb346939d18aacdfff1c, under 'build/'
[Sep 26 08:54:32.069] judgedaemon[14487]: Fetching executable failed for compile script '9': Failed to build executable in /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/executable/compile/9/62531e780378bb346939d18aacdfff1c.
[Sep 26 08:54:32.071] judgedaemon[14487]: API request POST judgehosts/internal-error
[Sep 26 08:54:32.167] judgedaemon[14487]: => internal error 1
[Sep 26 08:54:32.167] judgedaemon[14487]: API request POST judgehosts
[Sep 26 08:54:32.208] judgedaemon[14487]: API request POST judgehosts/fetch-work
[Sep 26 08:54:32.300] judgedaemon[14487]: ⇝ Received 19 'judging_run' judge tasks (endpoint default)
[Sep 26 08:54:32.301] judgedaemon[14487]:   Working directory: /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/2/2
[Sep 26 08:54:32.301] judgedaemon[14487]:   🔓 Executing chroot script: 'chroot-startstop.sh stop'
[Sep 26 08:54:32.328] judgedaemon[14487]:   🔒 Executing chroot script: 'chroot-startstop.sh start'
[Sep 26 08:54:32.381] judgedaemon[14487]: API request GET config
[Sep 26 08:54:32.415] judgedaemon[14487]: API request GET judgehosts/get_version_commands/20
[Sep 26 08:54:32.469] judgedaemon[14487]:   📋 Verifying versions.
[Sep 26 08:54:32.616] judgedaemon[14487]: API request PUT judgehosts/check_versions/20
[Sep 26 08:54:32.674] judgedaemon[14487]: API request GET judgehosts/get_files/source/2
[Sep 26 08:54:32.713] judgedaemon[14487]:   💾 Fetching new executable 'compile/20' with hash '4e301e4bc46ab73673e209ee3437707d'.
[Sep 26 08:54:32.713] judgedaemon[14487]: API request GET judgehosts/get_files/compile/20
[Sep 26 08:54:32.749] judgedaemon[14487]: Building executable in /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/executable/compile/20/4e301e4bc46ab73673e209ee3437707d, under 'build/'
[Sep 26 08:54:32.898] judgedaemon[14487]: Fetching executable failed for compile script '20': Failed to build executable in /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/executable/compile/20/4e301e4bc46ab73673e209ee3437707d.

--------------------------------------------------------------------------------

/home/maarten/git/contest/domjudge/bin/runguard: Failed to move the process to the cgroup
Try `/home/maarten/git/contest/domjudge/bin/runguard --help' for more information.
building failed with exitcode 255

@meisterT
Copy link
Member Author

@mpsijm thanks for testing, can you add cgroup_set_default_logger(CGROUP_LOG_DEBUG); to the main in runguard.cc and try again?

@mpsijm
Copy link
Contributor

mpsijm commented Sep 26, 2024

There you go, some debug output:
[Sep 26 11:07:23.651] judgedaemon[1956]: Installing signal handlers
[Sep 26 11:07:23.651] judgedaemon[1956]: 🔏 Executing chroot script: 'chroot-startstop.sh check'
[Sep 26 11:07:23.662] judgedaemon[1956]: Registering judgehost on endpoint default: http://localhost/api
[Sep 26 11:07:23.664] judgedaemon[1956]: API request POST judgehosts
[Sep 26 11:07:24.104] judgedaemon[1956]: API request GET config
[Sep 26 11:07:24.160] judgedaemon[1956]: API request GET languages
[Sep 26 11:07:24.249] judgedaemon[1956]: API request POST judgehosts/fetch-work
[Sep 26 11:07:24.333] judgedaemon[1956]: No submissions in queue (for endpoint default), waiting...
[Sep 26 11:07:56.626] judgedaemon[1956]: ⇝ Received 19 'judging_run' judge tasks (endpoint default)
[Sep 26 11:07:56.626] judgedaemon[1956]:   Working directory: /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/1/23
[Sep 26 11:07:56.627] judgedaemon[1956]:   🔒 Executing chroot script: 'chroot-startstop.sh start'
[Sep 26 11:07:56.681] judgedaemon[1956]: API request GET config
[Sep 26 11:07:56.717] judgedaemon[1956]: API request GET judgehosts/get_version_commands/1656
[Sep 26 11:07:56.774] judgedaemon[1956]:   📋 Verifying versions.
[Sep 26 11:07:56.897] judgedaemon[1956]: API request PUT judgehosts/check_versions/1656
[Sep 26 11:07:56.972] judgedaemon[1956]: API request GET judgehosts/get_files/source/1
[Sep 26 11:07:57.016] judgedaemon[1956]:   💾 Fetching new executable 'compile/9' with hash '62531e780378bb346939d18aacdfff1c'.
[Sep 26 11:07:57.017] judgedaemon[1956]: API request GET judgehosts/get_files/compile/9
[Sep 26 11:07:57.067] judgedaemon[1956]: Building executable in /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/executable/compile/9/62531e780378bb346939d18aacdfff1c, under 'build/'
[Sep 26 11:07:57.245] judgedaemon[1956]: Fetching executable failed for compile script '9': Failed to build executable in /home/maarten/git/contest/domjudge/output/judgings/domjudge-contributor-0/endpoint-default/executable/compile/9/62531e780378bb346939d18aacdfff1c.

--------------------------------------------------------------------------------

/home/maarten/git/contest/domjudge/bin/runguard: Failed to move the process to the cgroup
Try `/home/maarten/git/contest/domjudge/bin/runguard --help' for more information.
Found cgroup option cpuset, count 0
Found cgroup option cpu, count 1
Found cgroup option io, count 2
Found cgroup option memory, count 3
Found cgroup option hugetlb, count 4
Found cgroup option pids, count 5
Found cgroup option rdma, count 6
Found cgroup option misc, count 7
setting /sys/fs/cgroup/domjudge/dj_cgroup_2162__1727341677.150057/memory.max to "2147483648", pathlen 68
setting /sys/fs/cgroup/domjudge/dj_cgroup_2162__1727341677.150057/memory.swap.max to "0", pathlen 73
Will move pid 2163 to cgroup 'domjudge/dj_cgroup_2162__1727341677.150057/'
Adding controller memory
cgroup build procs path: /sys/fs/cgroup/domjudge/dj_cgroup_2162__1727341677.150057/cgroup.procs
Warning: cannot write tid 2163 to /sys/fs/cgroup/domjudge/dj_cgroup_2162__1727341677.150057/cgroup.procs:No such file or directory
Warning: cgroup_attach_task_pid failed: 50016
building failed with exitcode 255

@Kevinjil
Copy link
Contributor

Kevinjil commented Sep 26, 2024

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:

# grep -r $(echo $$) /sys/fs/cgroup --include 'cgroup.procs'
/sys/fs/cgroup/machine.slice/machine-libpod_pod_eb6f870aeb47c7f68c39940ee1841f7c1503844369812ca28001ba2180b90a5a.slice/libpod-4be6c5ad0c860471cc3a64fb8b37e30d67c9cc03b844508bceffeb01187ef6a5.scope/container/cgroup.procs:2474

Manually creating a domjudge group under the container slice and moving spawned processes in it is permitted from within the container, but it won't allow moving processes outside of the slice.

Processes in the container think that they are in the cgroup:

$ cat /proc/self/cgroup 
0::/

If we do not mount /sys/fs/cgroup in the container, the correct slice is mounted on /sys/fs/cgroup, but we are not permitted to create subgroups in it. We can see that a spawned process is in this virtual root group.

@meisterT
Copy link
Member Author

meisterT commented Sep 26, 2024

If you are running docker, you might need to pass --privileged --cgroupns=host --init (or a subset of it) as we do in the github integration test.

@Kevinjil
Copy link
Contributor

Kevinjil commented Sep 26, 2024

If you are running docker, you might need to pass --privileged --cgroupns=host --init (or a subset of it) as we do in the github integration test.

The (podman in my setup) container is already running privileged, and I tried using the host cgroupns but that did not solve it.

@mpsijm
Copy link
Contributor

mpsijm commented Sep 27, 2024

I've added cgroup: host and init: true to the domjudge service in docker-compose.yml, and that works! 🥳 It also works when only adding cgroup: host. It does not work when only adding init: true. The privileged: true option was already there.

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 😂

@nickygerritsen
Copy link
Member

So we need to test if cgroup: host also works on legacy v1 systems and on macOS and if so make that the default I'd say

@meisterT meisterT force-pushed the cgroupv2_exp branch 3 times, most recently from bce07d9 to dddd091 Compare October 5, 2024 13:05
judge/runguard.cc Show resolved Hide resolved
judge/runguard.cc Outdated Show resolved Hide resolved
@Kevinjil
Copy link
Contributor

Kevinjil commented Oct 9, 2024

The (podman in my setup) container is already running privileged, and I tried using the host cgroupns but that did not solve it.

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.

@nickygerritsen
Copy link
Member

nickygerritsen commented Oct 9, 2024

I'm probably doing something stupid, but on my Ubuntu 24.04, I get (with debug logging):

Compiling failed with exitcode 255, compiler output:
/home/nicky/Projects/domjudge/bin/runguard: creating cgroup: libcgroup: No such file or directory
Try `/home/nicky/Projects/domjudge/bin/runguard --help' for more information.
Found cgroup option cpuset, count 0
Found cgroup option cpu, count 1
Found cgroup option io, count 2
Found cgroup option memory, count 3
Found cgroup option hugetlb, count 4
Found cgroup option pids, count 5
Found cgroup option rdma, count 6
Found cgroup option misc, count 7
setting /sys/fs/cgroup/domjudge/dj_cgroup_8719_2_1728497611.913203/memory.max to "2147483648", pathlen 69
setting /sys/fs/cgroup/domjudge/dj_cgroup_8719_2_1728497611.913203/memory.swap.max to "0", pathlen 74

My /proc/mounts:

❯ cat /proc/mounts | grep cgroup
cgroup2 /sys/fs/cgroup cgroup2 rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot 0 0

@mpsijm
Copy link
Contributor

mpsijm commented Oct 10, 2024

@nickygerritsen Because you get libcgroup: No such file, can you check which packages you have installed on your Ubuntu, and which version(s) of *cgroup*.so are in your /usr/lib?

I checked on my system, and it looks like I don't even have libcgroup installed on my Arch 😂 But, I only run DOMjudge in a Docker container, so I probably don't need it on my host system. In the domjudge-contributor image, libcgroup-dev is installed. Inside the Docker container, I see this:

domjudge@domjudge-contributor:/usr$ find -name '*cgroup*.so'
./lib/x86_64-linux-gnu/libcgroup.so

@nickygerritsen
Copy link
Member

@nickygerritsen Because you get libcgroup: No such file, can you check which packages you have installed on your Ubuntu, and which version(s) of *cgroup*.so are in your /usr/lib?

I checked on my system, and it looks like I don't even have libcgroup installed on my Arch 😂 But, I only run DOMjudge in a Docker container, so I probably don't need it on my host system. In the domjudge-contributor image, libcgroup-dev is installed. Inside the Docker container, I see this:

domjudge@domjudge-contributor:/usr$ find -name '*cgroup*.so'
./lib/x86_64-linux-gnu/libcgroup.so
❯ apt-cache policy libcgroup2
libcgroup2:
  Installed: 2.0.2-2build1
  Candidate: 2.0.2-2build1
  Version table:
 *** 2.0.2-2build1 500
        500 http://nl.archive.ubuntu.com/ubuntu noble/universe amd64 Packages
        100 /var/lib/dpkg/status
❯ apt-cache policy libcgroup-dev
libcgroup-dev:
  Installed: 2.0.2-2build1
  Candidate: 2.0.2-2build1
  Version table:
 *** 2.0.2-2build1 500
        500 http://nl.archive.ubuntu.com/ubuntu noble/universe amd64 Packages
        100 /var/lib/dpkg/status

which seems good to me. I generated this this morning and don't have access to my machine right now, so can't check /usr/lib

@nickygerritsen
Copy link
Member

nickygerritsen commented Oct 10, 2024

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.

@nickygerritsen
Copy link
Member

nickygerritsen commented Oct 10, 2024

It seems Ubuntu doesn't enable the cpuset cgroup by default, as can be seen in:

❯ cat /usr/lib/systemd/system/[email protected] | grep Delegate
Delegate=pids memory cpu

I pushed a fix that will enable it using the existing create_cgroups script.

@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.

Copy link
Contributor

@mpsijm mpsijm left a 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:

judge/create_cgroups.in Outdated Show resolved Hide resolved
judge/create_cgroups.in Outdated Show resolved Hide resolved
judge/create_cgroups.in Show resolved Hide resolved
@mpsijm
Copy link
Contributor

mpsijm commented Oct 23, 2024

@meisterT You resolved my threads, but I don't see any changes in the related code 🤔

@nickygerritsen
Copy link
Member

I already pinged him, he will fix it tonight. Some wrong force push it seems.

@meisterT
Copy link
Member Author

Now pushed with more force :-), please take a look

@nickygerritsen
Copy link
Member

Looks good to me now. Anyone has any comments? Otherwise let's merge it.

Copy link
Contributor

@mpsijm mpsijm left a 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! ❤️

@nickygerritsen nickygerritsen added this pull request to the merge queue Oct 23, 2024
Merged via the queue into DOMjudge:main with commit 2d6f4ff Oct 23, 2024
28 checks passed
@mpsijm mpsijm mentioned this pull request Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants