Merge pull request #3403 from hercules-ci/issue-3398-path-info-cache-ttls

pathInfoCache: Respect disk cache TTLs #3398

6 files changed, 43 insertions, 15 deletions

diff --git a/README.md b/README.md
index 48cb1685c..a953c0f71 100644
--- a/README.md
+++ b/README.md
@@ -9,11 +9,11 @@ appear with Nix.
 
 To find out more about the tool, usage and installation instructions, please
 read the manual, which is available on the Nix website at
-<http://nixos.org/nix/manual>.
+<https://nixos.org/nix/manual>.
 
 ## Contributing
 
-Take a look at the [Hacking Section](http://nixos.org/nix/manual/#chap-hacking)
+Take a look at the [Hacking Section](https://nixos.org/nix/manual/#chap-hacking)
 of the manual. It helps you to get started with building Nix from source.
 
 ## License
@@ -21,4 +21,4 @@ of the manual. It helps you to get started with building Nix from source.
 Nix is released under the LGPL v2.1
 
 This product includes software developed by the OpenSSL Project for
-use in the [OpenSSL Toolkit](http://www.OpenSSL.org/).
+use in the [OpenSSL Toolkit](https://www.OpenSSL.org/).
diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh
index 3fea7e056..e00708f6c 100644
--- a/scripts/install-nix-from-closure.sh
+++ b/scripts/install-nix-from-closure.sh
@@ -87,7 +87,7 @@ if ! [ -e $dest ]; then
 fi
 
 if ! [ -w $dest ]; then
-    echo "$0: directory $dest exists, but is not writable by you. This could indicate that another user has already performed a single-user installation of Nix on this system. If you wish to enable multi-user support see http://nixos.org/nix/manual/#ssec-multi-user. If you wish to continue with a single-user install for $USER please run 'chown -R $USER $dest' as root." >&2
+    echo "$0: directory $dest exists, but is not writable by you. This could indicate that another user has already performed a single-user installation of Nix on this system. If you wish to enable multi-user support see https://nixos.org/nix/manual/#ssec-multi-user. If you wish to continue with a single-user install for $USER please run 'chown -R $USER $dest' as root." >&2
     exit 1
 fi
 
diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
index 717faec92..3a2d84861 100644
--- a/src/libstore/binary-cache-store.cc
+++ b/src/libstore/binary-cache-store.cc
@@ -106,7 +106,7 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo)
 
     {
         auto state_(state.lock());
-        state_->pathInfoCache.upsert(hashPart, std::shared_ptr<NarInfo>(narInfo));
+        state_->pathInfoCache.upsert(hashPart, PathInfoCacheValue { .value = std::shared_ptr<NarInfo>(narInfo) });
     }
 
     if (diskCache)
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index e59624cd3..c5929a41c 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -622,7 +622,8 @@ uint64_t LocalStore::addValidPath(State & state,
 
     {
         auto state_(Store::state.lock());
-        state_->pathInfoCache.upsert(storePathToHash(printStorePath(info.path)), std::make_shared<ValidPathInfo>(info));
+        state_->pathInfoCache.upsert(storePathToHash(printStorePath(info.path)),
+            PathInfoCacheValue{ .value = std::make_shared<const ValidPathInfo>(info) });
     }
 
     return id;
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 75fa5d1e6..b043feb0a 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -226,6 +226,14 @@ std::string Store::getUri()
     return "";
 }
 
+bool Store::PathInfoCacheValue::isKnownNow()
+{
+    std::chrono::duration ttl = didExist()
+        ? std::chrono::seconds(settings.ttlPositiveNarInfoCache)
+        : std::chrono::seconds(settings.ttlNegativeNarInfoCache);
+
+    return std::chrono::steady_clock::now() < time_point + ttl;
+}
 
 bool Store::isValidPath(const StorePath & storePath)
 {
@@ -234,9 +242,9 @@ bool Store::isValidPath(const StorePath & storePath)
     {
         auto state_(state.lock());
         auto res = state_->pathInfoCache.get(hashPart);
-        if (res) {
+        if (res && res->isKnownNow()) {
             stats.narInfoReadAverted++;
-            return *res != 0;
+            return res->didExist();
         }
     }
 
@@ -246,7 +254,7 @@ bool Store::isValidPath(const StorePath & storePath)
             stats.narInfoReadAverted++;
             auto state_(state.lock());
             state_->pathInfoCache.upsert(hashPart,
-                res.first == NarInfoDiskCache::oInvalid ? 0 : res.second);
+                res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue { .value = res.second });
             return res.first == NarInfoDiskCache::oValid;
         }
     }
@@ -301,11 +309,11 @@ void Store::queryPathInfo(const StorePath & storePath,
 
         {
             auto res = state.lock()->pathInfoCache.get(hashPart);
-            if (res) {
+            if (res && res->isKnownNow()) {
                 stats.narInfoReadAverted++;
-                if (!*res)
+                if (!res->didExist())
                     throw InvalidPath("path '%s' is not valid", printStorePath(storePath));
-                return callback(ref<const ValidPathInfo>(*res));
+                return callback(ref<const ValidPathInfo>(res->value));
             }
         }
 
@@ -316,7 +324,7 @@ void Store::queryPathInfo(const StorePath & storePath,
                 {
                     auto state_(state.lock());
                     state_->pathInfoCache.upsert(hashPart,
-                        res.first == NarInfoDiskCache::oInvalid ? 0 : res.second);
+                        res.first == NarInfoDiskCache::oInvalid ? PathInfoCacheValue{} : PathInfoCacheValue{ .value = res.second });
                     if (res.first == NarInfoDiskCache::oInvalid ||
                         res.second->path != storePath)
                         throw InvalidPath("path '%s' is not valid", printStorePath(storePath));
@@ -340,7 +348,7 @@ void Store::queryPathInfo(const StorePath & storePath,
 
                 {
                     auto state_(state.lock());
-                    state_->pathInfoCache.upsert(hashPart, info);
+                    state_->pathInfoCache.upsert(hashPart, PathInfoCacheValue { .value = info });
                 }
 
                 if (!info || info->path != parseStorePath(storePath)) {
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index a000757fb..e0484de13 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -16,6 +16,7 @@
 #include <unordered_set>
 #include <memory>
 #include <string>
+#include <chrono>
 
 
 namespace nix {
@@ -261,10 +262,28 @@ public:
 
 protected:
 
+    struct PathInfoCacheValue {
+
+        // Time of cache entry creation or update
+        std::chrono::time_point<std::chrono::steady_clock> time_point = std::chrono::steady_clock::now();
+
+        // Null if missing
+        std::shared_ptr<const ValidPathInfo> value;
+
+        // Whether the value is valid as a cache entry. The path may not exist.
+        bool isKnownNow();
+
+        // Past tense, because a path can only be assumed to exists when
+        // isKnownNow() && didExist()
+        inline bool didExist() {
+          return value != nullptr;
+        }
+    };
+
     struct State
     {
         // FIXME: fix key
-        LRUCache<std::string, std::shared_ptr<const ValidPathInfo>> pathInfoCache;
+        LRUCache<std::string, PathInfoCacheValue> pathInfoCache;
     };
 
     Sync<State> state;