nixos/nix-daemon: Enable cgroups delegation#339310
Conversation
|
Hi, thanks for working on this! I checked systemd documentation but can't really find much information on how |
|
@yshui I'm afraid not - I only found out about that behavior through trial and error. I don't know if there's any documentation on it. |
|
From my understanding, this change shouldn't affect back-compat and could be merged already. Any objections? |
| IOSchedulingClass = cfg.daemonIOSchedClass; | ||
| IOSchedulingPriority = cfg.daemonIOSchedPriority; | ||
| LimitNOFILE = 1048576; | ||
| Delegate = "yes"; |
There was a problem hiding this comment.
Should it be conditional on cfg.settings.use-cgroups? or does this just not afect anything if use-cgroups = false
There was a problem hiding this comment.
Making it conditional on use-cgroups would be acceptable, although as far as I can tell from the documentation, Delegate is specific to cgroups and shouldn't affect anything else.
There was a problem hiding this comment.
That feature can also be automatically enabled, making the detection logic a bit tricky at Nix eval time:
https://github.com/NixOS/nix/blob/46339db18db562607977523aa8cb552a1e9b08d3/src/libstore/globals.hh#L497-L498
|
Squash merged to add the message in the description to the commit |
|
Though better than the status quo (previously cgroups feature didn’t really work at all) this is not great. it would be better if nix-daemon would create cgroups (scopes) using the systemd dbus api in a dedicated slice. Instead of managing cgroups itself. (Easiest way would be if we wrap each build in a systemd-run command) problem now is that we can only configure cgroups settings like limits and systemd-oomd on cgroups managed by systemd. In practise this means we can only configure settings on nix-daemon.service this means that it’s possible that nix-daemon itself gets OOM killed if one of it builds are the one causing memory pressure. Which is obviously not desired. |
|
Reading the systemd source code and oomd man page I need to slightly correct myself:
For Delegate=yes units OOMPolicy defaults to continue instead of kill. So the end result is oomd and the kernel OOM killer will work as follows:
So this configuration is valid and works as expected. |
|
thanks for checking @arianvp. For reference, since there are a lot of similarities, here is what the Docker service looks like: https://gist.github.com/zimbatm/acec1b6f4e970d52b42dc0b761f9d114 . More specifically, it sets |
|
I think we don't want If nix-daemon dies we probably want the builds to die too? Unless it's smart enough to carry on after a restart like docker is . The OOM adjust makes sense. We might also want to set MemoryMin= on nix-daemon to give it some dedicated memory to work with under memory pressure. That makes me think though. Does eval run in nix-daemon or does it run in a separate process that the nix daemon spawns under a new cgroup? As eval is quite extremely heavy on memory it might be interesting to have it in a cgroup so we can make sure an eval eating all memory gets killed promptly without killing the entire daemon and any other builds in progress. Or does nix-daemon not do eval at all? It happens in the nix-build command itself right? In that case we're all good. |
|
Sounds good. I'll send a follow-up OOMAjust score. For MemoryMin, I need to do more instrumentation to find the correct value. |
Motivation for change
When
use-cgroupsis enabled, the nix daemon creates sub-cgroups for the build processes (and itself if NixOS/nix#11412 is merged, see NixOS/nix#9675).Delegateshould be set to prevent systemd from messing with the nix service's cgroups (https://github.com/systemd/systemd/blob/main/docs/CGROUP_DELEGATION.md) and ensure the OOM killer only targets the offending derivation and not the entire service (NixOS/nix#10374).Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.