From 3f55f8a8fb5e885297e288c078adc173316addf2 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Wed, 11 Mar 2020 20:04:47 +0100
Subject: [PATCH] pathInfoCache: Respect disk cache TTLs #3398

---
 src/libstore/binary-cache-store.cc |  2 +-
 src/libstore/local-store.cc        |  3 ++-
 src/libstore/store-api.cc          | 24 ++++++++++++++++--------
 src/libstore/store-api.hh          | 21 ++++++++++++++++++++-
 4 files changed, 39 insertions(+), 11 deletions(-)

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;