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

nixos/redis: Disable hugepages for redis via sysctl and not via a systemd-oneshot #71584

Merged
merged 1 commit into from
Oct 21, 2019
Merged

Conversation

maralorn
Copy link
Member

Motivation for this change

Fixes #71414

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @flokli

Sorry, something went wrong.

@flokli
Copy link
Contributor

flokli commented Oct 21, 2019

@GrahamcOfBorg build nixosTests.nextcloud.with-postgresql-and-redis

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 21, 2019
@flokli flokli merged commit f4725b3 into NixOS:master Oct 21, 2019
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Oct 21, 2019
};
boot.kernel.sysctl = {
"vm.nr_hugepages" = "0";
} // mkIf cfg.vmOverCommit { "vm.overcommit_memory" = "1"; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging modules with // does not work and will need to be fixed using mkMerge like in #72916.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't know that. What is the explanation?

Copy link
Member

@jtojnar jtojnar Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nix module system uses _type attributes to tag attribute sets:

nix-repl> lib.mkIf false { foo = 1; }
{ _type = "if"; condition = false; content = { ... }; }

So if you merge it into a different attribute:

nix-repl> { bar = 5; } // lib.mkIf false { foo = 1; }
{ _type = "if"; bar = 5; condition = false; content = { ... }; }

the bar will not be properly merged into content attribute, which the module attribute actually uses in the result.

We really need nixos-pills.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

followed up in #73049.

flokli added a commit to flokli/nixpkgs that referenced this pull request Nov 8, 2019
NixOS#71584 did merging without mkMerge.

cc @jtojnar
@flokli flokli mentioned this pull request Nov 8, 2019
10 tasks
flokli added a commit that referenced this pull request Nov 8, 2019
#71584 did merging without mkMerge.

cc @jtojnar

(cherry picked from commit 6303131)
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Jan 19, 2020

Verified

This commit was signed with the committer’s verified signature. The key has expired.
NixOS#71584 did merging without mkMerge.

cc @jtojnar

(cherry picked from commit 6303131)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redis module fails to disable Transparent Huge Pages (THP) in container
4 participants