Overhaul wopAddToStore by roberth · Pull Request #4030 · NixOS/nix

roberth · 2020-09-17T18:59:33Z

Implement RemoteStore::addToStoreFromDump. Closes Implement RemoteStore.addToStoreFromDump #4009
Constant memory addToStore in daemon. Closes WIP: Constant space daemon wopAddToStore -- contains #3801 #3802
Fixes Large files within source tree causes build to run out of memory #358 (confirmed now), possibly Unable to add a fixed-output large file to the store on Darwin #1117
Introduce ContentAddressMethod which represents the labels that precede CA hashes but not the hash
Fold wopAddTextToStore into wopAddToStore. Not super necessary but was easy with the above in place and fits well with the new ContentAddress* stuff
FramedSource and FramedSink are now reusable
Affects "Dumping very large path" message isn't helpful #1184

src/libstore/remote-store.hh

Ericson2314 · 2020-09-17T19:15:43Z

@roberth This is really excellent! You've managed to anticipate a bunch of stuff I hope to do, like combining the addToStore methods and ContentAddressMethod (which I used in #3959). Thank you!

src/libstore/daemon.cc

src/libstore/content-address.cc

src/libstore/remote-store.cc

roberth

@edolstra This is ready for review

roberth · 2020-09-18T09:37:54Z

src/libstore/daemon.cc

    }
 };

+static void writeValidPathInfo(ref<Store> store, unsigned int clientVersion, Sink & to, std::shared_ptr<const ValidPathInfo> info) {


Moved, unchanged.

roberth · 2020-09-18T09:39:52Z

src/libstore/daemon.cc

+            to << store->printStorePath(pathInfo->path);
+            writeValidPathInfo(store, clientVersion, to, pathInfo);
+
+        } else {


else branch is the unchanged previous protocol implementation.

roberth · 2020-09-18T09:41:22Z

src/libstore/remote-store.cc

 }


+ref<const ValidPathInfo> RemoteStore::readValidPathInfo(ConnectionHandle & conn, const StorePath & path) {


Moved, unchanged except improved to use ref over shared_ptr

roberth · 2020-09-18T09:45:11Z

src/libstore/remote-store.cc

 }

