-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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
Swapspace: dynamically manage swapspace on NixOS #88093
Conversation
###### implementation | ||
|
||
config = mkIf cfg.enable { | ||
systemd.services.swapspace = with pkgs; { |
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.
systemd.services.swapspace = with pkgs; { | |
systemd.services.swapspace = { |
you don't refer to things inside pkgs
anywhere.
@wmertens can you address the changes requested? |
@flokli yes sorry, they were very helpful, but this dropped off my radar. I'll address them this weekend |
@wmertens another poke ;-) |
@peterhoeg thanks for the thorough review, much appreciated. I'll update this this week. |
is this still worked on? |
fe078cb
to
ef928e6
Compare
Ok, updated and rebased. Thank you all for your comments, is this good for merging? |
I marked this as stale due to inactivity. → More info |
based on reviews from langston-barrett, flokli and peterhoeg, thanks!
|
||
nativeBuildInputs = [ autoreconfHook ]; | ||
|
||
patchPhase = '' |
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.
patchPhase = '' | |
postPatch = '' |
nativeBuildInputs = [ autoreconfHook ]; | ||
|
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.
nativeBuildInputs = [ autoreconfHook ]; |
duped with line 14
type = types.nullOr types.int; | ||
default = null; | ||
description = '' | ||
Maximum size of a swapfile. |
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 think it would be great to explain what happens when this is set to null. Same for minSwapSize.
description = '' | ||
Cooldown period between changes. | ||
SwapSpace will wait at least this many seconds before an action, | ||
like removing a swapfile after adding one. |
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.
Is null then the same as 0 or how does it interact?
Why is this staled out ? Does no one use dynamic swap on Nix? |
I haven't gotten around to all the proposed fixes yet and it's working fine
on my system so the sense of urgency is low :)
Feel free to add commits!
Wout.
…On Wed., Oct. 5, 2022, 2:21 p.m. MPK99 ***@***.***> wrote:
Why is this staled out ? Does no one use dynamic swap ?
—
Reply to this email directly, view it on GitHub
<#88093 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAANNFQCKPQG2U7WPW5D64LWBVXDTANCNFSM4NEOJOYQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
+ optionalString (cfg.cooldown != null) | ||
" --cooldown=${toString cfg.cooldown}" | ||
+ optionalString (cfg.minSwapSize != null) | ||
" --min_swapsize=${toString cfg.minSwapSize}" |
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.
From the documentation:
Never bother to allocate any swapfiles smaller than size bytes. There should be no need to change this variable except for testing.
I suggest removing this argument and the option. For testing, one can just use extraArgs
.
You should reuse the upstream systemd unit file by copying it to the package output and then using it in the NixOS module. Let me know if you need help with that. I'll be happy to help you on this. |
having dynamic swap on my laptop (and I bet many other systems) would be so nice... Zswap seems to work better than zram and being able to have a dynamic swap system would help with not having to sacrifice a definite storage space |
Superseded by #348588 |
Motivation for this change
Using NixOS with dynamic swap size management, the way macOS does it.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)