refactor: Improve Storage::get API to be able to return multiple times

diff --git a/src/ccache.cpp b/src/ccache.cpp
index 8197c1c975207008723bed16119642578ca00b1d..ec46ca81307f5b43afad8636e4003e93fcb93e34 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -1711,47 +1711,24 @@ hash_direct_mode_specific_data(Context& ctx,
 
   manifest_key = hash.digest();
 
-  auto cache_entry_data = ctx.storage.get(*manifest_key,
-                                          core::CacheEntryType::manifest,
-                                          storage::Storage::Mode::primary_only);
-
-  if (cache_entry_data) {
-    MTR_BEGIN("manifest", "manifest_get");
-    try {
-      read_manifest(ctx, *cache_entry_data);
-      result_key = ctx.manifest.look_up_result_digest(ctx);
-    } catch (const core::Error& e) {
-      LOG("Failed to look up result key in manifest: {}", e.what());
-    }
-    MTR_END("manifest", "manifest_get");
-    if (result_key) {
-      LOG_RAW("Got result key from manifest");
-    } else {
-      LOG_RAW("Did not find result key in manifest");
-    }
-  }
-  // Check secondary storage if not found in primary
-  if (!result_key) {
-    cache_entry_data = ctx.storage.get(*manifest_key,
-                                       core::CacheEntryType::manifest,
-                                       storage::Storage::Mode::secondary_only);
-    if (cache_entry_data) {
-      LOG_RAW("Looking for result key in fetched secondary manifest");
-      MTR_BEGIN("manifest", "secondary_manifest_get");
+  MTR_BEGIN("manifest", "manifest_get");
+  ctx.storage.get(
+    *manifest_key, core::CacheEntryType::manifest, [&](util::Bytes&& value) {
       try {
-        read_manifest(ctx, *cache_entry_data);
+        read_manifest(ctx, value);
         result_key = ctx.manifest.look_up_result_digest(ctx);
       } catch (const core::Error& e) {
         LOG("Failed to look up result key in manifest: {}", e.what());
       }
-      MTR_END("manifest", "secondary_manifest_get");
       if (result_key) {
-        LOG_RAW("Got result key from fetched secondary manifest");
+        LOG_RAW("Got result key from manifest");
+        return true;
       } else {
-        LOG_RAW("Did not find result key in fetched secondary manifest");
+        LOG_RAW("Did not find result key in manifest");
+        return false;
       }
-    }
-  }
+    });
+  MTR_END("manifest", "manifest_get");
 
   return {};
 }
@@ -1950,14 +1927,18 @@ from_cache(Context& ctx, FromCacheCallMode mode, const Digest& result_key)
   MTR_SCOPE("cache", "from_cache");
 
   // Get result from cache.
-  const auto cache_entry_data =
-    ctx.storage.get(result_key, core::CacheEntryType::result);
-  if (!cache_entry_data) {
+  util::Bytes cache_entry_data;
+  ctx.storage.get(
+    result_key, core::CacheEntryType::result, [&](util::Bytes&& value) {
+      cache_entry_data = std::move(value);
+      return true;
+    });
+  if (cache_entry_data.empty()) {
     return false;
   }
 
   try {
-    core::CacheEntry cache_entry(*cache_entry_data);
+    core::CacheEntry cache_entry(cache_entry_data);
     cache_entry.verify_checksum();
     core::Result::Deserializer deserializer(cache_entry.payload());
     core::ResultRetriever result_retriever(ctx, result_key);

diff --git a/src/storage/Storage.cpp b/src/storage/Storage.cpp
index c468a2e91b15b2cabcbada6b45ac6466263bcd7f..9e617fb3787816e7a1d64bfccbb841ba442b8e24 100644 (file)
--- a/src/storage/Storage.cpp
+++ b/src/storage/Storage.cpp
@@ -230,34 +230,29 @@ Storage::finalize()
   primary.finalize();
 }
 
-std::optional<util::Bytes>
+void
 Storage::get(const Digest& key,
              const core::CacheEntryType type,
-             const Mode mode)
+             const EntryReceiver& entry_receiver)
 {
   MTR_SCOPE("storage", "get");
 
-  if (mode != Mode::secondary_only) {
-    auto value = primary.get(key, type);
-    primary.increment_statistic(value ? core::Statistic::primary_storage_hit
-                                      : core::Statistic::primary_storage_miss);
-    if (value) {
-      if (m_config.reshare()) {
-        put_in_secondary_storage(key, *value, true);
-      }
-      return value;
+  auto value = primary.get(key, type);
+  primary.increment_statistic(value ? core::Statistic::primary_storage_hit
+                                    : core::Statistic::primary_storage_miss);
+  if (value) {
+    if (m_config.reshare()) {
+      put_in_secondary_storage(key, *value, true);
+    }
+    if (entry_receiver(std::move(*value))) {
+      return;
     }
   }
 
-  if (mode == Mode::primary_only) {
-    return std::nullopt;
-  }
-
-  auto value = get_from_secondary_storage(key);
-  if (value) {
-    primary.put(key, type, *value);
-  }
-  return value;
+  get_from_secondary_storage(key, [&](util::Bytes&& data) {
+    primary.put(key, type, data, true);
+    return entry_receiver(std::move(data));
+  });
 }
 
 void
@@ -420,8 +415,9 @@ Storage::get_backend(SecondaryStorageEntry& entry,
   }
 }
 
-std::optional<util::Bytes>
-Storage::get_from_secondary_storage(const Digest& key)
+void
+Storage::get_from_secondary_storage(const Digest& key,
+                                    const EntryReceiver& entry_receiver)
 {
   MTR_SCOPE("secondary_storage", "get");
 
@@ -432,21 +428,23 @@ Storage::get_from_secondary_storage(const Digest& key)
     }
 
     Timer timer;
-    const auto result = backend->impl->get(key);
+    auto result = backend->impl->get(key);
     const auto ms = timer.measure_ms();
     if (!result) {
       mark_backend_as_failed(*backend, result.error());
       continue;
     }
 
-    const auto& value = *result;
+    auto& value = *result;
     if (value) {
       LOG("Retrieved {} from {} ({:.2f} ms)",
           key.to_string(),
           backend->url_for_logging,
           ms);
       primary.increment_statistic(core::Statistic::secondary_storage_hit);
-      return *value;
+      if (entry_receiver(std::move(*value))) {
+        return;
+      }
     } else {
       LOG("No {} in {} ({:.2f} ms)",
           key.to_string(),
@@ -455,8 +453,6 @@ Storage::get_from_secondary_storage(const Digest& key)
       primary.increment_statistic(core::Statistic::secondary_storage_miss);
     }
   }
-
-  return std::nullopt;
 }
 
 void

