feat: Add share-hits attribute for secondary storage

With share-hits=false, a secondary storage backend will not share hits
to primary storage.

diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc
index 6b68f8f369484fbd2b78add6fc5faa2db634d56a..e62194fd94f9d255bf39c64176252fa23ba00e25 100644 (file)
--- a/doc/MANUAL.adoc
+++ b/doc/MANUAL.adoc
@@ -961,6 +961,8 @@ Examples:
 * `+http://example.com/*|shards=alpha,beta+` will put 50% of the cache on
   `+http://example.com/alpha+` and 50% on `+http://example.com/beta+`.
 --
+* *share-hits*: If *true*, write hits for this backend to primary storage. The
+  default is *true*.
 
 These are the available backends:
 

diff --git a/src/storage/Storage.cpp b/src/storage/Storage.cpp
index d1f0d5c4d8f639e3860bce7ef1a1b77c31c38708..85c0a6da937fb00de2825a1226f27a65ce03f72c 100644 (file)
--- a/src/storage/Storage.cpp
+++ b/src/storage/Storage.cpp
@@ -80,6 +80,7 @@ struct SecondaryStorageConfig
   std::vector<SecondaryStorageShardConfig> shards;
   secondary::SecondaryStorage::Backend::Params params;
   bool read_only = false;
+  bool share_hits = true;
 };
 
 struct SecondaryStorageBackendEntry
@@ -140,8 +141,8 @@ parse_storage_config(const nonstd::string_view entry)
     const auto& raw_value = kv_pair.second.value_or("true");
     const auto value =
       util::value_or_throw<core::Error>(util::percent_decode(raw_value));
-    if (key == "read-only" && value == "true") {
-      result.read_only = true;
+    if (key == "read-only") {
+      result.read_only = (value == "true");
     } else if (key == "shards") {
       const auto url_str = result.params.url.str();
       if (url_str.find('*') == std::string::npos) {
@@ -169,6 +170,8 @@ parse_storage_config(const nonstd::string_view entry)
 
         result.shards.push_back({std::string(name), weight});
       }
+    } else if (key == "share-hits") {
+      result.share_hits = (value == "true");
     }
 
     result.params.attributes.push_back(
@@ -252,28 +255,32 @@ Storage::get(const Digest& key, const core::CacheEntryType type)
     return path;
   }
 
-  const auto value = get_from_secondary_storage(key);
-  if (!value) {
+  const auto value_and_share_hits = get_from_secondary_storage(key);
+  if (!value_and_share_hits) {
     return nonstd::nullopt;
   }
+  const auto& value = value_and_share_hits->first;
+  const auto& share_hits = value_and_share_hits->second;
 
   TemporaryFile tmp_file(FMT("{}/tmp.get", m_config.temporary_dir()));
   m_tmp_files.push_back(tmp_file.path);
   try {
-    Util::write_file(tmp_file.path, *value);
+    Util::write_file(tmp_file.path, value);
   } catch (const core::Error& e) {
     throw core::Fatal("Error writing to {}: {}", tmp_file.path, e.what());
   }
 
-  primary.put(key, type, [&](const auto& path) {
-    try {
-      Util::copy_file(tmp_file.path, path);
-    } catch (const core::Error& e) {
-      LOG("Failed to copy {} to {}: {}", tmp_file.path, path, e.what());
-      // Don't indicate failure since get from primary storage was OK.
-    }
-    return true;
-  });
+  if (share_hits) {
+    primary.put(key, type, [&](const auto& path) {
+      try {
+        Util::copy_file(tmp_file.path, path);
+      } catch (const core::Error& e) {
+        LOG("Failed to copy {} to {}: {}", tmp_file.path, path, e.what());
+        // Don't indicate failure since get from primary storage was OK.
+      }
+      return true;
+    });
+  }
 
   return tmp_file.path;
 }
@@ -442,7 +449,7 @@ Storage::get_backend(SecondaryStorageEntry& entry,
   }
 }
 
