From d549250069ea2a01e0e16aa14df7565450cf4f05 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        |  2 +-
 src/libstore/store-api.cc          | 23 ++++++++++++++-------
 src/libstore/store-api.hh          | 33 +++++++++++++++++++++++++++++-
 4 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
index 10cde8704..3118b8ca5 100644
--- a/src/libstore/binary-cache-store.cc
+++ b/src/libstore/binary-cache-store.cc
@@ -104,7 +104,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(std::shared_ptr<NarInfo>(narInfo)));
     }
 
     if (diskCache)
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 84ddd964b..f5092151a 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -623,7 +623,7 @@ uint64_t LocalStore::addValidPath(State & state,
 
     {
         auto state_(Store::state.lock());
-        state_->pathInfoCache.upsert(storePathToHash(info.path), std::make_shared<ValidPathInfo>(info));
+        state_->pathInfoCache.upsert(storePathToHash(info.path), PathInfoCacheValue(info));
     }
 
     return id;
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 5f63c53b5..8cb9f8dee 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -261,6 +261,15 @@ 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 Path & storePath)
 {
@@ -271,9 +280,9 @@ bool Store::isValidPath(const Path & 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();
         }
     }
 
@@ -283,7 +292,7 @@ bool Store::isValidPath(const Path & storePath)
             stats.narInfoReadAverted++;
             auto state_(state.lock());
             state_->pathInfoCache.upsert(hashPart,
-                res.first == NarInfoDiskCache::oInvalid ? 0 : res.second);
+                res.first == NarInfoDiskCache::oInvalid ? 0 : PathInfoCacheValue(res.second));
             return res.first == NarInfoDiskCache::oValid;
         }
     }
@@ -340,11 +349,11 @@ void Store::queryPathInfo(const Path & storePath,
 
         {
             auto res = state.lock()->pathInfoCache.get(hashPart);
-            if (res) {
+            if (res && res->isKnownNow()) {
                 stats.narInfoReadAverted++;
-                if (!*res)
+                if (!res->didExist())
                     throw InvalidPath(format("path '%s' is not valid") % storePath);
-                return callback(ref<ValidPathInfo>(*res));
+                return callback(ref<ValidPathInfo>(res->value));
             }
         }
 
@@ -355,7 +364,7 @@ void Store::queryPathInfo(const Path & storePath,
                 {
                     auto state_(state.lock());
                     state_->pathInfoCache.upsert(hashPart,
-                        res.first == NarInfoDiskCache::oInvalid ? 0 : res.second);
+                        res.first == NarInfoDiskCache::oInvalid ? 0 : PathInfoCacheValue(res.second));
                     if (res.first == NarInfoDiskCache::oInvalid ||
                         (res.second->path != storePath && storePathToName(storePath) != ""))
                         throw InvalidPath(format("path '%s' is not valid") % storePath);
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index a751b4662..ace769ea6 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -15,6 +15,7 @@
 #include <unordered_set>
 #include <memory>
 #include <string>
+#include <chrono>
 
 
 namespace nix {
@@ -256,9 +257,39 @@ public:
 
 protected:
 
+    struct PathInfoCacheValue {
+
+        PathInfoCacheValue(std::shared_ptr<ValidPathInfo> value) :
+            time_point(std::chrono::steady_clock::now()),
+            value(value) {}
+
+        // Convenience constructor copying from reference
+        PathInfoCacheValue(const ValidPathInfo& value) :
+            PathInfoCacheValue(std::make_shared<ValidPathInfo>(value)) {}
+
+        // Record a missing path
+        PathInfoCacheValue(std::nullptr_t nil) :
+            PathInfoCacheValue(std::shared_ptr<ValidPathInfo>()) {}
+
+        // Time of cache entry creation or update(?)
+        std::chrono::time_point<std::chrono::steady_clock> time_point;
+
+        // Null if missing
+        std::shared_ptr<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 != 0;
+        }
+    };
+
     struct State
     {
-        LRUCache<std::string, std::shared_ptr<ValidPathInfo>> pathInfoCache;
+        LRUCache<std::string, PathInfoCacheValue> pathInfoCache;
     };
 
     Sync<State> state;