From c5b83d8913d73ea58ff9437c41bf6bd0c6839ad0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 16 Mar 2017 13:50:01 +0100 Subject: [PATCH] copyPaths(): Use queryValidPaths() to reduce SSH latency --- src/build-remote/build-remote.cc | 16 +++++++----- src/libstore/legacy-ssh-store.cc | 13 ++++++++++ src/libstore/local-store.cc | 2 +- src/libstore/local-store.hh | 2 +- src/libstore/remote-store.cc | 2 +- src/libstore/remote-store.hh | 2 +- src/libstore/store-api.cc | 33 ++++++++---------------- src/libstore/store-api.hh | 8 +++--- src/nix-copy-closure/nix-copy-closure.cc | 2 +- src/nix/copy.cc | 2 +- 10 files changed, 45 insertions(+), 37 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 3908dfac4..6b142db98 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -252,10 +252,10 @@ connected: string line; if (!getline(cin, line)) throw Error("hook caller didn't send inputs"); - auto inputs = tokenizeString>(line); + auto inputs = tokenizeString(line); if (!getline(cin, line)) throw Error("hook caller didn't send outputs"); - auto outputs = tokenizeString(line); + auto outputs = tokenizeString(line); AutoCloseFD uploadLock = openLockFile(currentLoad + "/" + hostName + ".upload-lock", true); auto old = signal(SIGALRM, handleAlarm); alarm(15 * 60); @@ -269,11 +269,15 @@ connected: printError("building ‘%s’ on ‘%s’", drvPath, hostName); sshStore->buildDerivation(drvPath, readDerivation(drvPath)); - std::remove_if(outputs.begin(), outputs.end(), [=](const Path & path) { return store->isValidPath(path); }); - if (!outputs.empty()) { - setenv("NIX_HELD_LOCKS", concatStringsSep(" ", outputs).c_str(), 1); /* FIXME: ugly */ - copyPaths(ref(sshStore), store, outputs); + PathSet missing; + for (auto & path : outputs) + if (!store->isValidPath(path)) missing.insert(path); + + if (!missing.empty()) { + setenv("NIX_HELD_LOCKS", concatStringsSep(" ", missing).c_str(), 1); /* FIXME: ugly */ + copyPaths(ref(sshStore), store, missing); } + return; }); } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 1a933259b..84bf0f727 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -226,6 +226,19 @@ struct LegacySSHStore : public Store out.insert(res.begin(), res.end()); } + PathSet queryValidPaths(const PathSet & paths, bool maybeSubstitute = false) override + { + auto conn(connections->get()); + + conn->to + << cmdQueryValidPaths + << false // lock + << maybeSubstitute + << paths; + conn->to.flush(); + + return readStorePaths(*this, conn->from); + } }; static RegisterStoreImplementation regStore([]( diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 63f069c2f..dcfa000c4 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -669,7 +669,7 @@ bool LocalStore::isValidPathUncached(const Path & path) } -PathSet LocalStore::queryValidPaths(const PathSet & paths) +PathSet LocalStore::queryValidPaths(const PathSet & paths, bool maybeSubstitute) { PathSet res; for (auto & i : paths) diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 49a0d7e0d..28e9a31c9 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -99,7 +99,7 @@ public: bool isValidPathUncached(const Path & path) override; - PathSet queryValidPaths(const PathSet & paths) override; + PathSet queryValidPaths(const PathSet & paths, bool maybeSubstitute = false) override; PathSet queryAllValidPaths() override; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 1ac2d7b6e..a1f2db5b0 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -187,7 +187,7 @@ bool RemoteStore::isValidPathUncached(const Path & path) } -PathSet RemoteStore::queryValidPaths(const PathSet & paths) +PathSet RemoteStore::queryValidPaths(const PathSet & paths, bool maybeSubstitute) { auto conn(connections->get()); if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) { diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 66540a2a2..a08bd3056 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -28,7 +28,7 @@ public: bool isValidPathUncached(const Path & path) override; - PathSet queryValidPaths(const PathSet & paths) override; + PathSet queryValidPaths(const PathSet & paths, bool maybeSubstitute = false) override; PathSet queryAllValidPaths() override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 9ab3f3632..b1bf961e1 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -377,7 +377,7 @@ void Store::queryPathInfo(const Path & storePath, } -PathSet Store::queryValidPaths(const PathSet & paths) +PathSet Store::queryValidPaths(const PathSet & paths, bool maybeSubstitute) { struct State { @@ -550,6 +550,8 @@ void copyClosure(ref srcStore, ref dstStore, for (auto & path : storePaths) srcStore->computeFSClosure(path, closure); + // FIXME: use copyStorePaths() + PathSet valid = dstStore->queryValidPaths(closure); if (valid.size() == closure.size()) return; @@ -791,35 +793,22 @@ std::list> getDefaultSubstituters() } -void copyPaths(ref from, ref to, const Paths & storePaths, bool substitute) +void copyPaths(ref from, ref to, const PathSet & storePaths, bool substitute) { - if (substitute) { - /* Filter out .drv files (we don't want to build anything). */ - PathSet paths2; - for (auto & path : storePaths) - if (!isDerivation(path)) paths2.insert(path); - unsigned long long downloadSize, narSize; - PathSet willBuild, willSubstitute, unknown; - to->queryMissing(PathSet(paths2.begin(), paths2.end()), - willBuild, willSubstitute, unknown, downloadSize, narSize); - /* FIXME: should use ensurePath(), but it only - does one path at a time. */ - if (!willSubstitute.empty()) - try { - to->buildPaths(willSubstitute); - } catch (Error & e) { - printMsg(lvlError, format("warning: %1%") % e.msg()); - } - } + PathSet valid = to->queryValidPaths(storePaths, substitute); + + PathSet missing; + for (auto & path : storePaths) + if (!valid.count(path)) missing.insert(path); std::string copiedLabel = "copied"; - logger->setExpected(copiedLabel, storePaths.size()); + logger->setExpected(copiedLabel, missing.size()); ThreadPool pool; processGraph(pool, - PathSet(storePaths.begin(), storePaths.end()), + PathSet(missing.begin(), missing.end()), [&](const Path & storePath) { if (to->isValidPath(storePath)) return PathSet(); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 92aa8862f..98f2803f8 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -324,8 +324,10 @@ protected: public: - /* Query which of the given paths is valid. */ - virtual PathSet queryValidPaths(const PathSet & paths); + /* Query which of the given paths is valid. Optionally, try to + substitute missing paths. */ + virtual PathSet queryValidPaths(const PathSet & paths, + bool maybeSubstitute = false); /* Query the set of all valid paths. Note that for some store backends, the name part of store paths may be omitted @@ -653,7 +655,7 @@ ref openStore(const std::string & uri = getEnv("NIX_REMOTE")); ref openStore(const std::string & uri, const Store::Params & params); -void copyPaths(ref from, ref to, const Paths & storePaths, bool substitute = false); +void copyPaths(ref from, ref to, const PathSet & storePaths, bool substitute = false); enum StoreType { tDaemon, diff --git a/src/nix-copy-closure/nix-copy-closure.cc b/src/nix-copy-closure/nix-copy-closure.cc index 05a4deea3..ab80d96b5 100755 --- a/src/nix-copy-closure/nix-copy-closure.cc +++ b/src/nix-copy-closure/nix-copy-closure.cc @@ -58,6 +58,6 @@ int main(int argc, char ** argv) PathSet closure; from->computeFSClosure(storePaths2, closure, false, includeOutputs); - copyPaths(from, to, Paths(closure.begin(), closure.end()), useSubstitutes); + copyPaths(from, to, closure, useSubstitutes); }); } diff --git a/src/nix/copy.cc b/src/nix/copy.cc index 976b0d3e0..083dc3506 100644 --- a/src/nix/copy.cc +++ b/src/nix/copy.cc @@ -46,7 +46,7 @@ struct CmdCopy : StorePathsCommand ref srcStore = srcUri.empty() ? store : openStore(srcUri); ref dstStore = dstUri.empty() ? store : openStore(dstUri); - copyPaths(srcStore, dstStore, storePaths); + copyPaths(srcStore, dstStore, PathSet(storePaths.begin(), storePaths.end())); } };