From 6a8de4c9dcd5c329f6488818517995647c577c87 Mon Sep 17 00:00:00 2001
From: Ben Radford <benradf@users.noreply.github.com>
Date: Fri, 28 Jul 2023 10:59:16 +0100
Subject: [PATCH] Avoid enumerating entire overlay store dir upfront.

As an optimisation for LocalStore, we read all the store directory entries into
a set. Checking for membership of this set is much faster than a stat syscall.
However for LocalOverlayStore, the lower store directory is expected to contain
a vast number of entries and reading them all can take a very long time.

So instead of enumerating them all upfront, we call pathExists as needed. This
means making stat syscalls for each store path, but the upper layer is expected
to be relatively small compared to the lower store so that should be okay.
---
 src/libstore/local-overlay-store.cc       | 16 ++++++++++++++++
 src/libstore/local-overlay-store.hh       |  2 ++
 src/libstore/local-store.hh               |  8 +++++---
 tests/overlay-local-store/verify-inner.sh |  3 +++
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc
index 30008de67..4bfad6d32 100644
--- a/src/libstore/local-overlay-store.cc
+++ b/src/libstore/local-overlay-store.cc
@@ -243,6 +243,22 @@ void LocalOverlayStore::optimiseStore()
 }
 
 
+bool LocalOverlayStore::verifyAllValidPaths(RepairFlag repair, StorePathSet & validPaths)
+{
+    StorePathSet done;
+    bool errors = false;
+
+    auto existsInStoreDir = [&](const StorePath & storePath) {
+        return pathExists(realStoreDir.get() + "/" + storePath.to_string());
+    };
+
+    for (auto & i : queryAllValidPaths())
+        verifyPath(i, existsInStoreDir, done, validPaths, repair, errors);
+
+    return errors;
+}
+
+
 Path LocalOverlayStore::toRealPathForHardLink(const StorePath & path)
 {
     return lowerStore->isValidPath(path)
diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh
index 1c1d54a8f..21af396a6 100644
--- a/src/libstore/local-overlay-store.hh
+++ b/src/libstore/local-overlay-store.hh
@@ -124,6 +124,8 @@ private:
 
     void optimiseStore() override;
 
+    bool verifyAllValidPaths(RepairFlag repair, StorePathSet & validPaths) override;
+
     /**
      * For lower-store paths, we used the lower store location. This avoids the
      * wasteful "copying up" that would otherwise happen.
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index f9db43517..322d27932 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -312,6 +312,11 @@ public:
 
     std::optional<std::string> getVersion() override;
 
+protected:
+
+    void verifyPath(const StorePath & path, std::function<bool(const StorePath &)> existsInStoreDir,
+        StorePathSet & done, StorePathSet & validPaths, RepairFlag repair, bool & errors);
+
 private:
 
     /**
@@ -335,9 +340,6 @@ private:
      */
     void invalidatePathChecked(const StorePath & path);
 
-    void verifyPath(const StorePath & path, std::function<bool(const StorePath &)> existsInStoreDir,
-        StorePathSet & done, StorePathSet & validPaths, RepairFlag repair, bool & errors);
-
     std::shared_ptr<const ValidPathInfo> queryPathInfoInternal(State & state, const StorePath & path);
 
     void updatePathInfo(State & state, const ValidPathInfo & info);
diff --git a/tests/overlay-local-store/verify-inner.sh b/tests/overlay-local-store/verify-inner.sh
index de40c3d05..64d3b63c1 100755
--- a/tests/overlay-local-store/verify-inner.sh
+++ b/tests/overlay-local-store/verify-inner.sh
@@ -50,6 +50,9 @@ find "$storeA" -name "*-dummy" -exec truncate -s 0 {} \;
 # Also truncate the file that only exists in lower store
 truncate -s 0 "$storeA/$lowerOnlyPath"
 
+# Ensure overlayfs is synchronised
+remountOverlayfs
+
 
 ## Now test that verify and repair work as expected