From 71ea973ae46aeaa082bb0ee347e4e3133ddd0ed4 Mon Sep 17 00:00:00 2001 From: Ernest Zaslavsky Date: Tue, 2 Sep 2025 18:00:46 +0300 Subject: [PATCH] s3 cleanup: remove obsolete retry-related classes Delete `default_retry_strategy` and `retryable_http_client`, no longer used in `s3_client` after recent refactors. --- configure.py | 2 - utils/CMakeLists.txt | 2 - utils/s3/retry_strategy.cc | 47 -------------- utils/s3/retry_strategy.hh | 44 ------------- utils/s3/retryable_http_client.cc | 101 ------------------------------ utils/s3/retryable_http_client.hh | 41 ------------ 6 files changed, 237 deletions(-) delete mode 100644 utils/s3/retry_strategy.cc delete mode 100644 utils/s3/retry_strategy.hh delete mode 100644 utils/s3/retryable_http_client.cc delete mode 100644 utils/s3/retryable_http_client.hh diff --git a/configure.py b/configure.py index f45514eebc..16558d8130 100755 --- a/configure.py +++ b/configure.py @@ -1082,8 +1082,6 @@ scylla_core = (['message/messaging_service.cc', 'utils/rest/client.cc', 'utils/s3/aws_error.cc', 'utils/s3/client.cc', - 'utils/s3/retryable_http_client.cc', - 'utils/s3/retry_strategy.cc', 'utils/s3/default_aws_retry_strategy.cc', 'utils/s3/credentials_providers/aws_credentials_provider.cc', 'utils/s3/credentials_providers/environment_aws_credentials_provider.cc', diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index b92583c8f1..d7c21575ac 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -52,8 +52,6 @@ target_sources(utils rest/client.cc s3/aws_error.cc s3/client.cc - s3/retryable_http_client.cc - s3/retry_strategy.cc s3/default_aws_retry_strategy.cc s3/credentials_providers/aws_credentials_provider.cc s3/credentials_providers/environment_aws_credentials_provider.cc diff --git a/utils/s3/retry_strategy.cc b/utils/s3/retry_strategy.cc deleted file mode 100644 index 420627e1d5..0000000000 --- a/utils/s3/retry_strategy.cc +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (C) 2024-present ScyllaDB - */ - -/* - * SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 - */ - -#include "retry_strategy.hh" -#include "aws_error.hh" -#include "utils/log.hh" - -using namespace std::chrono_literals; - -namespace aws { - -static logging::logger rs_logger("default_retry_strategy"); - -default_retry_strategy::default_retry_strategy(unsigned max_retries, unsigned scale_factor) : _max_retries(max_retries), _scale_factor(scale_factor) { -} - -seastar::future default_retry_strategy::should_retry(const aws_error& error, unsigned attempted_retries) const { - if (attempted_retries >= _max_retries) { - rs_logger.warn("Retries exhausted. Retry# {}", attempted_retries); - co_return false; - } - bool should_retry = error.is_retryable() == retryable::yes; - if (should_retry) { - rs_logger.debug("AWS HTTP client request failed. Reason: {}. Retry# {}", error.get_error_message(), attempted_retries); - } else { - rs_logger.warn("AWS HTTP client encountered non-retryable error. Reason: {}. Code: {}. Retry# {}", - error.get_error_message(), - std::to_underlying(error.get_error_type()), - attempted_retries); - } - co_return should_retry; -} - -std::chrono::milliseconds default_retry_strategy::delay_before_retry(const aws_error&, unsigned attempted_retries) const { - if (attempted_retries == 0) { - return 0ms; - } - - return std::chrono::milliseconds((1UL << attempted_retries) * _scale_factor); -} - -} // namespace aws diff --git a/utils/s3/retry_strategy.hh b/utils/s3/retry_strategy.hh deleted file mode 100644 index b40cc075a2..0000000000 --- a/utils/s3/retry_strategy.hh +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (C) 2024-present ScyllaDB - */ - -/* - * SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 - */ - -#pragma once -#include -#include - -namespace aws { - -class aws_error; - -class retry_strategy { -public: - virtual ~retry_strategy() = default; - // Returns true if the error can be retried given the error and the number of times already tried. - virtual seastar::future should_retry(const aws_error& error, unsigned attempted_retries) const = 0; - - // Calculates the time in milliseconds the client should wait before attempting another request based on the error and attemptedRetries count. - [[nodiscard]] virtual std::chrono::milliseconds delay_before_retry(const aws_error& error, unsigned attempted_retries) const = 0; - - [[nodiscard]] virtual unsigned get_max_retries() const = 0; -}; - -class default_retry_strategy : public retry_strategy { -protected: - unsigned _max_retries; - unsigned _scale_factor; - -public: - explicit default_retry_strategy(unsigned max_retries = 10, unsigned scale_factor = 25); - - seastar::future should_retry(const aws_error& error, unsigned attempted_retries) const override; - - [[nodiscard]] std::chrono::milliseconds delay_before_retry(const aws_error& error, unsigned attempted_retries) const override; - - [[nodiscard]] unsigned get_max_retries() const override { return _max_retries; } -}; - -} // namespace aws diff --git a/utils/s3/retryable_http_client.cc b/utils/s3/retryable_http_client.cc deleted file mode 100644 index 405d817056..0000000000 --- a/utils/s3/retryable_http_client.cc +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Copyright (C) 2025-present ScyllaDB - */ - -/* - * SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 - */ - -#include "retryable_http_client.hh" -#include "aws_error.hh" -#include "client.hh" -#include "utils/error_injection.hh" - -#include -#include -#include -#include - -namespace aws { - -using namespace seastar; -retryable_http_client::retryable_http_client(std::unique_ptr&& factory, - unsigned max_conn, - error_handler error_func, - http::experimental::client::retry_requests should_retry, - const aws::retry_strategy& retry_strategy) - : http(std::move(factory), max_conn, should_retry), _retry_strategy(retry_strategy), _error_handler(std::move(error_func)) { - assert(_error_handler); -} - -future<> retryable_http_client::do_retryable_request(const seastar::http::request& req, http::experimental::client::reply_handler handler, seastar::abort_source* as) { - // TODO: the http client does not check abort status on entry, and if - // we're already aborted when we get here we will paradoxally not be - // interrupted, because no registration etc will be done. So do a quick - // preemptive check already. - if (as && as->abort_requested()) { - co_await coroutine::return_exception_ptr(as->abort_requested_exception_ptr()); - } - uint32_t retries = 0; - std::exception_ptr e; - aws::aws_exception request_ex{aws::aws_error{aws::aws_error_type::OK, aws::retryable::yes}}; - while (true) { - try { - // We need to be able to simulate a retry in s3 tests - if (utils::get_local_injector().enter("s3_client_fail_authorization")) { - throw aws::aws_exception(aws::aws_error{aws::aws_error_type::HTTP_UNAUTHORIZED, - "EACCESS fault injected to simulate authorization failure", aws::retryable::no}); - } - e = {}; - co_return co_await http.make_request(req, handler, std::nullopt, as); - } catch (...) { - e = std::current_exception(); - request_ex = aws_exception(aws_error::from_exception_ptr(e)); - } - if (request_ex.error().get_error_type() == aws::aws_error_type::REQUEST_TIME_TOO_SKEWED || - request_ex.error().get_error_type() == aws::aws_error_type::EXPIRED_TOKEN) { - co_await coroutine::return_exception_ptr(std::move(e)); - } - if (!co_await _retry_strategy.should_retry(request_ex.error(), retries)) { - break; - } - co_await seastar::sleep(_retry_strategy.delay_before_retry(request_ex.error(), retries)); - ++retries; - } - - if (e) { - _error_handler(e); - } -} - -future<> retryable_http_client::make_request(const seastar::http::request& req, - http::experimental::client::reply_handler handle, - std::optional expected, - seastar::abort_source* as) { - co_await do_retryable_request( - req, - [handler = std::move(handle), expected](const http::reply& rep, input_stream&& in) mutable -> future<> { - auto payload = std::move(in); - auto status_class = http::reply::classify_status(rep._status); - - if (status_class != http::reply::status_class::informational && status_class != http::reply::status_class::success) { - std::optional possible_error = aws::aws_error::parse(co_await util::read_entire_stream_contiguous(payload)); - if (possible_error) { - co_await coroutine::return_exception(aws::aws_exception(std::move(possible_error.value()))); - } - co_await coroutine::return_exception(aws::aws_exception(aws::aws_error::from_http_code(rep._status))); - } - - if (expected.has_value() && rep._status != *expected) { - co_await coroutine::return_exception(httpd::unexpected_status_error(rep._status)); - } - co_await handler(rep, std::move(payload)); - }, - as); -} - -future<> retryable_http_client::close() { - return http.close(); -} - -} // namespace aws diff --git a/utils/s3/retryable_http_client.hh b/utils/s3/retryable_http_client.hh deleted file mode 100644 index 2e82b88a6f..0000000000 --- a/utils/s3/retryable_http_client.hh +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (C) 2025-present ScyllaDB - */ - -/* - * SPDX-License-Identifier: LicenseRef-ScyllaDB-Source-Available-1.0 - */ - -#pragma once -#include "retry_strategy.hh" -#include - -namespace aws { - -class retryable_http_client { -public: - using error_handler = std::function; - - retryable_http_client(std::unique_ptr&& factory, - unsigned max_conn, - error_handler error_func, - seastar::http::experimental::client::retry_requests should_retry, - const aws::retry_strategy& retry_strategy); - seastar::future<> make_request(const seastar::http::request& req, - seastar::http::experimental::client::reply_handler handle, - std::optional expected = std::nullopt, - seastar::abort_source* = nullptr); - seastar::future<> close(); - [[nodiscard]] const seastar::http::experimental::client& get_http_client() const { return http; }; - static void ignore_exception(std::exception_ptr) {} - -private: - seastar::future<> - do_retryable_request(const seastar::http::request& req, seastar::http::experimental::client::reply_handler handler, seastar::abort_source* as = nullptr); - - seastar::http::experimental::client http; - const aws::retry_strategy& _retry_strategy; - error_handler _error_handler; -}; - -} // namespace aws