libstore: extract NarInfoDiskCacheSettings from Settings

src/libstore-tests/nar-info-disk-cache.cc

@@ -25,7 +25,8 @@ TEST(NarInfoDiskCacheImpl, create_and_read)
     SQLiteStmt getIds;
 
     {
-        auto cache = getTestNarInfoDiskCache(dbPath.string());
+        auto cache = NarInfoDiskCache::getTest(
+            settings.getNarInfoDiskCacheSettings(), {.useWAL = settings.useSQLiteWAL}, dbPath.string());
 
         // Set up "background noise" and check that different caches receive different ids
         {
@@ -74,7 +75,8 @@ TEST(NarInfoDiskCacheImpl, create_and_read)
     {
         // We can't clear the in-memory cache, so we use a new cache object. This is
         // more realistic anyway.
-        auto cache2 = getTestNarInfoDiskCache(dbPath.string());
+        auto cache2 = NarInfoDiskCache::getTest(
+            settings.getNarInfoDiskCacheSettings(), {.useWAL = settings.useSQLiteWAL}, dbPath.string());
 
         {
             auto r = cache2->upToDateCacheExists("http://foo");

src/libstore/http-binary-cache-store.cc

@@ -2,8 +2,10 @@
 #include "nix/store/filetransfer.hh"
 #include "nix/store/globals.hh"
 #include "nix/store/nar-info-disk-cache.hh"
+#include "nix/store/sqlite.hh"
 #include "nix/util/callback.hh"
 #include "nix/store/store-registration.hh"
+#include "nix/store/globals.hh"
 
 namespace nix {
 
@@ -64,7 +66,7 @@ HttpBinaryCacheStore::HttpBinaryCacheStore(ref<Config> config, ref<FileTransfer>
     , fileTransfer{fileTransfer}
     , config{config}
 {
-    diskCache = getNarInfoDiskCache();
+    diskCache = NarInfoDiskCache::get(settings.getNarInfoDiskCacheSettings(), {.useWAL = settings.useSQLiteWAL});
 }
 
 void HttpBinaryCacheStore::init()

src/libstore/include/nix/store/globals.hh

@@ -50,7 +50,47 @@ public:
         {"build-compress-log"}};
 };
 
-class Settings : public virtual Config, private LocalSettings, private LogFileSettings, private WorkerSettings
+struct NarInfoDiskCacheSettings : public virtual Config
+{
+    Setting<unsigned int> ttlNegative{
+        this,
+        3600,
+        "narinfo-cache-negative-ttl",
+        R"(
+          The TTL in seconds for negative lookups.
+          If a store path is queried from a [substituter](#conf-substituters) but was not found, a negative lookup is cached in the local disk cache database for the specified duration.
+
+          Set to `0` to force updating the lookup cache.
+
+          To wipe the lookup cache completely:
+
+          ```shell-session
+          $ rm $HOME/.cache/nix/binary-cache-v*.sqlite*
+          # rm /root/.cache/nix/binary-cache-v*.sqlite*
+          ```
+        )"};
+
+    Setting<unsigned int> ttlPositive{
+        this,
+        30 * 24 * 3600,
+        "narinfo-cache-positive-ttl",
+        R"(
+          The TTL in seconds for positive lookups. If a store path is queried
+          from a substituter, the result of the query is cached in the
+          local disk cache database including some of the NAR metadata. The
+          default TTL is a month, setting a shorter TTL for positive lookups
+          can be useful for binary caches that have frequent garbage
+          collection, in which case having a more frequent cache invalidation
+          would prevent trying to pull the path again and failing with a hash
+          mismatch if the build isn't reproducible.
+        )"};
+};
+
+class Settings : public virtual Config,
+                 private LocalSettings,
+                 private LogFileSettings,
+                 private WorkerSettings,
+                 private NarInfoDiskCacheSettings
 {
     StringSet getDefaultSystemFeatures();
 
@@ -103,6 +143,19 @@ public:
         return *this;
     }
 
+    /**
+     * Get the NAR info disk cache settings.
+     */
+    NarInfoDiskCacheSettings & getNarInfoDiskCacheSettings()
+    {
+        return *this;
+    }
+
+    const NarInfoDiskCacheSettings & getNarInfoDiskCacheSettings() const
+    {
+        return *this;
+    }
+
     static unsigned int getDefaultCores();
 
     /**
@@ -311,39 +364,6 @@ public:
         )",
         {"trusted-binary-caches"}};
 
-    Setting<unsigned int> ttlNegativeNarInfoCache{
-        this,
-        3600,
-        "narinfo-cache-negative-ttl",
-        R"(
-          The TTL in seconds for negative lookups.
-          If a store path is queried from a [substituter](#conf-substituters) but was not found, a negative lookup is cached in the local disk cache database for the specified duration.
-
-          Set to `0` to force updating the lookup cache.
-
-          To wipe the lookup cache completely:
-
-          ```shell-session
-          $ rm $HOME/.cache/nix/binary-cache-v*.sqlite*
-          # rm /root/.cache/nix/binary-cache-v*.sqlite*
-          ```
-        )"};
-
-    Setting<unsigned int> ttlPositiveNarInfoCache{
-        this,
-        30 * 24 * 3600,
-        "narinfo-cache-positive-ttl",
-        R"(
-          The TTL in seconds for positive lookups. If a store path is queried
-          from a substituter, the result of the query is cached in the
-          local disk cache database including some of the NAR metadata. The
-          default TTL is a month, setting a shorter TTL for positive lookups
-          can be useful for binary caches that have frequent garbage
-          collection, in which case having a more frequent cache invalidation
-          would prevent trying to pull the path again and failing with a hash
-          mismatch if the build isn't reproducible.
-        )"};
-
     // move it out in the 2nd pass
     Setting<bool> printMissing{
         this, true, "print-missing", "Whether to print what paths need to be built or downloaded."};

src/libstore/include/nix/store/nar-info-disk-cache.hh

@@ -7,9 +7,20 @@
 
 namespace nix {
 
-class NarInfoDiskCache
+struct SQLiteSettings;
+struct NarInfoDiskCacheSettings;
+
+struct NarInfoDiskCache
 {
-public:
+    using Settings = NarInfoDiskCacheSettings;
+
+    const Settings & settings;
+
+    NarInfoDiskCache(const Settings & settings)
+        : settings(settings)
+    {
+    }
+
     typedef enum { oValid, oInvalid, oUnknown } Outcome;
 
     virtual ~NarInfoDiskCache() {}
@@ -35,14 +46,20 @@ public:
     virtual void upsertAbsentRealisation(const std::string & uri, const DrvOutput & id) = 0;
     virtual std::pair<Outcome, std::shared_ptr<Realisation>>
     lookupRealisation(const std::string & uri, const DrvOutput & id) = 0;
-};
 
-/**
- * Return a singleton cache object that can be used concurrently by
- * multiple threads.
- */
-ref<NarInfoDiskCache> getNarInfoDiskCache();
+    /**
+     * Return a singleton cache object that can be used concurrently by
+     * multiple threads.
+     *
+     * @note the parameters are only used to initialize this the first time this is called.
+     * In subsequent calls, these arguments are ignored.
+     *
+     * @todo Probably should instead create a memo table so multiple settings -> multiple instances,
+     * but this is not yet a problem in practice.
+     */
+    static ref<NarInfoDiskCache> get(const Settings & settings, SQLiteSettings);
 
-ref<NarInfoDiskCache> getTestNarInfoDiskCache(Path dbPath);
+    static ref<NarInfoDiskCache> getTest(const Settings & settings, SQLiteSettings, Path dbPath);
+};
 
 } // namespace nix

src/libstore/include/nix/store/sqlite.hh

@@ -33,6 +33,12 @@ enum class SQLiteOpenMode {
     Immutable,
 };
 
+struct SQLiteSettings
+{
+    SQLiteOpenMode mode = SQLiteOpenMode::Normal;
+    bool useWAL;
+};
+
 /**
  * RAII wrapper to close a SQLite database automatically.
  */
@@ -42,11 +48,7 @@ struct SQLite
 
     SQLite() {}
 
-    struct Settings
-    {
-        SQLiteOpenMode mode = SQLiteOpenMode::Normal;
-        bool useWAL;
-    };
+    using Settings = SQLiteSettings;
 
     SQLite(const std::filesystem::path & path, Settings && settings);
     SQLite(const SQLite & from) = delete;

src/libstore/include/nix/store/store-api.hh

@@ -40,7 +40,8 @@ struct BasicDerivation;
 struct Derivation;
 
 struct SourceAccessor;
-class NarInfoDiskCache;
+struct NarInfoDiskCache;
+struct NarInfoDiskCacheSettings;
 class Store;
 
 typedef std::map<std::string, StorePath> OutputPathMap;
@@ -300,7 +301,7 @@ protected:
          * Whether the value is valid as a cache entry. The path may not
          * exist.
          */
-        bool isKnownNow();
+        bool isKnownNow(const NarInfoDiskCacheSettings & settings);
 
         /**
          * Past tense, because a path can only be assumed to exists when

src/libstore/nar-info-disk-cache.cc

@@ -58,10 +58,8 @@ create table if not exists LastPurge (
 
 )sql";
 
-class NarInfoDiskCacheImpl : public NarInfoDiskCache
+struct NarInfoDiskCacheImpl : NarInfoDiskCache
 {
-public:
-
     /* How often to purge expired entries from the cache. */
     const int purgeInterval = 24 * 3600;
 
@@ -86,13 +84,17 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
 
     Sync<State> _state;
 
-    NarInfoDiskCacheImpl(Path dbPath = (getCacheDir() / "binary-cache-v7.sqlite").string())
+    NarInfoDiskCacheImpl(
+        const Settings & settings,
+        SQLiteSettings sqliteSettings,
+        Path dbPath = (getCacheDir() / "binary-cache-v7.sqlite").string())
+        : NarInfoDiskCache{settings}
     {
         auto state(_state.lock());
 
         createDirs(dirOf(dbPath));
 
-        state->db = SQLite(dbPath, {.useWAL = settings.useSQLiteWAL});
+        state->db = SQLite(dbPath, SQLite::Settings{sqliteSettings});
 
         state->db.isCache();
 
@@ -155,8 +157,8 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
                     .use()
                     // Use a minimum TTL to prevent --refresh from
                     // nuking the entire disk cache.
-                    (now - std::max(settings.ttlNegativeNarInfoCache.get(), 3600U))(
-                        now - std::max(settings.ttlPositiveNarInfoCache.get(), 30 * 24 * 3600U))
+                    (now - std::max(settings.ttlNegative.get(), 3600U))(
+                        now - std::max(settings.ttlPositive.get(), 30 * 24 * 3600U))
                     .exec();
 
                 debug("deleted %d entries from the NAR info disk cache", sqlite3_changes(state->db));
@@ -254,8 +256,8 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
 
                 auto now = time(0);
 
-                auto queryNAR(state->queryNAR.use()(cache.id)(hashPart) (now - settings.ttlNegativeNarInfoCache)(
-                    now - settings.ttlPositiveNarInfoCache));
+                auto queryNAR(
+                    state->queryNAR.use()(cache.id)(hashPart) (now - settings.ttlNegative)(now - settings.ttlPositive));
 
                 if (!queryNAR.next())
                     return {oUnknown, 0};
@@ -296,7 +298,7 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
                 auto now = time(0);
 
                 auto queryRealisation(state->queryRealisation.use()(cache.id)(id.to_string())(
-                    now - settings.ttlNegativeNarInfoCache)(now - settings.ttlPositiveNarInfoCache));
+                    now - settings.ttlNegative)(now - settings.ttlPositive));
 
                 if (!queryRealisation.next())
                     return {oUnknown, 0};
@@ -372,15 +374,15 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
     }
 };
 
