search

nix-community / terraform-nixos

A set of Terraform modules that are designed to deploy NixOS [maintainer=@adrian-gierakowski]
Apache License 2.0
333 stars 61 forks source link

Use nix-store --export/import instead of nix-copy-closure for build-on-target case #63

Closed exarkun closed 4 months ago

exarkun commented 2 years ago

Fixes #62

I've tested this with a number of deployments for a host that runs GitLab and its dependencies. My latency to the target server is around 100ms and my upstream bandwidth to it is around 20Mbit/sec. I use build_on_target = true. On master@HEAD it takes about 11 minutes to copy all of the derivations for the system for a GitLab upgrade and 3-4 more minutes to finish the deployment. On this branch the 11 minute step becomes a 20-30 second step and the total deployment finishes in 3-4 minutes instead of 14-15 minutes.

However, sometimes the deployment fails during the --export/--import step with an ssh error that I haven't diagnosed yet. Even when I have to restart the deployment because of this the result is still ~10 minutes faster than on master@HEAD (though I would sure like to fix whatever is causing that failure).

hacklschorsch commented 2 years ago

LGTM / I like this 👍

exarkun commented 2 years ago

I wonder if there's something else I can do to help this get merged.

Profpatsch commented 2 years ago

Maybe we should migrate this repository to the https://github.com/nix-community organization if it’s unmaintained, then more contributors can be added.

dpc commented 1 year ago

See #64

smulikHakipod commented 1 year ago

Hey @adrian-gierakowski ! Now that you are the maintainer, can we merge it? Thanks!

adrian-gierakowski commented 1 year ago

@smulikHakipod would you be able to add a terraform var to allow user to choose which method of copying should be used, and set the default value to the original implementation. Thanks!

smulikHakipod commented 1 year ago

Yeah, sure, will do it on Tuesday

edolstra commented 1 year ago

Note that nix-store --export / --import is pretty deprecated. Its format is undocumented and it will not have a replacement in the new CLI. It's inefficient because it copies all store paths, not just the ones that are missing on the target.

nix-copy-closure is supposed to send all store paths in one stream, see https://github.com/NixOS/nix/commit/fe1f34fa60ad79e339c38e58af071a44774663f7. But maybe that's not working for some reason. (Maybe it only works for the ssh-ng store?)

adrian-gierakowski commented 1 year ago

Note that nix-store --export / --import is pretty deprecated. Its format is undocumented and it will not have a replacement in the new CLI. It's inefficient because it copies all store paths, not just the ones that are missing on the target.

nix-copy-closure is supposed to send all store paths in one stream, see NixOS/nix@fe1f34f. But maybe that's not working for some reason. (Maybe it only works for the ssh-ng store?)

Thanks for the info @edolstra!

@smulikHakipod we should probably point out the above in the terraform var description.

@exarkun given the above info, maybe there is a way you could fix your problem without introducing dependency on a deprecated feature?

exarkun commented 1 year ago

Note that nix-store --export / --import is pretty deprecated. Its format is undocumented and it will not have a replacement in the new CLI. It's inefficient because it copies all store paths, not just the ones that are missing on the target. nix-copy-closure is supposed to send all store paths in one stream, see NixOS/nix@fe1f34f. But maybe that's not working for some reason. (Maybe it only works for the ssh-ng store?)

Thanks for the info @edolstra!

@smulikHakipod we should probably point out the above in the terraform var description.

@exarkun given the above info, maybe there is a way you could fix your problem without introducing dependency on a deprecated feature?

It looks like the performance improving feature was added to a version of nix that came out after this issue was filed? Then maybe nix-copy-closure using a recent version of nix (newer than 2.16.1, maybe?) is all that is necessary to get this performance improvement - and no other changes to terraform-nixos are required.

If that interpretation sounds right, I might be able to do some performance testing with a new version of nix to see if this is the reality or not - though it might be a while before I get around to doing so (but the issue is already years old so that seems fine).

smulikHakipod commented 1 year ago

I will have a look in nix-copy-closure now

smulikHakipod commented 1 year ago

Using nix copy does work great and fast, yet it does not support NixOS 23.05 (as its uses nix 2.13 which is too old) (it works, its justs just slow on old nix versions), its still probably a better solution I guess. I made a PR.

exarkun commented 4 months ago

It looks like #76 should entirely supersede this.