From: Joel Rosdahl Date: Mon, 12 Sep 2022 19:27:52 +0000 (+0200) Subject: refactor: Improve Storage::get API to be able to return multiple times X-Git-Tag: v4.7~53 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0db6d06df4d70432fbf279759a7d726782115ac8;p=thirdparty%2Fccache.git refactor: Improve Storage::get API to be able to return multiple times --- diff --git a/src/ccache.cpp b/src/ccache.cpp index 8197c1c97..ec46ca813 100644 --- 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 c468a2e91..9e617fb37 100644 --- a/src/storage/Storage.cpp +++ b/src/storage/Storage.cpp @@ -230,34 +230,29 @@ Storage::finalize() primary.finalize(); } -std::optional +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 -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 eba46897d..152c8349b 100644 --- a/src/storage/Storage.hpp +++ b/src/storage/Storage.hpp @@ -26,6 +26,7 @@ #include +#include #include #include #include @@ -51,10 +52,11 @@ public: primary::PrimaryStorage primary; - enum class Mode { secondary_fallback, secondary_only, primary_only }; - std::optional get(const Digest& key, - core::CacheEntryType type, - const Mode mode = Mode::secondary_fallback); + using EntryReceiver = std::function; + + 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 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 value, diff --git a/src/storage/primary/PrimaryStorage.cpp b/src/storage/primary/PrimaryStorage.cpp index 8a520d3e7..c9e00ee80 100644 --- 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 value) + nonstd::span 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 58ed02add..2789a141d 100644 --- 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 value); + nonstd::span value, + bool only_if_missing = false); void remove(const Digest& key, core::CacheEntryType type);