-ref<NarInfoDiskCache> getNarInfoDiskCache()
+ref<NarInfoDiskCache> NarInfoDiskCache::get(const Settings & settings, SQLiteSettings sqliteSettings)
 {
-    static ref<NarInfoDiskCache> cache = make_ref<NarInfoDiskCacheImpl>();
+    static ref<NarInfoDiskCache> cache = make_ref<NarInfoDiskCacheImpl>(settings, sqliteSettings);
     return cache;
 }
 
-ref<NarInfoDiskCache> getTestNarInfoDiskCache(Path dbPath)
+ref<NarInfoDiskCache> NarInfoDiskCache::getTest(const Settings & settings, SQLiteSettings sqliteSettings, Path dbPath)
 {
-    return make_ref<NarInfoDiskCacheImpl>(dbPath);
+    return make_ref<NarInfoDiskCacheImpl>(settings, sqliteSettings, dbPath);
 }
 
 } // namespace nix

src/libstore/store-api.cc

@@ -338,10 +338,10 @@ bool StoreConfig::getReadOnly() const
     return settings.readOnlyMode;
 }
 
-bool Store::PathInfoCacheValue::isKnownNow()
+bool Store::PathInfoCacheValue::isKnownNow(const NarInfoDiskCacheSettings & settings)
 {
-    std::chrono::duration ttl = didExist() ? std::chrono::seconds(settings.ttlPositiveNarInfoCache)
-                                           : std::chrono::seconds(settings.ttlNegativeNarInfoCache);
+    std::chrono::duration ttl =
+        didExist() ? std::chrono::seconds(settings.ttlPositive) : std::chrono::seconds(settings.ttlNegative);
 
     return std::chrono::steady_clock::now() < time_point + ttl;
 }