+void
+ConnectionHandle::withFramedSink(std::function<void(Sink &sink)> fun) {


Moved, unchanged.

roberth · 2020-09-18T09:45:58Z

src/libstore/remote-store.hh

-    StorePath addToStore(const string & name, const Path & srcPath,
-        FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256,
-        PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override;


Default implementation is used.

roberth · 2020-09-18T09:47:41Z

src/libutil/serialise.hh

+   Use with FramedSink, which also allows the logical stream to be terminated
+   in the event of an exception.
+*/
+struct FramedSource : Source


Updated comment, implemention moved and unchanged.

roberth · 2020-09-18T09:48:57Z

src/libutil/serialise.hh

+   The exception_ptr reference can be used to terminate the stream when you
+   detect that an error has occurred on the remote end.
+*/
+struct FramedSink : nix::BufferedSink


New comment, interface changed to take a BufferedSink instead of the whole Connection. Other than that, implementation unchanged.

src/libstore/daemon.cc

src/libstore/remote-store.cc

Ericson2314

Baring those 2 little things, I cannot recommend this enough!

CC @edolstra

Co-authored-by: John Ericson <[email protected]>

A ValidPathInfo is created anyway. By returning it we can save a roundtrip and we have a nicer interface.

Also checked that all usages satisfy the requirement and removed dead code.

edolstra · 2020-09-22T09:42:18Z

Thanks!

edolstra · 2020-09-22T13:02:39Z

After merging this, commands like nix build nixpkgs#hello hang in nix::RemoteStore::addCAToStore() when using an older nix-daemon:

$ gdb --args nix build nixpkgs#hello  -vvvvv
GNU gdb (GDB) 8.3
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from nix...
(gdb) r
Starting program: /home/eelco/Dev/nix/outputs/out/bin/nix build nixpkgs\#hello -vvvvv
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libthread_db.so.1".
[New Thread 0x7ffff615a700 (LWP 11590)]
[New Thread 0x7ffff57d9700 (LWP 11591)]
[New Thread 0x7ffff4fd8700 (LWP 11592)]
[New Thread 0x7ffff47d7700 (LWP 11593)]
[New Thread 0x7ffff3fd6700 (LWP 11594)]
[New Thread 0x7ffff37d5700 (LWP 11595)]
[New Thread 0x7ffff2fd4700 (LWP 11596)]
[New Thread 0x7ffff27d3700 (LWP 11597)]
[New Thread 0x7ffff1fd2700 (LWP 11598)]
[New Thread 0x7ffff17d1700 (LWP 11599)]
[New Thread 0x7ffff0fd0700 (LWP 11600)]
[New Thread 0x7ffff07cf700 (LWP 11601)]
[New Thread 0x7fffeffce700 (LWP 11602)]
[New Thread 0x7fffef7cd700 (LWP 11603)]
[New Thread 0x7fffeefcc700 (LWP 11604)]
[New Thread 0x7fffee7cb700 (LWP 11605)]
[New Thread 0x7fffd5fba700 (LWP 11606)]
resolved search path element '/home/eelco/Dev/nix/outputs/out/share/nix/corepkgs' to '/home/eelco/Dev/nix/outputs/out/share/nix/corepkgs'
checking access to '/home/eelco/Dev/nix/outputs/out/share/nix/corepkgs/module.nix'
evaluating file '/home/eelco/Dev/nix/outputs/out/share/nix/corepkgs/module.nix'
acquiring global GC lock '/nix/var/nix/gc.lock'
acquiring read lock on '/nix/var/nix/temproots/11607'
acquiring write lock on '/nix/var/nix/temproots/11607'
downgrading to read lock on '/nix/var/nix/temproots/11607'
using cache entry '{"name":"flake-registry.json","type":"file","url":"https://github.com/NixOS/flake-registry/raw/master/flake-registry.json"}' -> '{"etag":"\"30cebe0247c3594057b8c05ed7ce4f6aefd2eb2ab1bc27ae6625e44b0e81a559\"","url":"https://raw.githubusercontent.com/NixOS/flake-registry/master/flake-registry.json"}', '/nix/store/050wdgdbrqya3p8h348iz8bb7lif7pww-flake-registry.json'
acquiring global GC lock '/nix/var/nix/gc.lock'
looked up 'flake:nixpkgs' -> 'github:NixOS/nixpkgs'
acquiring write lock on '/nix/var/nix/temproots/11607'
downgrading to read lock on '/nix/var/nix/temproots/11607'
using cache entry '{"name":"source","type":"file","url":"https://api.github.com/repos/NixOS/nixpkgs/commits/HEAD?access_token=7f761097b61f99a4b0dc29d5013e0281a515dd51"}' -> '{"etag":"\"1c85670289a5edf05036806ac6977f0473c82ae1e6df38114ab55ac83246b1c4\"","url":"https://api.github.com/repos/NixOS/nixpkgs/commits/HEAD?access_token=7f761097b61f99a4b0dc29d5013e0281a515dd51"}', '/nix/store/pq2dr427xh1s96aijgy95bwzrz7qkixp-source'
HEAD revision for 'https://api.github.com/repos/NixOS/nixpkgs/commits/HEAD?access_token=7f761097b61f99a4b0dc29d5013e0281a515dd51' is 06b27898a4563ef2e0e653fd78f76b77c31820ff
did not find cache entry for '{"rev":"06b27898a4563ef2e0e653fd78f76b77c31820ff","type":"git-tarball"}'
did not find cache entry for '{"name":"source","type":"tarball","url":"https://api.github.com/repos/NixOS/nixpkgs/tarball/06b27898a4563ef2e0e653fd78f76b77c31820ff?access_token=7f761097b61f99a4b0dc29d5013e0281a515dd51"}'
acquiring write lock on '/nix/var/nix/temproots/11607'
downgrading to read lock on '/nix/var/nix/temproots/11607'
using cache entry '{"name":"source","type":"file","url":"https://api.github.com/repos/NixOS/nixpkgs/tarball/06b27898a4563ef2e0e653fd78f76b77c31820ff?access_token=7f761097b61f99a4b0dc29d5013e0281a515dd51"}' -> '{"etag":"\"75bee42da539dd691221f59fe59e108703e357f6a358b6e404eddca4e671f625\"","url":"https://codeload.github.com/NixOS/nixpkgs/legacy.tar.gz/06b27898a4563ef2e0e653fd78f76b77c31820ff"}', '/nix/store/jbk31x413yr8a4ispmgjpjkfaxkgpvw4-source'
acquiring write lock on '/nix/var/nix/temproots/11607'
downgrading to read lock on '/nix/var/nix/temproots/11607'
^C
Thread 1 "nix" received signal SIGUSR1, User defined signal 1.
0x00007ffff726570d in pthread_cond_wait@@GLIBC_2.3.2 () from /nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libpthread.so.0
(gdb) bt
#0  0x00007ffff726570d in pthread_cond_wait@@GLIBC_2.3.2 () from /nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libpthread.so.0
#1  0x00007ffff74a386c in std::condition_variable::wait(std::unique_lock<std::mutex>&) () from /nix/store/danv012gh0aakh8xnk2b35vahklz72mk-gcc-9.2.0-lib/lib/libstdc++.so.6
#2  0x00007ffff78f6f3f in nix::Sync<nix::Pool<nix::RemoteStore::Connection>::State, std::mutex>::Lock::wait (cv=..., this=0x7fffffff4260) at src/libutil/sync.hh:53
#3  nix::Pool<nix::RemoteStore::Connection>::get (this=0x6de070) at src/libutil/pool.hh:138
#4  0x00007ffff78eb85c in nix::RemoteStore::getConnection (this=this@entry=0x6de538) at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/shared_ptr_base.h:1020
#5  0x00007ffff78ee146 in nix::RemoteStore::queryPathInfoUncached (this=0x6de538, path=..., callback=...) at src/libstore/remote-store.cc:448
#6  0x00007ffff791aad1 in nix::Store::queryPathInfo (this=<optimized out>, storePath=..., callback=...)
    at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/atomic_base.h:229
#7  0x00007ffff791b06e in nix::Store::queryPathInfo (this=0x6de580, storePath=...) at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/atomic_base.h:229
#8  0x00007ffff78ed007 in nix::RemoteStore::addCAToStore (this=0x6de538, dump=warning: RTTI symbol not found for class 'nix::sinkToSource(std::function<void (nix::Sink&)>, std::function<void ()>)::SinkToSource'
..., name=..., 
    caMethod=std::variant<struct nix::TextHashMethod, struct nix::FixedOutputHashMethod> [index 1] = {...}, references=..., repair=nix::NoRepair) at src/libstore/remote-store.cc:608
#9  0x00007ffff78ed42b in virtual thunk to nix::RemoteStore::addToStoreFromDump(nix::Source&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, nix::FileIngestionMethod, nix::HashType, nix::RepairFlag) () at src/libutil/serialise.hh:170
#10 0x00007ffff79138ef in nix::Store::addToStore(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, nix::FileIngestionMethod, nix::HashType, std::function<bool (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&, nix::RepairFlag) (this=0x6de580, name="source", _srcPath=..., method=<optimized out>, hashAlgo=<optimized out>, filter=..., repair=nix::NoRepair)
    at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/unique_ptr.h:352
#11 0x00007ffff7a5ba2b in nix::fetchers::downloadTarball (store=..., 
    url="https://api.github.com/repos/NixOS/nixpkgs/tarball/06b27898a4563ef2e0e653fd78f76b77c31820ff?access_token=7f761097b61f99a4b0dc29d5013e0281a515dd51", name="source", 
    immutable=immutable@entry=true) at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/shared_ptr_base.h:1020
