From 2f4a1dd6e03f3005e1f11dc98dda2d2d214b1f6f Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 27 Apr 2024 21:24:36 +0200 Subject: [PATCH] libstore: de-callback-ify Store::queryPathInfoUncached Change-Id: I23a156aaff5328f67ca16ccd85c0ea1711b21e35 --- src/libstore/binary-cache-store.cc | 15 +-- src/libstore/binary-cache-store.hh | 3 +- src/libstore/build/local-derivation-goal.cc | 9 +- src/libstore/dummy-store.cc | 5 +- src/libstore/legacy-ssh-store.cc | 41 +++--- src/libstore/local-store.cc | 14 +-- src/libstore/local-store.hh | 3 +- src/libstore/remote-store.cc | 41 +++--- src/libstore/remote-store.hh | 3 +- src/libstore/store-api.cc | 131 ++++++++------------ src/libstore/store-api.hh | 9 +- 11 files changed, 106 insertions(+), 168 deletions(-) diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index bf7265c35..88b8db972 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -349,8 +349,7 @@ void BinaryCacheStore::narFromPath(const StorePath & storePath, Sink & sink) stats.narReadBytes += narSize.length; } -void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, - Callback> callback) noexcept +std::shared_ptr BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath) { auto uri = getUri(); auto storePathS = printStorePath(storePath); @@ -360,17 +359,13 @@ void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, auto narInfoFile = narInfoFileFor(storePath); - try { - auto data = getFile(narInfoFile); + auto data = getFile(narInfoFile); - if (!data) return callback({}); + if (!data) return {}; - stats.narInfoRead++; + stats.narInfoRead++; - callback(std::make_shared(*this, *data, narInfoFile)); - } catch (...) { - callback.rethrow(); - } + return std::make_shared(*this, *data, narInfoFile); } StorePath BinaryCacheStore::addToStore( diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index af2b50084..b6fbcfbc8 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -107,8 +107,7 @@ public: bool isValidPathUncached(const StorePath & path) override; - void queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept override; + std::shared_ptr queryPathInfoUncached(const StorePath & path) override; std::optional queryPathFromHashPart(const std::string & hashPart) override; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index da1db5771..1b014605d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1260,8 +1260,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In return paths; } - void queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept override + std::shared_ptr queryPathInfoUncached(const StorePath & path) override { if (goal.isAllowed(path)) { try { @@ -1271,12 +1270,12 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In info->registrationTime = 0; info->ultimate = false; info->sigs.clear(); - callback(info); + return info; } catch (InvalidPath &) { - callback(nullptr); + return nullptr; } } else - callback(nullptr); + return nullptr; }; void queryReferrers(const StorePath & path, StorePathSet & referrers) override diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 74d6ed3b5..f1c2b43b1 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -33,10 +33,9 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store return *uriSchemes().begin(); } - void queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept override + std::shared_ptr queryPathInfoUncached(const StorePath & path) override { - callback(nullptr); + return nullptr; } /** diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 2d8667a85..5044876cb 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -143,36 +143,33 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor return *uriSchemes().begin() + "://" + host; } - void queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept override + std::shared_ptr queryPathInfoUncached(const StorePath & path) override { - try { - auto conn(connections->get()); + auto conn(connections->get()); - /* No longer support missing NAR hash */ - assert(GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4); + /* No longer support missing NAR hash */ + assert(GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4); - debug("querying remote host '%s' for info on '%s'", host, printStorePath(path)); + debug("querying remote host '%s' for info on '%s'", host, printStorePath(path)); - conn->to << ServeProto::Command::QueryPathInfos << PathSet{printStorePath(path)}; - conn->to.flush(); + conn->to << ServeProto::Command::QueryPathInfos << PathSet{printStorePath(path)}; + conn->to.flush(); - auto p = readString(conn->from); - if (p.empty()) return callback(nullptr); - auto path2 = parseStorePath(p); - assert(path == path2); - auto info = std::make_shared( - path, - ServeProto::Serialise::read(*this, *conn)); + auto p = readString(conn->from); + if (p.empty()) return nullptr; + auto path2 = parseStorePath(p); + assert(path == path2); + auto info = std::make_shared( + path, + ServeProto::Serialise::read(*this, *conn)); - if (info->narHash == Hash::dummy) - throw Error("NAR hash is now mandatory"); + if (info->narHash == Hash::dummy) + throw Error("NAR hash is now mandatory"); - auto s = readString(conn->from); - assert(s == ""); + auto s = readString(conn->from); + assert(s == ""); - callback(std::move(info)); - } catch (...) { callback.rethrow(); } + return info; } void addToStore(const ValidPathInfo & info, Source & source, diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 2f59b3591..ee401d7fc 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -880,16 +880,12 @@ uint64_t LocalStore::addValidPath(State & state, } -void LocalStore::queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept +std::shared_ptr LocalStore::queryPathInfoUncached(const StorePath & path) { - try { - callback(retrySQLite>([&]() { - auto state(_state.lock()); - return queryPathInfoInternal(*state, path); - })); - - } catch (...) { callback.rethrow(); } + return retrySQLite>([&]() { + auto state(_state.lock()); + return queryPathInfoInternal(*state, path); + }); } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index b8d1f02ab..daf50989c 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -168,8 +168,7 @@ public: StorePathSet queryAllValidPaths() override; - void queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept override; + std::shared_ptr queryPathInfoUncached(const StorePath & path) override; void queryReferrers(const StorePath & path, StorePathSet & referrers) override; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 20c1c50f2..94c426c02 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -304,32 +304,25 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S } -void RemoteStore::queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept +std::shared_ptr RemoteStore::queryPathInfoUncached(const StorePath & path) { + auto conn(getConnection()); + conn->to << WorkerProto::Op::QueryPathInfo << printStorePath(path); try { - std::shared_ptr info; - { - auto conn(getConnection()); - conn->to << WorkerProto::Op::QueryPathInfo << printStorePath(path); - try { - conn.processStderr(); - } catch (Error & e) { - // Ugly backwards compatibility hack. - if (e.msg().find("is not valid") != std::string::npos) - throw InvalidPath(std::move(e.info())); - throw; - } - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) { - bool valid; conn->from >> valid; - if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); - } - info = std::make_shared( - StorePath{path}, - WorkerProto::Serialise::read(*this, *conn)); - } - callback(std::move(info)); - } catch (...) { callback.rethrow(); } + conn.processStderr(); + } catch (Error & e) { + // Ugly backwards compatibility hack. + if (e.msg().find("is not valid") != std::string::npos) + throw InvalidPath(std::move(e.info())); + throw; + } + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) { + bool valid; conn->from >> valid; + if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); + } + return std::make_shared( + StorePath{path}, + WorkerProto::Serialise::read(*this, *conn)); } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 0cae81e16..7ce47bee2 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -53,8 +53,7 @@ public: StorePathSet queryAllValidPaths() override; - void queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept override; + std::shared_ptr queryPathInfoUncached(const StorePath & path) override; void queryReferrers(const StorePath & path, StorePathSet & referrers) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index f696e4c1f..553f9587a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -660,23 +660,6 @@ bool Store::isValidPathUncached(const StorePath & path) } -ref Store::queryPathInfo(const StorePath & storePath) -{ - std::promise> promise; - - queryPathInfo(storePath, - {[&](std::future> result) { - try { - promise.set_value(result.get()); - } catch (...) { - promise.set_exception(std::current_exception()); - } - }}); - - return promise.get_future().get(); -} - - static bool goodStorePath(const StorePath & expected, const StorePath & actual) { return @@ -685,64 +668,52 @@ static bool goodStorePath(const StorePath & expected, const StorePath & actual) } -void Store::queryPathInfo(const StorePath & storePath, - Callback> callback) noexcept +ref Store::queryPathInfo(const StorePath & storePath) { auto hashPart = std::string(storePath.hashPart()); - try { - { - auto res = state.lock()->pathInfoCache.get(std::string(storePath.to_string())); - if (res && res->isKnownNow()) { - stats.narInfoReadAverted++; - if (!res->didExist()) - throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); - return callback(ref(res->value)); - } + { + auto res = state.lock()->pathInfoCache.get(std::string(storePath.to_string())); + if (res && res->isKnownNow()) { + stats.narInfoReadAverted++; + if (!res->didExist()) + throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); + return ref(res->value); } + } - if (diskCache) { - auto res = diskCache->lookupNarInfo(getUri(), hashPart); - if (res.first != NarInfoDiskCache::oUnknown) { - stats.narInfoReadAverted++; - { - auto state_(state.lock()); - state_->pathInfoCache.upsert(std::string(storePath.to_string()), - res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second }); - if (res.first == NarInfoDiskCache::oInvalid || - !goodStorePath(storePath, res.second->path)) - throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); - } - return callback(ref(res.second)); - } - } - - } catch (...) { return callback.rethrow(); } - - auto callbackPtr = std::make_shared(std::move(callback)); - - queryPathInfoUncached(storePath, - {[this, storePath, hashPart, callbackPtr](std::future> fut) { - - try { - auto info = fut.get(); - - if (diskCache) - diskCache->upsertNarInfo(getUri(), hashPart, info); - - { - auto state_(state.lock()); - state_->pathInfoCache.upsert(std::string(storePath.to_string()), PathInfoCacheValue { .value = info }); - } - - if (!info || !goodStorePath(storePath, info->path)) { - stats.narInfoMissing++; + if (diskCache) { + auto res = diskCache->lookupNarInfo(getUri(), hashPart); + if (res.first != NarInfoDiskCache::oUnknown) { + stats.narInfoReadAverted++; + { + auto state_(state.lock()); + state_->pathInfoCache.upsert(std::string(storePath.to_string()), + res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second }); + if (res.first == NarInfoDiskCache::oInvalid || + !goodStorePath(storePath, res.second->path)) throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); - } + } + return ref(res.second); + } + } - (*callbackPtr)(ref(info)); - } catch (...) { callbackPtr->rethrow(); } - }}); + auto info = queryPathInfoUncached(storePath); + + if (diskCache) + diskCache->upsertNarInfo(getUri(), hashPart, info); + + { + auto state_(state.lock()); + state_->pathInfoCache.upsert(std::string(storePath.to_string()), PathInfoCacheValue { .value = info }); + } + + if (!info || !goodStorePath(storePath, info->path)) { + stats.narInfoMissing++; + throw InvalidPath("path '%s' is not valid", printStorePath(storePath)); + } + + return ref(info); } void Store::queryRealisation(const DrvOutput & id, @@ -852,19 +823,17 @@ StorePathSet Store::queryValidPaths(const StorePathSet & paths, SubstituteFlag m auto doQuery = [&](const StorePath & path) { checkInterrupt(); - queryPathInfo(path, {[path, &state_, &wakeup](std::future> fut) { - auto state(state_.lock()); - try { - auto info = fut.get(); - state->valid.insert(path); - } catch (InvalidPath &) { - } catch (...) { - state->exc = std::current_exception(); - } - assert(state->left); - if (!--state->left) - wakeup.notify_one(); - }}); + auto state(state_.lock()); + try { + auto info = queryPathInfo(path); + state->valid.insert(path); + } catch (InvalidPath &) { + } catch (...) { + state->exc = std::current_exception(); + } + assert(state->left); + if (!--state->left) + wakeup.notify_one(); }; for (auto & path : paths) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index cb9f8e4a6..8cdfff576 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -366,12 +366,6 @@ public: */ ref queryPathInfo(const StorePath & path); - /** - * Asynchronous version of queryPathInfo(). - */ - void queryPathInfo(const StorePath & path, - Callback> callback) noexcept; - /** * Query the information about a realisation. */ @@ -407,8 +401,7 @@ public: protected: - virtual void queryPathInfoUncached(const StorePath & path, - Callback> callback) noexcept = 0; + virtual std::shared_ptr queryPathInfoUncached(const StorePath & path) = 0; virtual void queryRealisationUncached(const DrvOutput &, Callback> callback) noexcept = 0;