diff options
| -rw-r--r-- | doc/MANUAL.adoc | 9 | ||||
| -rw-r--r-- | src/storage/Storage.cpp | 38 | ||||
| -rw-r--r-- | src/storage/Storage.hpp | 4 | ||||
| -rw-r--r-- | src/storage/secondary/SecondaryStorage.cpp | 2 | ||||
| -rw-r--r-- | test/suites/secondary_file.bash | 27 |
5 files changed, 20 insertions, 60 deletions
diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc index eaa25d98..290d3cf3 100644 --- a/doc/MANUAL.adoc +++ b/doc/MANUAL.adoc @@ -1072,8 +1072,6 @@ 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*. === Storage interaction @@ -1086,14 +1084,13 @@ on cache hits and misses: | *Primary storage* | *Secondary storage* | *What happens* | miss | miss | Compile, write to primary, write to secondary^[1]^ -| miss | hit | Read from secondary, write to primary^[2]^ -| hit | - | Read from primary, don't write to secondary^[3]^ +| miss | hit | Read from secondary, write to primary +| hit | - | Read from primary, don't write to secondary^[2]^ |============================================================================== ^[1]^ Unless secondary storage has attribute `read-only=true`. + -^[2]^ Unless secondary storage has attribute `share-hits=false`. + -^[3]^ Unless primary storage is set to share its cache hits with the +^[2]^ Unless primary storage is set to share its cache hits with the <<config_reshare,*reshare*>> option. diff --git a/src/storage/Storage.cpp b/src/storage/Storage.cpp index e3f9b963..af8c1151 100644 --- a/src/storage/Storage.cpp +++ b/src/storage/Storage.cpp @@ -41,7 +41,6 @@ #include <third_party/url.hpp> -#include <algorithm> #include <cmath> #include <unordered_map> #include <vector> @@ -83,7 +82,6 @@ struct SecondaryStorageConfig std::vector<SecondaryStorageShardConfig> shards; secondary::SecondaryStorage::Backend::Params params; bool read_only = false; - bool share_hits = true; }; struct SecondaryStorageBackendEntry @@ -175,8 +173,6 @@ parse_storage_config(const std::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( @@ -267,33 +263,29 @@ Storage::get(const Digest& key, return std::nullopt; } - const auto value_and_share_hits = get_from_secondary_storage(key); - if (!value_and_share_hits) { + const auto value = get_from_secondary_storage(key); + if (!value) { return std::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(FMT("Error writing to {}: {}", tmp_file.path, e.what())); } - if (share_hits) { - primary.put(key, type, [&](const auto& path) { - try { - Util::ensure_dir_exists(Util::dir_name(path)); - 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; - }); - } + primary.put(key, type, [&](const auto& path) { + try { + Util::ensure_dir_exists(Util::dir_name(path)); + 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; } @@ -477,7 +469,7 @@ Storage::get_backend(SecondaryStorageEntry& entry, } } -std::optional<std::pair<util::Blob, bool>> +std::optional<util::Blob> Storage::get_from_secondary_storage(const Digest& key) { MTR_SCOPE("secondary_storage", "get"); @@ -503,7 +495,7 @@ Storage::get_from_secondary_storage(const Digest& key) backend->url_for_logging, ms); primary.increment_statistic(core::Statistic::secondary_storage_hit); - return std::make_pair(*value, entry->config.share_hits); + return *value; } else { LOG("No {} in {} ({:.2f} ms)", key.to_string(), diff --git a/src/storage/Storage.hpp b/src/storage/Storage.hpp index 5d9abd0b..6810fe13 100644 --- a/src/storage/Storage.hpp +++ b/src/storage/Storage.hpp @@ -28,7 +28,6 @@ #include <memory> #include <optional> #include <string> -#include <utility> #include <vector> class Digest; @@ -82,8 +81,7 @@ private: const Digest& key, std::string_view operation_description, const bool for_writing); - std::optional<std::pair<util::Blob, bool>> - get_from_secondary_storage(const Digest& key); + std::optional<util::Blob> get_from_secondary_storage(const Digest& key); void put_in_secondary_storage(const Digest& key, const util::Blob& value, diff --git a/src/storage/secondary/SecondaryStorage.cpp b/src/storage/secondary/SecondaryStorage.cpp index e8004221..a9ae934f 100644 --- a/src/storage/secondary/SecondaryStorage.cpp +++ b/src/storage/secondary/SecondaryStorage.cpp @@ -26,7 +26,7 @@ namespace storage::secondary { bool SecondaryStorage::Backend::is_framework_attribute(const std::string& name) { - return name == "read-only" || name == "shards" || name == "share-hits"; + return name == "read-only" || name == "shards"; } std::chrono::milliseconds diff --git a/test/suites/secondary_file.bash b/test/suites/secondary_file.bash index 1a688625..e432dd7e 100644 --- a/test/suites/secondary_file.bash +++ b/test/suites/secondary_file.bash @@ -240,33 +240,6 @@ SUITE_secondary_file() { 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 3 '*' secondary # CACHEDIR.TAG + result + manifest - - # ------------------------------------------------------------------------- TEST "Recache" CCACHE_RECACHE=1 $CCACHE_COMPILE -c test.c |