#12 0x00007ffff7a3c173 in nix::fetchers::GitArchiveInputScheme::fetch (this=<optimized out>, store=..., _input=...)
    at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/ext/atomicity.h:96
#13 0x00007ffff7a13ebc in nix::fetchers::Input::fetch (this=this@entry=0x7fffffff5710, store=...)
    at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/ext/atomicity.h:96
#14 0x00007ffff7e24a67 in nix::FlakeRef::fetchTree (this=this@entry=0x7fffffff5710, store=...) at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/ext/atomicity.h:96
#15 0x00007ffff7e15598 in nix::flake::fetchOrSubstituteTree (state=..., originalRef=..., allowLookup=<optimized out>, flakeCache=std::vector of length 0, capacity 0)
    at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/ext/atomicity.h:96
#16 0x00007ffff7e16aca in nix::flake::getFlake (state=..., originalRef=..., allowLookup=<optimized out>, flakeCache=...) at src/libexpr/flake/flake.cc:175
#17 0x00007ffff7e1a0ec in nix::flake::lockFlake (state=..., topRef=..., lockFlags=...) at src/libexpr/flake/flake.cc:261
#18 0x000000000056258c in nix::InstallableFlake::getLockedFlake (this=0x6ff1b0) at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/shared_ptr_base.h:1020
#19 0x0000000000562807 in nix::InstallableFlake::toDerivation[abi:cxx11]() (this=0x6ff1b0) at src/nix/installables.cc:462
#20 0x0000000000562e0d in nix::InstallableFlake::toDerivations (this=<optimized out>) at src/nix/installables.cc:507
#21 0x000000000055e339 in nix::InstallableValue::toBuildables (this=0x6ff1b0) at src/nix/installables.cc:338
#22 0x00000000005585fe in nix::build (store=..., mode=nix::Realise::Outputs, installables=std::vector of length 1, capacity 1 = {...}, bMode=bMode@entry=nix::bmNormal)
    at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/shared_ptr_base.h:1020
