diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index f0e113eb0959..ba78a88343c9 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -78,9 +78,12 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) if (!drv->type().hasKnownOutputPaths()) experimentalFeatureSettings.require(Xp::CaDerivations); + StorePathSet outputPaths; for (auto & i : drv->outputsAndOptPaths(worker.store)) if (i.second.second) - worker.store.addTempRoot(*i.second.second); + outputPaths.insert(*i.second.second); + + worker.store.addTempRoots(outputPaths); /* We don't yet have any safe way to cache an impure derivation at this step. */ diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 50368a53cd67..31cc2c512719 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -695,6 +695,15 @@ static void performOp( break; } + case WorkerProto::Op::AddTempRoots: { + auto paths = WorkerProto::Serialise::read(*store, rconn); + logger->startWork(); + store->addTempRoots(paths); + logger->stopWork(); + conn.to << 1; + break; + } + case WorkerProto::Op::AddPermRoot: { if (!trusted) throw Error( diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 4f5541c47b1b..a4814779f34d 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -75,7 +75,7 @@ void LocalStore::createTempRootsFile() } } -void LocalStore::addTempRoot(const StorePath & path) +void LocalStore::addTempRoots(const StorePathSet & paths) { if (config->readOnly) { debug( @@ -124,12 +124,14 @@ void LocalStore::addTempRoot(const StorePath & path) } try { - debug("sending GC root '%s'", printStorePath(path)); - writeFull(fdRootsSocket->get(), printStorePath(path) + "\n", false); - char c; - readFull(fdRootsSocket->get(), &c, 1); - assert(c == '1'); - debug("got ack for GC root '%s'", printStorePath(path)); + for (auto & path : paths) { + debug("sending GC root '%s'", printStorePath(path)); + writeFull(fdRootsSocket->get(), printStorePath(path) + "\n", false); + char c; + readFull(fdRootsSocket->get(), &c, 1); + assert(c == '1'); + debug("got ack for GC root '%s'", printStorePath(path)); + } } catch (SystemError & e) { /* The garbage collector may have exited, so we need to restart. */ @@ -146,10 +148,22 @@ void LocalStore::addTempRoot(const StorePath & path) } } - /* Record the store path in the temporary roots file so it will be + /* Record the store paths in the temporary roots file so they will be seen by a future run of the garbage collector. */ - auto s = printStorePath(path) + '\0'; - writeFull(_fdTempRoots.lock()->get(), s); + + std::string s; + + for (auto & path : paths) + s += printStorePath(path) + '\0'; + + { + auto fdTempRoots(_fdTempRoots.lock()); + + /* This might not be atomic, but that's fine. Writes go in-order, and if + we partially write a store path, findTempRoots() will just ignore it, + and we'll send it the new temproots below if it's still running. */ + writeFull(fdTempRoots->get(), s); + } } static std::string censored = "{censored}"; diff --git a/src/libstore/include/nix/store/gc-store.hh b/src/libstore/include/nix/store/gc-store.hh index d3d42470e2c9..661d0f28b03a 100644 --- a/src/libstore/include/nix/store/gc-store.hh +++ b/src/libstore/include/nix/store/gc-store.hh @@ -93,7 +93,7 @@ struct GCResults * * The notion of GC roots actually not part of this class. * - * - The base `Store` class has `Store::addTempRoot()` because for a store + * - The base `Store` class has `Store::addTempRoots()` because for a store * that doesn't support garbage collection at all, a temporary GC root is * safely implementable as no-op. * diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index 1add5d2b62c1..a94914d7a72e 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -299,7 +299,7 @@ public: RepairFlag repair, std::shared_ptr provenance) override; - void addTempRoot(const StorePath & path) override; + void addTempRoots(const StorePathSet & paths) override; private: diff --git a/src/libstore/include/nix/store/remote-store.hh b/src/libstore/include/nix/store/remote-store.hh index 289d694aedd3..3333c7130779 100644 --- a/src/libstore/include/nix/store/remote-store.hh +++ b/src/libstore/include/nix/store/remote-store.hh @@ -121,7 +121,7 @@ struct RemoteStore : public virtual Store, void ensurePath(const StorePath & path) override; - void addTempRoot(const StorePath & path) override; + void addTempRoots(const StorePathSet & paths) override; Roots findRoots(bool censor) override; diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index e16e2c77d36d..87a0d0f37530 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -727,9 +727,18 @@ public: * Add a store path as a temporary root of the garbage collector. * The root disappears as soon as we exit. */ - virtual void addTempRoot(const StorePath & path) + void addTempRoot(const StorePath & path) { - debug("not creating temporary root, store doesn't support GC"); + addTempRoots({path}); + } + + /** + * Add multiple store paths as temporary roots of the garbage collector. + * The roots disappears as soon as we exit. + */ + virtual void addTempRoots(const StorePathSet & paths) + { + debug("not creating temporary roots, store doesn't support GC"); } /** diff --git a/src/libstore/include/nix/store/worker-protocol.hh b/src/libstore/include/nix/store/worker-protocol.hh index dd7b3a603750..05b6ecc76499 100644 --- a/src/libstore/include/nix/store/worker-protocol.hh +++ b/src/libstore/include/nix/store/worker-protocol.hh @@ -113,6 +113,7 @@ struct WorkerProto static constexpr std::string_view featureQueryActiveBuilds = "queryActiveBuilds"; static constexpr std::string_view featureProvenance = "provenance"; static constexpr std::string_view featureVersionedAddToStoreMultiple = "versionedAddToStoreMultiple"; + static constexpr std::string_view featureAddTempRoots = "addTempRoots"; /** * A unidirectional read connection, to be used by the read half of the @@ -238,6 +239,7 @@ enum struct WorkerProto::Op : uint64_t { BuildPathsWithResults = 46, AddPermRoot = 47, QueryActiveBuilds = 48, + AddTempRoots = 49, }; struct WorkerProto::ClientHandshakeInfo diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 14665716c345..0532029f872c 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -693,10 +693,26 @@ void RemoteStore::ensurePath(const StorePath & path) readInt(conn->from); } -void RemoteStore::addTempRoot(const StorePath & path) +void RemoteStore::addTempRoots(const StorePathSet & paths) { + if (paths.empty()) + return; + auto conn(getConnection()); - conn->addTempRoot(*this, &conn.daemonException, path); + + if (conn->protoVersion.features.contains(WorkerProto::featureAddTempRoots)) { + conn->to << WorkerProto::Op::AddTempRoots; + WorkerProto::write(*this, *conn, paths); + conn.processStderr(); + readInt(conn->from); + } else { + /* Fallback for daemons that don't support the batched + operation. Note that this is very slow for large sets of + paths on high-latency links, due to a network round-trip per + path. */ + for (auto & path : paths) + conn->addTempRoot(*this, &conn.daemonException, path); + } } Roots RemoteStore::findRoots(bool censor) diff --git a/src/libstore/restricted-store.cc b/src/libstore/restricted-store.cc index 06bf5746d7fb..09fea5e4514a 100644 --- a/src/libstore/restricted-store.cc +++ b/src/libstore/restricted-store.cc @@ -124,7 +124,7 @@ struct RestrictedStore : public virtual IndirectRootStore, public virtual GcStor unsupported("buildDerivation"); } - void addTempRoot(const StorePath & path) override {} + void addTempRoots(const StorePathSet & paths) override {} void addIndirectRoot(const std::filesystem::path & path) override {} diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e53e741acb4b..d2effb6353cd 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -974,8 +974,7 @@ std::map copyPaths( CheckSigsFlag checkSigs, SubstituteFlag substitute) { - for (auto & path : storePaths) - dstStore.addTempRoot(path); + dstStore.addTempRoots(storePaths); auto valid = dstStore.queryValidPaths(storePaths, substitute); diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index 3c7acbfb4784..8baab6442317 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -26,6 +26,7 @@ const WorkerProto::Version WorkerProto::latest = { std::string{WorkerProto::featureQueryActiveBuilds}, std::string{WorkerProto::featureProvenance}, std::string{WorkerProto::featureVersionedAddToStoreMultiple}, + std::string{WorkerProto::featureAddTempRoots}, }, }; diff --git a/src/nix/nix-store/nix-store.cc b/src/nix/nix-store/nix-store.cc index 1cc4d7aa9c07..0c36d7d39c9c 100644 --- a/src/nix/nix-store/nix-store.cc +++ b/src/nix/nix-store/nix-store.cc @@ -953,8 +953,7 @@ static void opServe(Strings opFlags, Strings opArgs) bool substitute = readInt(in); auto paths = ServeProto::Serialise::read(*store, rconn); if (lock && writeAllowed) - for (auto & path : paths) - store->addTempRoot(path); + store->addTempRoots(paths); if (substitute && writeAllowed) { store->substitutePaths(paths);