From bec4da9e2669fbcb85cea742220ad39cbcf383f2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 21 Jan 2026 20:48:35 -0500 Subject: [PATCH] `NarCache` improvements - Separate implementation, abstract implements - `NarCache` work in constant memory This is using `RestoreSink` to dedup some write-side IO, but I am not so sure that is a good idea. We'll continue reworking it. - Use pathlocks to avoid download storms - Use `Descriptor`-based logic, not `pathExists` to avoid symlink races if anything is deleted. --- src/libstore/binary-cache-store.cc | 2 +- .../include/nix/store/local-nar-cache.hh | 20 +++ src/libstore/include/nix/store/meson.build | 1 + .../include/nix/store/remote-fs-accessor.hh | 4 +- src/libstore/local-nar-cache.cc | 134 ++++++++++++++++++ src/libstore/meson.build | 1 + src/libstore/remote-fs-accessor.cc | 9 +- src/libstore/remote-store.cc | 2 +- src/libutil/include/nix/util/nar-cache.hh | 27 ++-- src/libutil/nar-cache.cc | 92 +++--------- 10 files changed, 199 insertions(+), 93 deletions(-) create mode 100644 src/libstore/include/nix/store/local-nar-cache.hh create mode 100644 src/libstore/local-nar-cache.cc diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 87d46f84b..27e434732 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -562,7 +562,7 @@ ref BinaryCacheStore::getFSAccessor(bool requireValidPath) std::shared_ptr BinaryCacheStore::getFSAccessor(const StorePath & storePath, bool requireValidPath) { - return getRemoteFSAccessor(requireValidPath)->accessObject(storePath); + return static_cast>(getRemoteFSAccessor(requireValidPath)->accessObject(storePath)); } void BinaryCacheStore::addSignatures(const StorePath & storePath, const std::set & sigs) diff --git a/src/libstore/include/nix/store/local-nar-cache.hh b/src/libstore/include/nix/store/local-nar-cache.hh new file mode 100644 index 000000000..0edf171b2 --- /dev/null +++ b/src/libstore/include/nix/store/local-nar-cache.hh @@ -0,0 +1,20 @@ +#pragma once +///@file + +#include "nix/util/nar-cache.hh" + +#include +#include + +namespace nix { + +/** + * Create a NAR cache with local disk storage. + * + * Uses file locks to ensure only one process downloads a NAR at a time. + * + * @param cacheDir Directory to store cached NAR files + */ +std::unique_ptr makeLocalNarCache(std::filesystem::path cacheDir); + +} // namespace nix diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index c7026818e..d1abf4a8f 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -50,6 +50,7 @@ headers = [ config_pub_h ] + files( 'length-prefixed-protocol-helper.hh', 'local-binary-cache-store.hh', 'local-fs-store.hh', + 'local-nar-cache.hh', 'local-overlay-store.hh', 'local-store.hh', 'log-store.hh', diff --git a/src/libstore/include/nix/store/remote-fs-accessor.hh b/src/libstore/include/nix/store/remote-fs-accessor.hh index a8c5c8a0a..5bf0485f8 100644 --- a/src/libstore/include/nix/store/remote-fs-accessor.hh +++ b/src/libstore/include/nix/store/remote-fs-accessor.hh @@ -19,7 +19,7 @@ class RemoteFSAccessor : public SourceAccessor */ std::map> narHashes; - NarCache narCache; + std::unique_ptr narCache; bool requireValidPath; @@ -32,7 +32,7 @@ public: /** * @return nullptr if the store does not contain any object at that path. */ - std::shared_ptr accessObject(const StorePath & path); + ref accessObject(const StorePath & path); RemoteFSAccessor( ref store, bool requireValidPath = true, std::optional cacheDir = {}); diff --git a/src/libstore/local-nar-cache.cc b/src/libstore/local-nar-cache.cc new file mode 100644 index 000000000..0e0e3c985 --- /dev/null +++ b/src/libstore/local-nar-cache.cc @@ -0,0 +1,134 @@ +#include "nix/util/nar-cache.hh" +#include "nix/util/file-system.hh" +#include "nix/util/file-descriptor.hh" +#include "nix/util/fs-sink.hh" +#include "nix/store/pathlocks.hh" + +#include +#include + +namespace nix { + +namespace { + +/** + * NAR cache with local disk storage (private implementation). + * + * Uses file locks to ensure only one process downloads a NAR at a time. + */ +class LocalNarCache : public NarCache +{ + RestoreSink cacheSink; + +public: + + LocalNarCache(std::filesystem::path cacheDir) + : cacheSink(false) + { + createDirs(cacheDir); + cacheSink.dstPath = std::move(cacheDir); + } + + ref getOrInsert(const Hash & narHash, std::function populate) override; +}; + +} // anonymous namespace + +ref LocalNarCache::getOrInsert(const Hash & narHash, std::function populate) +{ + // Check in-memory cache first + if (auto * accessor = get(nars, narHash)) + return *accessor; + + auto cacheAccessor = [&](ref accessor) { + nars.emplace(narHash, accessor); + return accessor; + }; + + auto makeCacheFile = [&](const std::string & ext) -> CanonPath { + return {narHash.to_string(HashFormat::Nix32, false) + "." + ext}; + }; + + auto cacheFile = makeCacheFile("nar"); + auto listingFile = makeCacheFile("ls"); + auto lockFile = makeCacheFile("lock"); + + auto cacheFilePath = cacheSink.dstPath / cacheFile.rel(); + auto lockFilePath = cacheSink.dstPath / lockFile.rel(); + auto listingFilePath = cacheSink.dstPath / listingFile.rel(); + + // Helper to try loading from cache files using FD operations to avoid race conditions + auto tryLoadFromCache = [&]() -> std::optional> { + try { + // Try to open cache file - will throw if doesn't exist + AutoCloseFD cacheFD = openFileReadonly(cacheFilePath); + + // Try lazy accessor with listing file first + try { + AutoCloseFD listingFD = openFileReadonly(listingFilePath); + auto listingContent = readFile(listingFD.get()); + return cacheAccessor(makeLazyNarAccessor( + nlohmann::json::parse(listingContent).template get(), + seekableGetNarBytes(cacheFilePath))); + } catch (SystemError &) { + // Listing file missing or invalid, fall back to full NAR + } + + // Fall back to reading full NAR + auto narContent = readFile(cacheFD.get()); + return cacheAccessor(makeNarAccessor(std::move(narContent))); + } catch (SystemError &) { + // Cache file doesn't exist or can't be opened + return std::nullopt; + } + }; + + // Check if already cached (before acquiring lock) + if (auto accessor = tryLoadFromCache()) + return *accessor; + + // Acquire lock to ensure only one process downloads this NAR + AutoCloseFD lockFD = openLockFile(lockFilePath, true); + FdLock lock(lockFD.get(), ltWrite, true, "waiting for NAR cache lock"); + + // Check again after acquiring lock (another process might have just finished) + if (auto accessor = tryLoadFromCache()) + return *accessor; + + // Download and cache the NAR + NarListing listing; + try { + /* FIXME: do this asynchronously. */ + cacheSink.createRegularFile(cacheFile, [&](CreateRegularFileSink & fileSink) { + auto source = sinkToSource([&](Sink & parseSink) { + TeeSink teeSink{fileSink, parseSink}; + populate(teeSink); + }); + listing = parseNarListing(*source); + }); + } catch (...) { + ignoreExceptionExceptInterrupt(); + StringSink narSink; + populate(narSink); + return cacheAccessor(makeNarAccessor(std::move(narSink.s))); + } + + try { + cacheSink.createRegularFile(listingFile, [&](CreateRegularFileSink & sink) { + auto s = nlohmann::json(listing).dump(); + StringSource source{s}; + source.drainInto(sink); + }); + } catch (...) { + ignoreExceptionExceptInterrupt(); + } + + return cacheAccessor(makeLazyNarAccessor(std::move(listing), seekableGetNarBytes(cacheFilePath))); +} + +std::unique_ptr makeLocalNarCache(std::filesystem::path cacheDir) +{ + return std::make_unique(std::move(cacheDir)); +} + +} // namespace nix diff --git a/src/libstore/meson.build b/src/libstore/meson.build index a01aa903a..bb9c69011 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -315,6 +315,7 @@ sources = files( 'legacy-ssh-store.cc', 'local-binary-cache-store.cc', 'local-fs-store.cc', + 'local-nar-cache.cc', 'local-overlay-store.cc', 'local-store.cc', 'log-store.cc', diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc index b52baadd0..9c441b7bb 100644 --- a/src/libstore/remote-fs-accessor.cc +++ b/src/libstore/remote-fs-accessor.cc @@ -1,11 +1,12 @@ #include "nix/store/remote-fs-accessor.hh" +#include "nix/store/local-nar-cache.hh" namespace nix { RemoteFSAccessor::RemoteFSAccessor( ref store, bool requireValidPath, std::optional cacheDir) : store(store) - , narCache(cacheDir) + , narCache(cacheDir ? makeLocalNarCache(*cacheDir) : makeMemoryNarCache()) , requireValidPath(requireValidPath) { } @@ -18,11 +19,11 @@ std::pair, CanonPath> RemoteFSAccessor::fetch(const CanonPat return {ref{accessObject(storePath)}, CanonPath{restPath}}; } -std::shared_ptr RemoteFSAccessor::accessObject(const StorePath & storePath) +ref RemoteFSAccessor::accessObject(const StorePath & storePath) { // Check if we already have the NAR hash for this store path if (auto * narHash = get(narHashes, storePath.hashPart())) - return narCache.getOrInsert(*narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); }); + return narCache->getOrInsert(*narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); }); // Query the path info to get the NAR hash auto info = store->queryPathInfo(storePath); @@ -31,7 +32,7 @@ std::shared_ptr RemoteFSAccessor::accessObject(const StorePath & narHashes.emplace(storePath.hashPart(), info->narHash); // Get or create the NAR accessor - return narCache.getOrInsert(info->narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); }); + return narCache->getOrInsert(info->narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); }); } std::optional RemoteFSAccessor::maybeLstat(const CanonPath & path) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 103bb5a7e..d84c13ae6 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -841,7 +841,7 @@ ref RemoteStore::getFSAccessor(bool requireValidPath) std::shared_ptr RemoteStore::getFSAccessor(const StorePath & path, bool requireValidPath) { - return getRemoteFSAccessor(requireValidPath)->accessObject(path); + return static_cast>(getRemoteFSAccessor(requireValidPath)->accessObject(path)); } void RemoteStore::ConnectionHandle::withFramedSink(std::function fun) diff --git a/src/libutil/include/nix/util/nar-cache.hh b/src/libutil/include/nix/util/nar-cache.hh index 0f5ac2d53..ae8ccfbbc 100644 --- a/src/libutil/include/nix/util/nar-cache.hh +++ b/src/libutil/include/nix/util/nar-cache.hh @@ -3,45 +3,42 @@ #include "nix/util/hash.hh" #include "nix/util/nar-accessor.hh" #include "nix/util/ref.hh" -#include "nix/util/source-accessor.hh" #include #include #include -#include +#include namespace nix { /** - * A cache for NAR accessors with optional disk caching. + * Abstract cache for NAR accessors. */ class NarCache { - /** - * Optional directory for caching NARs and listings on disk. - */ - std::optional cacheDir; - +protected: /** * Map from NAR hash to NAR accessor. */ - std::map> nars; + std::map> nars; public: - /** - * Create a NAR cache with an optional cache directory for disk storage. - */ - NarCache(std::optional cacheDir = {}); + virtual ~NarCache() = default; /** - * Lookup or create a NAR accessor, optionally using disk cache. + * Lookup or create a NAR accessor. * * @param narHash The NAR hash to use as cache key * @param populate Function called with a Sink to populate the NAR if not cached * @return The cached or newly created accessor */ - ref getOrInsert(const Hash & narHash, std::function populate); + virtual ref getOrInsert(const Hash & narHash, std::function populate) = 0; }; +/** + * Create an in-memory only NAR cache. + */ +std::unique_ptr makeMemoryNarCache(); + } // namespace nix diff --git a/src/libutil/nar-cache.cc b/src/libutil/nar-cache.cc index ba5e60ced..8eb8fc674 100644 --- a/src/libutil/nar-cache.cc +++ b/src/libutil/nar-cache.cc @@ -1,84 +1,36 @@ #include "nix/util/nar-cache.hh" -#include "nix/util/file-system.hh" - -#include -#include -#include -#include namespace nix { -NarCache::NarCache(std::optional cacheDir_) - : cacheDir(std::move(cacheDir_)) -{ - if (cacheDir) - createDirs(*cacheDir); -} +namespace { -ref NarCache::getOrInsert(const Hash & narHash, std::function populate) +/** + * In-memory only NAR cache (private implementation). + */ +class MemoryNarCache : public NarCache +{ +public: + ref getOrInsert(const Hash & narHash, std::function populate) override; +}; + +} // anonymous namespace + +ref MemoryNarCache::getOrInsert(const Hash & narHash, std::function populate) { // Check in-memory cache first if (auto * accessor = get(nars, narHash)) return *accessor; - auto cacheAccessor = [&](ref accessor) { - nars.emplace(narHash, accessor); - return accessor; - }; + StringSink sink; + populate(sink); + auto accessor = makeNarAccessor(std::move(sink.s)); + nars.emplace(narHash, accessor); + return accessor; +} - auto getNar = [&]() { - StringSink sink; - populate(sink); - return std::move(sink.s); - }; - - if (cacheDir) { - auto makeCacheFile = [&](const std::string & ext) { - auto res = *cacheDir / narHash.to_string(HashFormat::Nix32, false); - res += "."; - res += ext; - return res; - }; - - auto cacheFile = makeCacheFile("nar"); - auto listingFile = makeCacheFile("ls"); - - if (nix::pathExists(cacheFile)) { - try { - return cacheAccessor(makeLazyNarAccessor( - nlohmann::json::parse(nix::readFile(listingFile)).template get(), - seekableGetNarBytes(cacheFile))); - } catch (SystemError &) { - } - - try { - return cacheAccessor(makeNarAccessor(nix::readFile(cacheFile))); - } catch (SystemError &) { - } - } - - auto nar = getNar(); - - try { - /* FIXME: do this asynchronously. */ - writeFile(cacheFile, nar); - } catch (...) { - ignoreExceptionExceptInterrupt(); - } - - auto narAccessor = makeNarAccessor(std::move(nar)); - - try { - nlohmann::json j = narAccessor->getListing(); - writeFile(listingFile, j.dump()); - } catch (...) { - ignoreExceptionExceptInterrupt(); - } - - return cacheAccessor(narAccessor); - } - - return cacheAccessor(makeNarAccessor(getNar())); +std::unique_ptr makeMemoryNarCache() +{ + return std::make_unique(); } } // namespace nix