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

keep-paths: keep store paths alive without touching the filesystem. #1938

Closed
wants to merge 1 commit into from

Conversation

shlevy
Copy link
Member

@shlevy shlevy commented Mar 2, 2018

Before this, scripts that do a nix build whose validity needn't extend
past the script's lifetime would have to either hope there was no
garbage collection between the build and the usage of it or create a
temporary directory and manage cleanup. With nix keep-paths, path
lifetimes can track script lifetimes (or shorter) without any races or
possibly interrupted cleanup steps.

Before this, scripts that do a nix build whose validity needn't extend
past the script's lifetime would have to either hope there was no
garbage collection between the build and the usage of it or create a
temporary directory and manage cleanup. With nix keep-paths, path
lifetimes can track script lifetimes (or shorter) without any races or
possibly interrupted cleanup steps.
@edolstra
Copy link
Member

edolstra commented Mar 2, 2018

This does not necessarily seem better than using --out-link to prevent the result from being garbage-collected. It seems pretty heavy to keep an entire process in memory (along with a nix-daemon worker process) just to keep track of a root.

@shlevy
Copy link
Member Author

shlevy commented Mar 2, 2018

With --out-link though you need to do cleanup. I've found result symlinks that weren't cleaned up when a script was interrupted to hard tons of times...

@edolstra
Copy link
Member

edolstra commented Mar 2, 2018

With nix keep-paths you have to clean up a process. Leaking a process is potentially much worse than leaking a symlink.

@shlevy
Copy link
Member Author

shlevy commented Mar 2, 2018

@edolstra You don't have to do anything to clean up the process. If your shell dies, it closes the fd and keep-paths gets a HUP.

@shlevy
Copy link
Member Author

shlevy commented Mar 2, 2018

(my test only closes the fd manually because I wanted to validate gc worked afterwards)

@7c6f434c
Copy link
Member

7c6f434c commented Mar 9, 2018

On the other hand, doesn't this solution require work on the daemon side while not fully eliminating the race condition? Maybe just nohup a process that (independently of Nix) waits for its stdin to get closed, then deletes a directory on tmpfs? In that case --link logic is supposed to guarantee that the symlink is created before GC has any chance to run.

@shlevy
Copy link
Member Author

shlevy commented Mar 9, 2018

@7c6f434c What race condition is there? Note that the daemon already supports process-lifetime temporary roots (this is how there's no chance of GC between evaluation and building in nix-build).

@shlevy
Copy link
Member Author

shlevy commented Mar 9, 2018

@7c6f434c If we're spawning a background process already, why not just have something that actually tracks roots with process lifetime, rather than hopefully having a chance to clean up a tempdir?

@7c6f434c
Copy link
Member

7c6f434c commented Mar 9, 2018

@shlevy You need to find outPath anyway, and outPath="$(nix-build --no-out-link something.nix)" is so convenient it will actually get used. Or nix build when it starts to print the outPath. You can evaluate outPath without instantiating, but that's enough of additional verbosity (and I think you cannot just build the resulting outPath without a re-evaluation) that nix keep-paths risks to be a feature that people use but usually incorrectly.

@shlevy
Copy link
Member Author

shlevy commented Mar 9, 2018

@7c6f434c Well if this gets merged the next step would be a Store implementation that punts indirect root creation off to keep-paths, then you can just do nix-build and it Just Works.

But, there's actually no risk even with your approach if you simply re-run nix-build after you register the root. Agreed it's verbose though.

@7c6f434c
Copy link
Member

7c6f434c commented Mar 9, 2018

I am not sure it will Just Work, as you have to specify the FD at some point as there is no safe FD number to assume (if we include scripts running under ulimit). But yes, it will be nice.

What is a rough outline of the needed flags/open FDs/communication with daemon?

Double-build is one of the multiple available ways that are verbose enough to incentivise Using It Wrong.

@shlevy
Copy link
Member Author

shlevy commented Mar 9, 2018

coproc nix keep-paths
nix-build --store keep-paths://auto?in=${COPROC[1]}&out=${COPROC[0]}

@shlevy
Copy link
Member Author

shlevy commented Mar 9, 2018

(obviously the keep-paths:// protocol doesn't exist yet)

@7c6f434c
Copy link
Member

7c6f434c commented Mar 9, 2018

And some quoting is also needed.

Frankly, that looks scary enough to wrap, and if I wrap it anyway, I can wrap using a solution that works now: nix-build with a link into a temprorary directory, open the output or change directory there, wait for parent to die.

@shlevy shlevy closed this Mar 9, 2018
@xaverdh
Copy link

xaverdh commented Oct 24, 2020

Still it would be nice to be able to keep store paths in a race free way alive without using the filesystem (e.g. for nixos-rebuild). Temporary directories are quite hard to use without introducing new races.

Maybe something like a "file descriptor api" for nix would be handy? Something like this:

Allow passing a file descriptor from the parent process, which is used as a communication channel. The nix process communicates when the build is done and outputs the build result over the fd, and then waits for the caller to communicate, that the gc lock on store path can be lifted, before exiting.

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

4 participants