-nonstd::optional<std::string>
+nonstd::optional<std::pair<std::string, bool>>
 Storage::get_from_secondary_storage(const Digest& key)
 {
   for (const auto& entry : m_secondary_storages) {
@@ -466,7 +473,7 @@ Storage::get_from_secondary_storage(const Digest& key)
           backend->url_for_logging,
           ms);
       primary.increment_statistic(core::Statistic::secondary_storage_hit);
-      return *value;
+      return std::make_pair(*value, entry->config.share_hits);
     } else {
       LOG("No {} in {} ({:.2f} ms)",
           key.to_string(),

diff --git a/src/storage/Storage.hpp b/src/storage/Storage.hpp
index 310eebfd38491e5c478368480ad1030750e1a564..3e596745ad9be36956f655aa7c4caa86f680c71d 100644 (file)
--- a/src/storage/Storage.hpp
+++ b/src/storage/Storage.hpp
@@ -28,6 +28,7 @@
 #include <functional>
 #include <memory>
 #include <string>
+#include <utility>
 #include <vector>
 
 class Digest;
@@ -78,7 +79,8 @@ private:
               const Digest& key,
               nonstd::string_view operation_description,
               const bool for_writing);
-  nonstd::optional<std::string> get_from_secondary_storage(const Digest& key);
+  nonstd::optional<std::pair<std::string, bool>>
+  get_from_secondary_storage(const Digest& key);
 
   void put_in_secondary_storage(const Digest& key,
                                 const std::string& value,

diff --git a/src/storage/secondary/SecondaryStorage.cpp b/src/storage/secondary/SecondaryStorage.cpp
index 41b3d38d3c186cd5d5c2895c9532881f22c4120c..c73f19db4301de98ba651a718f4514dc7840884b 100644 (file)
--- a/src/storage/secondary/SecondaryStorage.cpp
+++ b/src/storage/secondary/SecondaryStorage.cpp
@@ -27,7 +27,7 @@ namespace secondary {
 bool
 SecondaryStorage::Backend::is_framework_attribute(const std::string& name)
 {
-  return name == "read-only" || name == "shards";
+  return name == "read-only" || name == "shards" || name == "share-hits";
 }
 
 std::chrono::milliseconds

diff --git a/test/suites/secondary_file.bash b/test/suites/secondary_file.bash
index 85ae6285b0827afa820f604a053d11abdb63da1b..5fc518374599a178eadad0f379bc39552eca4352 100644 (file)
--- a/test/suites/secondary_file.bash
+++ b/test/suites/secondary_file.bash
@@ -219,4 +219,31 @@ SUITE_secondary_file() {
     expect_stat secondary_storage_hit 2
     expect_stat secondary_storage_miss 0
     expect_file_count 3 '*' secondary # CACHEDIR.TAG + result + manifest
+
+    # -------------------------------------------------------------------------
+    TEST "Don't share hits"
+
+    $CCACHE_COMPILE -c test.c
+    expect_stat direct_cache_hit 0
+    expect_stat cache_miss 1
+    expect_stat files_in_cache 2
+    expect_stat primary_storage_hit 0
+    expect_stat primary_storage_miss 2
+    expect_stat secondary_storage_hit 0
+    expect_stat secondary_storage_miss 2
+    expect_file_count 3 '*' secondary # CACHEDIR.TAG + result + manifest
+
+    $CCACHE -C >/dev/null
+    expect_stat files_in_cache 0
+
+    CCACHE_SECONDARY_STORAGE+="|share-hits=false"
+    $CCACHE_COMPILE -c test.c
+    expect_stat direct_cache_hit 1
+    expect_stat cache_miss 1
+    expect_stat files_in_cache 0
+    expect_stat primary_storage_hit 0
+    expect_stat primary_storage_miss 4
+    expect_stat secondary_storage_hit 2
+    expect_stat secondary_storage_miss 2
+    expect_file_count 0
 }