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

@flokli
Copy link
Contributor

flokli commented Oct 21, 2019

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

};
boot.kernel.sysctl = {
"vm.nr_hugepages" = "0";
} // mkIf cfg.vmOverCommit { "vm.overcommit_memory" = "1"; };
Copy link
Contributor

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
Contributor

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