From 0b848592a8d84e3b3bda23584308ab24965e55ca Mon Sep 17 00:00:00 2001 From: Paul Cristian Sarbu Date: Mon, 15 Jul 2024 12:35:05 +0200 Subject: Fix naming inconsistencies in retry configuration The retry_parameters.hpp header-only library defining the Retry class now is the retry_config.hpp header-only library defining the RetryConfig class. --- src/buildtool/common/remote/TARGETS | 8 +- src/buildtool/common/remote/retry.hpp | 10 +- src/buildtool/common/remote/retry_config.hpp | 133 +++++++++++++++++++++++ src/buildtool/common/remote/retry_parameters.hpp | 133 ----------------------- src/buildtool/main/TARGETS | 4 +- src/buildtool/main/retry.cpp | 9 +- src/buildtool/main/serve.cpp | 8 +- 7 files changed, 153 insertions(+), 152 deletions(-) create mode 100644 src/buildtool/common/remote/retry_config.hpp delete mode 100644 src/buildtool/common/remote/retry_parameters.hpp (limited to 'src') diff --git a/src/buildtool/common/remote/TARGETS b/src/buildtool/common/remote/TARGETS index a5480e2b..52e3f91a 100644 --- a/src/buildtool/common/remote/TARGETS +++ b/src/buildtool/common/remote/TARGETS @@ -38,10 +38,10 @@ ] , "stage": ["src", "buildtool", "common", "remote"] } -, "retry_parameters": +, "retry_config": { "type": ["@", "rules", "CC", "library"] - , "name": ["retry_parameters"] - , "hdrs": ["retry_parameters.hpp"] + , "name": ["retry_config"] + , "hdrs": ["retry_config.hpp"] , "deps": [ ["src/buildtool/logging", "log_level"] , ["src/buildtool/logging", "logging"] @@ -56,7 +56,7 @@ [ ["src/buildtool/logging", "log_level"] , ["src/buildtool/logging", "logging"] , ["@", "grpc", "", "grpc++"] - , "retry_parameters" + , "retry_config" ] , "stage": ["src", "buildtool", "common", "remote"] } diff --git a/src/buildtool/common/remote/retry.hpp b/src/buildtool/common/remote/retry.hpp index b9298588..6e164af1 100644 --- a/src/buildtool/common/remote/retry.hpp +++ b/src/buildtool/common/remote/retry.hpp @@ -20,7 +20,7 @@ #include // std::move #include "grpcpp/grpcpp.h" -#include "src/buildtool/common/remote/retry_parameters.hpp" +#include "src/buildtool/common/remote/retry_config.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" @@ -56,7 +56,7 @@ template [[nodiscard]] auto WithRetry(F const& f, Logger const& logger) noexcept -> bool { try { - auto const& attempts = Retry::GetMaxAttempts(); + auto const& attempts = RetryConfig::GetMaxAttempts(); for (auto attempt = 1U; attempt <= attempts; ++attempt) { auto [ok, fatal, error_msg] = f(); if (ok) { @@ -71,7 +71,7 @@ template // don't wait if it was the last attempt if (attempt < attempts) { auto const sleep_for_seconds = - Retry::GetSleepTimeSeconds(attempt); + RetryConfig::GetSleepTimeSeconds(attempt); logger.Emit(kRetryLogLevel, "Attempt {}/{} failed{} Retrying in {} seconds.", attempt, @@ -107,7 +107,7 @@ template -> std::pair { grpc::Status status{}; try { - auto attempts = Retry::GetMaxAttempts(); + auto attempts = RetryConfig::GetMaxAttempts(); for (auto attempt = 1U; attempt <= attempts; ++attempt) { status = f(); if (status.ok() or @@ -117,7 +117,7 @@ template // don't wait if it was the last attempt if (attempt < attempts) { auto const sleep_for_seconds = - Retry::GetSleepTimeSeconds(attempt); + RetryConfig::GetSleepTimeSeconds(attempt); logger.Emit( kRetryLogLevel, "Attempt {}/{} failed: {}: {}: Retrying in {} seconds.", diff --git a/src/buildtool/common/remote/retry_config.hpp b/src/buildtool/common/remote/retry_config.hpp new file mode 100644 index 00000000..8957ac1f --- /dev/null +++ b/src/buildtool/common/remote/retry_config.hpp @@ -0,0 +1,133 @@ +// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef INCLUDED_SRC_BUILDTOOL_COMMON_RETRY_PARAMETERS_HPP +#define INCLUDED_SRC_BUILDTOOL_COMMON_RETRY_PARAMETERS_HPP + +#include +#include + +#include "src/buildtool/logging/log_level.hpp" +#include "src/buildtool/logging/logger.hpp" + +constexpr unsigned int kDefaultInitialBackoffSeconds{1}; +constexpr unsigned int kDefaultMaxBackoffSeconds{60}; +constexpr unsigned int kDefaultAttempts{1}; +constexpr auto kRetryLogLevel = LogLevel::Progress; +class RetryConfig { + using dist_type = std::uniform_int_distribution; + + public: + RetryConfig() = default; + [[nodiscard]] static auto Instance() -> RetryConfig& { + static RetryConfig instance{}; + return instance; + } + + [[nodiscard]] static auto SetInitialBackoffSeconds(unsigned int x) noexcept + -> bool { + if (x < 1) { + Logger::Log( + LogLevel::Error, + "Invalid initial amount of seconds provided: {}. Value must " + "be strictly greater than 0.", + x); + return false; + } + Instance().initial_backoff_seconds_ = x; + return true; + } + + [[nodiscard]] static auto SetMaxBackoffSeconds(unsigned int x) noexcept + -> bool { + if (x < 1) { + Logger::Log(LogLevel::Error, + "Invalid max backoff provided: {}. Value must be " + "strictly greater than 0.", + x); + return false; + } + Instance().max_backoff_seconds_ = x; + return true; + } + + [[nodiscard]] static auto GetMaxBackoffSeconds() noexcept -> unsigned int { + return Instance().max_backoff_seconds_; + } + + [[nodiscard]] static auto SetMaxAttempts(unsigned int x) noexcept -> bool { + if (x < 1) { + Logger::Log(LogLevel::Error, + "Invalid number of max number of attempts provided: " + "{}. Value must be strictly greater than 0", + x); + return false; + } + Instance().attempts_ = x; + return true; + } + + [[nodiscard]] static auto GetInitialBackoffSeconds() noexcept + -> unsigned int { + return Instance().initial_backoff_seconds_; + } + + [[nodiscard]] static auto GetMaxAttempts() noexcept -> unsigned int { + return Instance().attempts_; + } + + [[nodiscard]] static auto Jitter(unsigned int backoff) noexcept -> + typename dist_type::result_type { + auto& inst = Instance(); + try { + dist_type dist{0, backoff * 3}; + std::unique_lock lock(inst.mutex_); + return dist(inst.rng_); + } catch (...) { + return 0; + } + } + + /// \brief The waiting time is exponentially increased at each \p attempt + /// until it exceeds max_backoff_seconds. + /// + /// To avoid overloading of the reachable resources, a jitter (aka, random + /// value) is added to distributed the workload. + [[nodiscard]] static auto GetSleepTimeSeconds(unsigned int attempt) noexcept + -> unsigned int { + auto backoff = RetryConfig::GetInitialBackoffSeconds(); + auto const& max_backoff = RetryConfig::GetMaxBackoffSeconds(); + // on the first attempt, we don't double the backoff time + // also we do it in a for loop to avoid overflow + for (auto x = 1U; x < attempt; ++x) { + backoff <<= 1U; + if (backoff >= max_backoff) { + backoff = max_backoff; + break; + } + } + return backoff + RetryConfig::Jitter(backoff); + } + + private: + unsigned int initial_backoff_seconds_{kDefaultInitialBackoffSeconds}; + unsigned int max_backoff_seconds_{kDefaultMaxBackoffSeconds}; + unsigned int attempts_{kDefaultAttempts}; + LogLevel retry_log_level_{kRetryLogLevel}; + std::mutex mutex_; + std::random_device dev_; + std::mt19937 rng_{dev_()}; +}; + +#endif // INCLUDED_SRC_BUILDTOOL_COMMON_RETRY_PARAMETERS_HPP diff --git a/src/buildtool/common/remote/retry_parameters.hpp b/src/buildtool/common/remote/retry_parameters.hpp deleted file mode 100644 index bc4c474a..00000000 --- a/src/buildtool/common/remote/retry_parameters.hpp +++ /dev/null @@ -1,133 +0,0 @@ -// Copyright 2023 Huawei Cloud Computing Technology Co., Ltd. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef INCLUDED_SRC_BUILDTOOL_COMMON_RETRY_PARAMETERS_HPP -#define INCLUDED_SRC_BUILDTOOL_COMMON_RETRY_PARAMETERS_HPP - -#include -#include - -#include "src/buildtool/logging/log_level.hpp" -#include "src/buildtool/logging/logger.hpp" - -constexpr unsigned int kDefaultInitialBackoffSeconds{1}; -constexpr unsigned int kDefaultMaxBackoffSeconds{60}; -constexpr unsigned int kDefaultAttempts{1}; -constexpr auto kRetryLogLevel = LogLevel::Progress; -class Retry { - using dist_type = std::uniform_int_distribution; - - public: - Retry() = default; - [[nodiscard]] static auto Instance() -> Retry& { - static Retry instance{}; - return instance; - } - - [[nodiscard]] static auto SetInitialBackoffSeconds(unsigned int x) noexcept - -> bool { - if (x < 1) { - Logger::Log( - LogLevel::Error, - "Invalid initial amount of seconds provided: {}. Value must " - "be strictly greater than 0.", - x); - return false; - } - Instance().initial_backoff_seconds_ = x; - return true; - } - - [[nodiscard]] static auto SetMaxBackoffSeconds(unsigned int x) noexcept - -> bool { - if (x < 1) { - Logger::Log(LogLevel::Error, - "Invalid max backoff provided: {}. Value must be " - "strictly greater than 0.", - x); - return false; - } - Instance().max_backoff_seconds_ = x; - return true; - } - - [[nodiscard]] static auto GetMaxBackoffSeconds() noexcept -> unsigned int { - return Instance().max_backoff_seconds_; - } - - [[nodiscard]] static auto SetMaxAttempts(unsigned int x) noexcept -> bool { - if (x < 1) { - Logger::Log(LogLevel::Error, - "Invalid number of max number of attempts provided: " - "{}. Value must be strictly greater than 0", - x); - return false; - } - Instance().attempts_ = x; - return true; - } - - [[nodiscard]] static auto GetInitialBackoffSeconds() noexcept - -> unsigned int { - return Instance().initial_backoff_seconds_; - } - - [[nodiscard]] static auto GetMaxAttempts() noexcept -> unsigned int { - return Instance().attempts_; - } - - [[nodiscard]] static auto Jitter(unsigned int backoff) noexcept -> - typename dist_type::result_type { - auto& inst = Instance(); - try { - dist_type dist{0, backoff * 3}; - std::unique_lock lock(inst.mutex_); - return dist(inst.rng_); - } catch (...) { - return 0; - } - } - - /// \brief The waiting time is exponentially increased at each \p attempt - /// until it exceeds max_backoff_seconds. - /// - /// To avoid overloading of the reachable resources, a jitter (aka, random - /// value) is added to distributed the workload. - [[nodiscard]] static auto GetSleepTimeSeconds(unsigned int attempt) noexcept - -> unsigned int { - auto backoff = Retry::GetInitialBackoffSeconds(); - auto const& max_backoff = Retry::GetMaxBackoffSeconds(); - // on the first attempt, we don't double the backoff time - // also we do it in a for loop to avoid overflow - for (auto x = 1U; x < attempt; ++x) { - backoff <<= 1U; - if (backoff >= max_backoff) { - backoff = max_backoff; - break; - } - } - return backoff + Retry::Jitter(backoff); - } - - private: - unsigned int initial_backoff_seconds_{kDefaultInitialBackoffSeconds}; - unsigned int max_backoff_seconds_{kDefaultMaxBackoffSeconds}; - unsigned int attempts_{kDefaultAttempts}; - LogLevel retry_log_level_{kRetryLogLevel}; - std::mutex mutex_; - std::random_device dev_; - std::mt19937 rng_{dev_()}; -}; - -#endif // INCLUDED_SRC_BUILDTOOL_COMMON_RETRY_PARAMETERS_HPP diff --git a/src/buildtool/main/TARGETS b/src/buildtool/main/TARGETS index 75c5bddc..ce7ec45f 100644 --- a/src/buildtool/main/TARGETS +++ b/src/buildtool/main/TARGETS @@ -66,7 +66,7 @@ , "srcs": ["retry.cpp"] , "stage": ["src", "buildtool", "main"] , "deps": [["src/buildtool/common", "retry_cli"]] - , "private-deps": [["src/buildtool/common/remote", "retry_parameters"]] + , "private-deps": [["src/buildtool/common/remote", "retry_config"]] } , "describe": { "type": ["@", "rules", "CC", "library"] @@ -286,7 +286,7 @@ , ["src/buildtool/logging", "log_level"] , ["src/buildtool/logging", "logging"] , "common" - , ["src/buildtool/common/remote", "retry_parameters"] + , ["src/buildtool/common/remote", "retry_config"] ] } , "build_utils": diff --git a/src/buildtool/main/retry.cpp b/src/buildtool/main/retry.cpp index f2ba4b3a..c7c100cf 100644 --- a/src/buildtool/main/retry.cpp +++ b/src/buildtool/main/retry.cpp @@ -22,26 +22,27 @@ #else -#include "src/buildtool/common/remote/retry_parameters.hpp" +#include "src/buildtool/common/remote/retry_config.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" [[nodiscard]] auto SetupRetryConfig(RetryArguments const& args) -> bool { if (args.max_attempts) { - if (!Retry::SetMaxAttempts(*args.max_attempts)) { + if (!RetryConfig::SetMaxAttempts(*args.max_attempts)) { Logger::Log(LogLevel::Error, "Invalid value for max-attempts."); return false; } } if (args.initial_backoff_seconds) { - if (!Retry::SetInitialBackoffSeconds(*args.initial_backoff_seconds)) { + if (!RetryConfig::SetInitialBackoffSeconds( + *args.initial_backoff_seconds)) { Logger::Log(LogLevel::Error, "Invalid value for initial-backoff-seconds."); return false; } } if (args.max_backoff_seconds) { - if (!Retry::SetMaxBackoffSeconds(*args.max_backoff_seconds)) { + if (!RetryConfig::SetMaxBackoffSeconds(*args.max_backoff_seconds)) { Logger::Log(LogLevel::Error, "Invalid value for max-backoff-seconds."); return false; diff --git a/src/buildtool/main/serve.cpp b/src/buildtool/main/serve.cpp index e21eac0f..e5dec606 100644 --- a/src/buildtool/main/serve.cpp +++ b/src/buildtool/main/serve.cpp @@ -31,7 +31,7 @@ #include "src/buildtool/build_engine/expression/configuration.hpp" #include "src/buildtool/build_engine/expression/expression.hpp" #include "src/buildtool/common/location.hpp" -#include "src/buildtool/common/remote/retry_parameters.hpp" +#include "src/buildtool/common/remote/retry_config.hpp" #include "src/buildtool/file_system/file_system_manager.hpp" #include "src/buildtool/logging/log_level.hpp" #include "src/buildtool/logging/logger.hpp" @@ -67,7 +67,7 @@ namespace { max_attempts->ToString()); return false; } - if (!Retry::SetMaxAttempts(max_attempts->Number())) { + if (!RetryConfig::SetMaxAttempts(max_attempts->Number())) { Logger::Log(LogLevel::Error, "Invalid value for \"max-attempts\" {}.", max_attempts->Number()); @@ -83,7 +83,7 @@ namespace { initial_backoff->ToString()); return false; } - if (!Retry::SetMaxAttempts(initial_backoff->Number())) { + if (!RetryConfig::SetMaxAttempts(initial_backoff->Number())) { Logger::Log(LogLevel::Error, "Invalid value for \"initial-backoff-seconds\" {}.", initial_backoff->Number()); @@ -99,7 +99,7 @@ namespace { max_backoff->ToString()); return false; } - if (!Retry::SetMaxAttempts(max_backoff->Number())) { + if (!RetryConfig::SetMaxAttempts(max_backoff->Number())) { Logger::Log(LogLevel::Error, "Invalid value for \"max-backoff-seconds\" {}.", max_backoff->Number()); -- cgit v1.2.3