From c82b67fa05a161600b264347321dfda618c79efc Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 10 Aug 2025 17:48:19 +0200 Subject: [PATCH 1/2] BasicClientConnection::queryPathInfo(): Don't throw exception for invalid paths This caused RemoteStore::queryPathInfoUncached() to mark the connection as invalid (see RemoteStore::ConnectionHandle::~ConnectionHandle()), causing it to disconnect and reconnect after every lookup of an invalid path. This caused huge slowdowns in conjunction with 19f89eb6842747570f262c003d977f02cb155968 and lazy-trees. --- .../include/nix/store/worker-protocol-connection.hh | 3 ++- src/libstore/remote-store.cc | 13 +++++++------ src/libstore/worker-protocol-connection.cc | 6 +++--- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/libstore/include/nix/store/worker-protocol-connection.hh b/src/libstore/include/nix/store/worker-protocol-connection.hh index f7ddfea4f..73dd50719 100644 --- a/src/libstore/include/nix/store/worker-protocol-connection.hh +++ b/src/libstore/include/nix/store/worker-protocol-connection.hh @@ -109,7 +109,8 @@ struct WorkerProto::BasicClientConnection : WorkerProto::BasicConnection const StorePathSet & paths, SubstituteFlag maybeSubstitute); - UnkeyedValidPathInfo queryPathInfo(const StoreDirConfig & store, bool * daemonException, const StorePath & path); + std::optional + queryPathInfo(const StoreDirConfig & store, bool * daemonException, const StorePath & path); void putBuildDerivationRequest( const StoreDirConfig & store, diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index d3446093d..8c2f268c3 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -259,13 +259,14 @@ void RemoteStore::queryPathInfoUncached( const StorePath & path, Callback> callback) noexcept { try { - std::shared_ptr info; - { + auto info = ({ auto conn(getConnection()); - info = std::make_shared( - StorePath{path}, conn->queryPathInfo(*this, &conn.daemonException, path)); - } - callback(std::move(info)); + conn->queryPathInfo(*this, &conn.daemonException, path); + }); + if (!info) + callback(nullptr); + else + callback(std::make_shared(StorePath{path}, *info)); } catch (...) { callback.rethrow(); } diff --git a/src/libstore/worker-protocol-connection.cc b/src/libstore/worker-protocol-connection.cc index 015a79ad6..987d0c8dd 100644 --- a/src/libstore/worker-protocol-connection.cc +++ b/src/libstore/worker-protocol-connection.cc @@ -244,7 +244,7 @@ void WorkerProto::BasicServerConnection::postHandshake(const StoreDirConfig & st WorkerProto::write(store, *this, info); } -UnkeyedValidPathInfo WorkerProto::BasicClientConnection::queryPathInfo( +std::optional WorkerProto::BasicClientConnection::queryPathInfo( const StoreDirConfig & store, bool * daemonException, const StorePath & path) { to << WorkerProto::Op::QueryPathInfo << store.printStorePath(path); @@ -253,14 +253,14 @@ UnkeyedValidPathInfo WorkerProto::BasicClientConnection::queryPathInfo( } catch (Error & e) { // Ugly backwards compatibility hack. if (e.msg().find("is not valid") != std::string::npos) - throw InvalidPath(std::move(e.info())); + return std::nullopt; throw; } if (GET_PROTOCOL_MINOR(protoVersion) >= 17) { bool valid; from >> valid; if (!valid) - throw InvalidPath("path '%s' is not valid", store.printStorePath(path)); + return std::nullopt; } return WorkerProto::Serialise::read(store, *this); } From f51779ee2518c961cf4c063e676ea328cb9ea6cf Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 10 Aug 2025 21:40:03 +0200 Subject: [PATCH 2/2] RemoteStore::addToStoreFromDump(): Invalidate cache entry for added path --- src/libstore/include/nix/store/store-api.hh | 2 ++ src/libstore/remote-store.cc | 4 +++- src/libstore/store-api.cc | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 528375851..6393ccbc7 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -311,6 +311,8 @@ protected: LRUCache pathInfoCache; }; + void invalidatePathInfoCacheFor(const StorePath & path); + SharedSync state; std::shared_ptr diskCache; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 8c2f268c3..5694fa466 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -457,7 +457,9 @@ StorePath RemoteStore::addToStoreFromDump( } if (fsm != dumpMethod) unsupported("RemoteStore::addToStoreFromDump doesn't support this `dumpMethod` `hashMethod` combination"); - return addCAToStore(dump, name, hashMethod, hashAlgo, references, repair)->path; + auto storePath = addCAToStore(dump, name, hashMethod, hashAlgo, references, repair)->path; + invalidatePathInfoCacheFor(storePath); + return storePath; } void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index dd8c39557..bd5ae9284 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -319,6 +319,11 @@ bool Store::PathInfoCacheValue::isKnownNow() return std::chrono::steady_clock::now() < time_point + ttl; } +void Store::invalidatePathInfoCacheFor(const StorePath & path) +{ + state.lock()->pathInfoCache.erase(path.to_string()); +} + std::map> Store::queryStaticPartialDerivationOutputMap(const StorePath & path) { std::map> outputs;