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

push: try to use the targets binary caches #73

Closed
wants to merge 1 commit into from

Conversation

andir
Copy link
Contributor

@andir andir commented Sep 11, 2019

By allowing the remote to substitute using the configured binary caches
(e.g. https://cache.nixos.org) you have to upload a lot less data to the
remote host. This improves the user experience on some residential
connections that sadly still have miserable upload performance. Even
worse is the situation on metered connections where the user might have
to pay for each megabyte of data.

Nix copy will ask the remote store to try to fulfill a list of
transmitted paths and only those that can not be provided by any of the
registered binary caches will be uploaded from the local machine.

There is no real downside to this. It might take a few seconds to verify
if a specific path can be downloaded from a binary cache. That time is
probably not significant when compared to uploading an entire system
closure from a slow (or metered) connection.


This change is Reviewable

By allowing the remote to substitute using the configured binary caches
(e.g. https://cache.nixos.org) you have to upload a lot less data to the
remote host. This improves the user experience on some residential
connections that sadly still have miserable upload performance. Even
worse is the situation on metered connections where the user might have
to pay for each megabyte of data.

Nix copy will ask the remote store to try to fulfill a list of
transmitted paths and only those that can not be provided by any of the
registered binary caches will be uploaded from the local machine.

There is no real downside to this. It might take a few seconds to verify
if a specific path can be downloaded from a binary cache. That time is
probably not significant when compared to uploading an entire system
closure from a slow (or metered) connection.
Copy link
Contributor

@johanot johanot left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. I totally agree that remote substitution is sane to do. Hardcoding it would however break some things for us, since we actually have servers that are unable to access the internet. This results in:

Pushing paths to container-s01:
        * /nix/store/rpjhddsa4cp5yl2k2j6rys51avqk16x1-nixos-system-container-s01-19.03pre-git
        * /nix/store/l0i9dkfa58yv6xgzy0fm8sjn6zfwnijb-nixos-system-container-s01-19.03pre-git.drv
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 294 ms
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 639 ms
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 1240 ms
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 2398 ms

.. others might have similar setups, or perhaps we're just the weird ones. :) In any case, I think I'll recommend some flag or option to control this behavior. I have no strong feeling for whether it should be default on or off, though.

It could be either just a cmdline flag for morph, or perhaps even better: a NixOS-option, that'll allow for substitution to be enabled/disabled by config at individual host-level.

@@ -241,6 +241,7 @@ func Push(ctx *ssh.SSHContext, host Host, paths ...string) (err error) {
for _, path := range paths {
cmd := exec.Command(
"nix", "copy",
"--substitute-on-destination",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the short flag instead? -s .. Because.. --substitute-on-destination is neither compatible with Nix v2.2 or 2.3. (ref: NixOS/nix@aa739e7)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I usually dislike using short opts since it makes it harder to understand the code but in this case it is probably warranted due to the breaking change in nix.

@andir
Copy link
Contributor Author

andir commented Sep 11, 2019

Thank you for the PR. I totally agree that remote substitution is sane to do. Hardcoding it would however break some things for us, since we actually have servers that are unable to access the internet. This results in:

Pushing paths to container-s01:
        * /nix/store/rpjhddsa4cp5yl2k2j6rys51avqk16x1-nixos-system-container-s01-19.03pre-git
        * /nix/store/l0i9dkfa58yv6xgzy0fm8sjn6zfwnijb-nixos-system-container-s01-19.03pre-git.drv
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 294 ms
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 639 ms
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 1240 ms
warning: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't connect to server (7); retrying in 2398 ms

.. others might have similar setups, or perhaps we're just the weird ones. :) In any case, I think I'll recommend some flag or option to control this behavior. I have no strong feeling for whether it should be default on or off, though.

It could be either just a cmdline flag for morph, or perhaps even better: a NixOS-option, that'll allow for substitution to be enabled/disabled by config at individual host-level.

I can understand the use case and the need for the option. Wouldn't you be better off by setting an empty list of substituters on all those machines? That sounds like what you actually want.

@johanot
Copy link
Contributor

johanot commented Sep 11, 2019

Wouldn't you be better setting an empty list of substituters on all those machines?

I guess you might be right... and.. As long as we introduce this in a new release, we can always "rel-note" us out of any (potentially) breaking changes. I'll do some more testing with this and get back to you.

@bb010g
Copy link
Contributor

bb010g commented Oct 8, 2019

I think merging this now with an opt-in morph {push,deploy} --substitute-on-destination flag would be the best option. Making it default can be pursued later, and a complementary --no-substitute-on-destination flag could be placed currently in scripts where necessary to avoid breakage if/when the default changes.

@johanot
Copy link
Contributor

johanot commented Oct 8, 2019

I tend to agree with @bb010g after all. @andir can you perhaps change the PR to include a --substitute-on-destination flag for morph?

@mweinelt
Copy link
Contributor

At the moment it is quite tedious to deploy from within networks with only little upstream bandwidth, so I'd be very happy about this change.

@adamtulinius
Copy link
Contributor

Please create a new issue if #83 didn't fix this in an acceptable manner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants