From 5b2aa61f1b8fe253c963874ccb4700f6ff99526e Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 26 Oct 2021 16:55:57 +0200 Subject: [PATCH 1/4] =?UTF-8?q?Don=E2=80=99t=20require=20`ca-derivations`?= =?UTF-8?q?=20when=20`=5F=5FcontentAddressed=20=3D=20false`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we explicitely opt-out of it, there’s no need to require the experimental feature --- src/libexpr/primops.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 6b3cafec8..f05cca169 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -985,8 +985,9 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * } if (i->name == state.sContentAddressed) { - settings.requireExperimentalFeature(Xp::CaDerivations); contentAddressed = state.forceBool(*i->value, pos); + if (contentAddressed) + settings.requireExperimentalFeature(Xp::CaDerivations); } /* The `args' attribute is special: it supplies the From 96670ed2163d3d1a296c9b053833362ec8c06985 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 27 Oct 2021 11:36:51 +0200 Subject: [PATCH 2/4] Expose an async interface for `queryRealisation` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doesn’t change much so far because everything is still using it synchronously, but should allow the binary cache to fetch stuff in parallel --- src/libstore/binary-cache-store.cc | 30 ++------ src/libstore/binary-cache-store.hh | 3 +- .../build/drv-output-substitution-goal.hh | 2 +- src/libstore/build/local-derivation-goal.cc | 7 +- src/libstore/dummy-store.cc | 5 +- src/libstore/legacy-ssh-store.cc | 3 +- src/libstore/local-store.cc | 23 +++++-- src/libstore/local-store.hh | 3 +- src/libstore/remote-store.cc | 34 ++++++---- src/libstore/remote-store.hh | 3 +- src/libstore/store-api.cc | 68 +++++++++++++++++++ src/libstore/store-api.hh | 12 +++- 12 files changed, 138 insertions(+), 55 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 943132754..fb687db42 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -437,39 +437,19 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s })->path; } -std::optional BinaryCacheStore::queryRealisation(const DrvOutput & id) +void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id, + Callback> callback) noexcept { - if (diskCache) { - auto [cacheOutcome, maybeCachedRealisation] = - diskCache->lookupRealisation(getUri(), id); - switch (cacheOutcome) { - case NarInfoDiskCache::oValid: - debug("Returning a cached realisation for %s", id.to_string()); - return *maybeCachedRealisation; - case NarInfoDiskCache::oInvalid: - debug("Returning a cached missing realisation for %s", id.to_string()); - return {}; - case NarInfoDiskCache::oUnknown: - break; - } - } - auto outputInfoFilePath = realisationsPrefix + "/" + id.to_string() + ".doi"; auto rawOutputInfo = getFile(outputInfoFilePath); if (rawOutputInfo) { auto realisation = Realisation::fromJSON( nlohmann::json::parse(*rawOutputInfo), outputInfoFilePath); - - if (diskCache) - diskCache->upsertRealisation( - getUri(), realisation); - - return {realisation}; + callback(std::make_shared(realisation)); + return; } else { - if (diskCache) - diskCache->upsertAbsentRealisation(getUri(), id); - return std::nullopt; + callback(nullptr); } } diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 723f2e805..87ce610af 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -108,7 +108,8 @@ public: void registerDrvOutput(const Realisation & info) override; - std::optional queryRealisation(const DrvOutput &) override; + void queryRealisationUncached(const DrvOutput &, + Callback> callback) noexcept override; void narFromPath(const StorePath & path, Sink & sink) override; diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 63ab53d89..1b11a298e 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -20,7 +20,7 @@ private: // The realisation corresponding to the given output id. // Will be filled once we can get it. - std::optional outputInfo; + std::shared_ptr outputInfo; /* The remaining substituters. */ std::list> subs; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 2182f0bb4..b076bf998 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1224,13 +1224,14 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo // corresponds to an allowed derivation { throw Error("registerDrvOutput"); } - std::optional queryRealisation(const DrvOutput & id) override + void queryRealisationUncached(const DrvOutput & id, + Callback> callback) noexcept override // XXX: This should probably be allowed if the realisation corresponds to // an allowed derivation { if (!goal.isAllowed(id)) - throw InvalidPath("cannot query an unknown output id '%s' in recursive Nix", id.to_string()); - return next->queryRealisation(id); + callback(nullptr); + next->queryRealisation(id, std::move(callback)); } void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 36c6e725c..62dc21c59 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -50,8 +50,9 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store void narFromPath(const StorePath & path, Sink & sink) override { unsupported("narFromPath"); } - std::optional queryRealisation(const DrvOutput&) override - { unsupported("queryRealisation"); } + void queryRealisationUncached(const DrvOutput &, + Callback> callback) noexcept override + { callback(nullptr); } }; static RegisterStoreImplementation regDummyStore; diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 814960bb5..c9c3be1b0 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -367,7 +367,8 @@ public: return conn->remoteVersion; } - std::optional queryRealisation(const DrvOutput&) override + void queryRealisationUncached(const DrvOutput &, + Callback> callback) noexcept override // TODO: Implement { unsupported("queryRealisation"); } }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 225a19e1e..92d8c36ac 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1838,13 +1838,24 @@ std::optional LocalStore::queryRealisation_( return { res }; } -std::optional -LocalStore::queryRealisation(const DrvOutput & id) +void LocalStore::queryRealisationUncached(const DrvOutput & id, + Callback> callback) noexcept { - return retrySQLite>([&]() { - auto state(_state.lock()); - return queryRealisation_(*state, id); - }); + try { + auto maybeRealisation + = retrySQLite>([&]() { + auto state(_state.lock()); + return queryRealisation_(*state, id); + }); + if (maybeRealisation) + callback( + std::make_shared(maybeRealisation.value())); + else + callback(nullptr); + + } catch (...) { + callback.rethrow(); + } } FixedOutputHash LocalStore::hashCAPath( diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 301425eb1..b172e897a 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -207,7 +207,8 @@ public: std::optional queryRealisation_(State & state, const DrvOutput & id); std::optional> queryRealisationCore_(State & state, const DrvOutput & id); - std::optional queryRealisation(const DrvOutput&) override; + void queryRealisationUncached(const DrvOutput&, + Callback> callback) noexcept override; private: diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 7decc059c..9c00d6212 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -677,23 +677,33 @@ void RemoteStore::registerDrvOutput(const Realisation & info) conn.processStderr(); } -std::optional RemoteStore::queryRealisation(const DrvOutput & id) +void RemoteStore::queryRealisationUncached(const DrvOutput & id, + Callback> callback) noexcept { auto conn(getConnection()); conn->to << wopQueryRealisation; conn->to << id.to_string(); conn.processStderr(); - if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 31) { - auto outPaths = worker_proto::read(*this, conn->from, Phantom>{}); - if (outPaths.empty()) - return std::nullopt; - return {Realisation{.id = id, .outPath = *outPaths.begin()}}; - } else { - auto realisations = worker_proto::read(*this, conn->from, Phantom>{}); - if (realisations.empty()) - return std::nullopt; - return *realisations.begin(); - } + + auto real = [&]() -> std::shared_ptr { + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 31) { + auto outPaths = worker_proto::read( + *this, conn->from, Phantom> {}); + if (outPaths.empty()) + return nullptr; + return std::make_shared(Realisation { .id = id, .outPath = *outPaths.begin() }); + } else { + auto realisations = worker_proto::read( + *this, conn->from, Phantom> {}); + if (realisations.empty()) + return nullptr; + return std::make_shared(*realisations.begin()); + } + }(); + + try { + callback(std::shared_ptr(real)); + } catch (...) { return callback.rethrow(); } } static void writeDerivedPaths(RemoteStore & store, ConnectionHandle & conn, const std::vector & reqs) diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index a3036e6b0..79d031a24 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -88,7 +88,8 @@ public: void registerDrvOutput(const Realisation & info) override; - std::optional queryRealisation(const DrvOutput &) override; + void queryRealisationUncached(const DrvOutput &, + Callback> callback) noexcept override; void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b0d3688ce..b8d702200 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -542,6 +542,74 @@ void Store::queryPathInfo(const StorePath & storePath, }}); } +void Store::queryRealisation(const DrvOutput & id, + Callback> callback) noexcept +{ + + try { + if (diskCache) { + auto [cacheOutcome, maybeCachedRealisation] + = diskCache->lookupRealisation(getUri(), id); + switch (cacheOutcome) { + case NarInfoDiskCache::oValid: + debug("Returning a cached realisation for %s", id.to_string()); + callback(maybeCachedRealisation); + return; + case NarInfoDiskCache::oInvalid: + debug( + "Returning a cached missing realisation for %s", + id.to_string()); + callback(nullptr); + return; + case NarInfoDiskCache::oUnknown: + break; + } + } + } catch (...) { + return callback.rethrow(); + } + + auto callbackPtr + = std::make_shared(std::move(callback)); + + queryRealisationUncached( + id, + { [this, id, callbackPtr]( + std::future> fut) { + try { + auto info = fut.get(); + + if (diskCache) { + if (info) + diskCache->upsertRealisation(getUri(), *info); + else + diskCache->upsertAbsentRealisation(getUri(), id); + } + + (*callbackPtr)(std::shared_ptr(info)); + + } catch (...) { + callbackPtr->rethrow(); + } + } }); +} + +std::shared_ptr Store::queryRealisation(const DrvOutput & id) +{ + using RealPtr = std::shared_ptr; + std::promise promise; + + queryRealisation(id, + {[&](std::future result) { + try { + promise.set_value(result.get()); + } catch (...) { + promise.set_exception(std::current_exception()); + } + }}); + + return promise.get_future().get(); +} void Store::substitutePaths(const StorePathSet & paths) { diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 7d02340df..8adcf276f 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -369,6 +369,14 @@ public: void queryPathInfo(const StorePath & path, Callback> callback) noexcept; + /* Query the information about a realisation. */ + std::shared_ptr queryRealisation(const DrvOutput &); + + /* Asynchronous version of queryRealisation(). */ + void queryRealisation(const DrvOutput &, + Callback> callback) noexcept; + + /* Check whether the given valid path info is sufficiently attested, by either being signed by a trusted public key or content-addressed, in order to be included in the given store. @@ -393,11 +401,11 @@ protected: virtual void queryPathInfoUncached(const StorePath & path, Callback> callback) noexcept = 0; + virtual void queryRealisationUncached(const DrvOutput &, + Callback> callback) noexcept = 0; public: - virtual std::optional queryRealisation(const DrvOutput &) = 0; - /* Queries the set of incoming FS references for a store path. The result is not cleared. */ virtual void queryReferrers(const StorePath & path, StorePathSet & referrers) From fbc70034b3f5bd82d0cfe24a9a82a6d00237b46e Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 27 Oct 2021 11:50:57 +0200 Subject: [PATCH 3/4] Make the realisation fetching from binary caches async That way we can fetch several realisations from the same cache in parallel --- src/libstore/binary-cache-store.cc | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index fb687db42..08dde0c54 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -441,16 +441,25 @@ void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id, Callback> callback) noexcept { auto outputInfoFilePath = realisationsPrefix + "/" + id.to_string() + ".doi"; - auto rawOutputInfo = getFile(outputInfoFilePath); - if (rawOutputInfo) { - auto realisation = Realisation::fromJSON( - nlohmann::json::parse(*rawOutputInfo), outputInfoFilePath); - callback(std::make_shared(realisation)); - return; - } else { - callback(nullptr); - } + auto callbackPtr = std::make_shared(std::move(callback)); + + Callback> newCallback = { + [=](std::future> fut) { + try { + auto data = fut.get(); + if (!data) return (*callbackPtr)(nullptr); + + auto realisation = Realisation::fromJSON( + nlohmann::json::parse(*data), outputInfoFilePath); + return (*callbackPtr)(std::make_shared(realisation)); + } catch (...) { + callbackPtr->rethrow(); + } + } + }; + + getFile(outputInfoFilePath, std::move(newCallback)); } void BinaryCacheStore::registerDrvOutput(const Realisation& info) { From f4c869977c391b31eb4f20486f7da03b026e2401 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 28 Oct 2021 16:43:54 +0200 Subject: [PATCH 4/4] Make the DrvOutputSubstitutionGoal more async --- .../build/drv-output-substitution-goal.cc | 44 +++++++++++++++++-- .../build/drv-output-substitution-goal.hh | 12 ++++- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index be270d079..b9602e696 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -1,6 +1,8 @@ #include "drv-output-substitution-goal.hh" +#include "finally.hh" #include "worker.hh" #include "substitution-goal.hh" +#include "callback.hh" namespace nix { @@ -50,14 +52,42 @@ void DrvOutputSubstitutionGoal::tryNext() return; } - auto sub = subs.front(); + sub = subs.front(); subs.pop_front(); // FIXME: Make async - outputInfo = sub->queryRealisation(id); + // outputInfo = sub->queryRealisation(id); + outPipe.create(); + promise = decltype(promise)(); + + sub->queryRealisation( + id, { [&](std::future> res) { + try { + Finally updateStats([this]() { outPipe.writeSide.close(); }); + promise.set_value(res.get()); + } catch (...) { + promise.set_exception(std::current_exception()); + } + } }); + + worker.childStarted(shared_from_this(), {outPipe.readSide.get()}, true, false); + + state = &DrvOutputSubstitutionGoal::realisationFetched; +} + +void DrvOutputSubstitutionGoal::realisationFetched() +{ + worker.childTerminated(this); + + try { + outputInfo = promise.get_future().get(); + } catch (std::exception & e) { + printError(e.what()); + substituterFailed = true; + } + if (!outputInfo) { - tryNext(); - return; + return tryNext(); } for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { @@ -119,4 +149,10 @@ void DrvOutputSubstitutionGoal::work() (this->*state)(); } +void DrvOutputSubstitutionGoal::handleEOF(int fd) +{ + if (fd == outPipe.readSide.get()) worker.wakeUp(shared_from_this()); +} + + } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 1b11a298e..67ae2624a 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -3,6 +3,8 @@ #include "store-api.hh" #include "goal.hh" #include "realisation.hh" +#include +#include namespace nix { @@ -25,6 +27,13 @@ private: /* The remaining substituters. */ std::list> subs; + /* The current substituter. */ + std::shared_ptr sub; + + Pipe outPipe; + std::thread thr; + std::promise> promise; + /* Whether a substituter failed. */ bool substituterFailed = false; @@ -36,6 +45,7 @@ public: void init(); void tryNext(); + void realisationFetched(); void outPathValid(); void finished(); @@ -44,7 +54,7 @@ public: string key() override; void work() override; - + void handleEOF(int fd) override; }; }