#23 0x00000000004f77a6 in CmdBuild::run (this=0x6db110, store=...) at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/ext/atomicity.h:96
#24 0x00000000005075c8 in nix::StoreCommand::run (this=0x6db380) at src/nix/command.cc:44
#25 0x00000000005701e8 in nix::mainWrapped (argc=<optimized out>, argv=<optimized out>) at src/nix/main.cc:253
#26 0x00007ffff7ab68fd in std::function<void ()>::operator()() const (this=0x7fffffff7980)
    at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/bits/std_function.h:685
#27 nix::handleExceptions(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void ()>) (
    programName="/home/eelco/Dev/nix/outputs/out/bin/nix", fun=...) at src/libmain/shared.cc:312
#28 0x000000000047207b in main (argc=<optimized out>, argv=<optimized out>) at /nix/store/b3zsk4ihlpiimv3vff86bb5bxghgdzb9-gcc-9.2.0/include/c++/9.2.0/ext/new_allocator.h:80

I think addCAToStore() should not call queryPathInfo() since that triggers a deadlock.

edolstra · 2020-09-22T13:30:38Z

Fixed in 980edd1.

roberth added 5 commits September 17, 2020 20:21

Typo

3f93bc0

Remove redundant import

9ee3122

Add Source.drainInto(Sink)

29c82cc

Add ContentAddressMethod and parse/render it

dfa547c

Move FramedSource and FramedSink, extract withFramedSink

14b30b3

roberth mentioned this pull request Sep 17, 2020

Implement RemoteStore.addToStoreFromDump #4009

Closed