@@ -513,7 +513,7 @@ StorePathSet Store::querySubstitutablePaths(const StorePathSet & paths)
 bool Store::isValidPath(const StorePath & storePath)
 {
     auto res = pathInfoCache->lock()->get(storePath);
-    if (res && res->isKnownNow()) {
+    if (res && res->isKnownNow(settings.getNarInfoDiskCacheSettings())) {
         stats.narInfoReadAverted++;
         return res->didExist();
     }
@@ -579,7 +579,7 @@ std::optional<std::shared_ptr<const ValidPathInfo>> Store::queryPathInfoFromClie
     auto hashPart = std::string(storePath.hashPart());
 
     auto res = pathInfoCache->lock()->get(storePath);
-    if (res && res->isKnownNow()) {
+    if (res && res->isKnownNow(settings.getNarInfoDiskCacheSettings())) {
         stats.narInfoReadAverted++;
         if (res->didExist())
             return std::make_optional(res->value);

src/nix/main.cc

@@ -566,8 +566,8 @@ void mainWrapped(int argc, char ** argv)
 
     if (args.refresh) {
         fetchSettings.tarballTtl = 0;
-        settings.ttlNegativeNarInfoCache = 0;
-        settings.ttlPositiveNarInfoCache = 0;
+        settings.getNarInfoDiskCacheSettings().ttlNegative = 0;
+        settings.getNarInfoDiskCacheSettings().ttlPositive = 0;
     }
 
     if (args.command->second->forceImpureByDefault() && !evalSettings.pureEval.overridden) {