diff --git a/src/storage/Storage.hpp b/src/storage/Storage.hpp
index eba46897dbe93a38f7cd1c1f030b76a469b9c0f5..152c8349bb4792c74c507df0757e02c0b7120008 100644 (file)
--- a/src/storage/Storage.hpp
+++ b/src/storage/Storage.hpp
@@ -26,6 +26,7 @@
 
 #include <third_party/nonstd/span.hpp>
 
+#include <functional>
 #include <memory>
 #include <optional>
 #include <string>
@@ -51,10 +52,11 @@ public:
 
   primary::PrimaryStorage primary;
 
-  enum class Mode { secondary_fallback, secondary_only, primary_only };
-  std::optional<util::Bytes> get(const Digest& key,
-                                 core::CacheEntryType type,
-                                 const Mode mode = Mode::secondary_fallback);
+  using EntryReceiver = std::function<bool(util::Bytes&&)>;
+
+  void get(const Digest& key,
+           core::CacheEntryType type,
+           const EntryReceiver& entry_receiver);
 
   void put(const Digest& key,
            core::CacheEntryType type,
@@ -81,7 +83,8 @@ private:
               std::string_view operation_description,
               const bool for_writing);
 
-  std::optional<util::Bytes> get_from_secondary_storage(const Digest& key);
+  void get_from_secondary_storage(const Digest& key,
+                                  const EntryReceiver& entry_receiver);
 
   void put_in_secondary_storage(const Digest& key,
                                 nonstd::span<const uint8_t> value,

diff --git a/src/storage/primary/PrimaryStorage.cpp b/src/storage/primary/PrimaryStorage.cpp
index 8a520d3e7277de59d9870521238985ff31d9f954..c9e00ee80177ed920f77d8f8f0b257f2e724f01c 100644 (file)
--- a/src/storage/primary/PrimaryStorage.cpp
+++ b/src/storage/primary/PrimaryStorage.cpp
@@ -204,11 +204,17 @@ PrimaryStorage::get(const Digest& key, const core::CacheEntryType type) const
 void
 PrimaryStorage::put(const Digest& key,
                     const core::CacheEntryType type,
-                    nonstd::span<const uint8_t> value)
+                    nonstd::span<const uint8_t> value,
+                    bool only_if_missing)
 {
   MTR_SCOPE("primary_storage", "put");
 
   const auto cache_file = look_up_cache_file(key, type);
+  if (only_if_missing && cache_file.stat) {
+    LOG("Not storing {} since it already exists", cache_file.path);
+    return;
+  }
+
   switch (type) {
   case core::CacheEntryType::manifest:
     m_manifest_key = key;

diff --git a/src/storage/primary/PrimaryStorage.hpp b/src/storage/primary/PrimaryStorage.hpp
index 58ed02addce19c0c75303879efa8109e81dc91eb..2789a141d28c69e21d3bb92ccd12cbb2505c2a43 100644 (file)
--- a/src/storage/primary/PrimaryStorage.hpp
+++ b/src/storage/primary/PrimaryStorage.hpp
@@ -59,7 +59,8 @@ public:
 
   void put(const Digest& key,
            core::CacheEntryType type,
-           nonstd::span<const uint8_t> value);
+           nonstd::span<const uint8_t> value,
+           bool only_if_missing = false);
 
   void remove(const Digest& key, core::CacheEntryType type);