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
preallocateContents option: disable by default #4094
Conversation
3c59d27
to
5221790
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/good-filesystem-for-the-nix-store/3566/13 |
As stated in the related issue, in the absence of feedback I did what I thought was best and removed draft:
I'll be glad to fix things if requested, let's get this issue finally closed! and I'll let you folks decide what to backport, the first PR might be useful on stable nix or we can just tell people to use nixUnstable if they want compression on btrfs |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/good-filesystem-for-the-nix-store/3566/26 |
It looks like it works :) I rebuilt my nixos system with nix from this commit, and fetched ghc from the binary cache on a zstd-compressed btrfs file system: compression works:
|
btw if this is not merged because too complex I'd suggest just disabling fallocate by default, here's a rant from #xfs@freenode earlier today (TL;DR: don't use
disabling fallocate by default is a one line change from current code and would probably make everyone happy... |
Yes I wouldn't mind disabling it by default. Contiguous allocation is also a lot less important now that everybody uses SSDs. |
using fallocate() to preallocate files space does more harm than good: - breaks compression on btrfs - has been called "not the right thing to do" by xfs developers (because delayed allocation that most filesystems implement leads to smarter allocation than what the filesystem needs to do if we upfront fallocate files)
Ok, updated this branch to just toggle the boolean value. Thanks! |
Thanks! |
Is this change usable in nixos-unstable or do we need to wait for a nix release? |
Last update to nixUnstable in nixpkgs master was around Nov 18, so it's not there yet -- I'm not sure how "good commits" are decided but you can probably just open a PR like NixOS/nixpkgs#104289 that just bumps the suffix/rev/hash to today's commits :) |
Done, thanks. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/good-filesystem-for-the-nix-store/3566/27 |
Sorry to jump into this so late, but I'm seeing the error
and that led me here. According to What confuses me is that I can't seem to disable it using I must be clearly missing something. If you have any pointers, I'd appreciate that :) |
2.3 is the stable branch, you need to be on 2.4 (e.g. having opted it to nixUnstable or for example nix flakes that pull it) to have this option. |
aah, that worked! thank you! |
On btrfs, fallocate preallocate file space and basically make it no-cow,
thus disabling compression which is probably desirable for nix store.
Change the preallocateContents to tristate true/false/auto and if
auto disable preallocation on btrfs.
Fixes #3550.
set as draft because I'm not happy with this:
SandboxMode
that @cole-h suggested.enum struct
(enum class
appears elsewhere in the code so I used that, it's apparently the same) but it wouldn't let me reuse true/false/auto keywords -- I'm very sad, what's the point of namespacing then :P erm, sorry, I mean I'll rename that when I move the option off.That aside I ended up going for a "check the first time it's used and update the option" -- it works well enough. The daemon forks when a request comes in so it basically checks with statfs everytime a new request comes in, but that's probably reasonable enough.