From 5cf18067204d632fd2bc4e0c6bce8c77ad6ef202 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Thu, 17 Jun 2021 23:19:16 -0700 Subject: [PATCH] crimson/common/interruptible_future: refactor handle_interruption handle_interruption can't really be validly used outside of with_interruption_cond. Make private, and adjust is_interruption to not require an instance. Signed-off-by: Samuel Just --- src/crimson/common/exception.h | 19 ----- src/crimson/common/interruptible_future.h | 80 +++++-------------- .../osd/pg_interval_interrupt_condition.h | 2 +- src/test/crimson/test_interruptible_future.cc | 2 +- 4 files changed, 24 insertions(+), 79 deletions(-) diff --git a/src/crimson/common/exception.h b/src/crimson/common/exception.h index dafb21a9419..682fef69b7e 100644 --- a/src/crimson/common/exception.h +++ b/src/crimson/common/exception.h @@ -35,25 +35,6 @@ private: const bool still_primary; }; -template -inline seastar::future<> with_interruption( - OpFunc&& opfunc, OnInterrupt&& efunc) { - if constexpr (may_loop) { - return ::seastar::repeat( - [opfunc=std::move(opfunc), efunc=std::move(efunc)]() mutable { - return ::crimson::interruptible::interruptor::template futurize< - std::result_of_t>::apply(std::move(opfunc), std::make_tuple()) - .template handle_interruption(std::move(efunc)); - }); - } else { - return ::crimson::interruptible::interruptor::template futurize< - std::result_of_t>::apply(std::move(opfunc), std::make_tuple()) - .template handle_interruption(std::move(efunc)); - } -} - template inline seastar::future<> handle_system_shutdown(Func&& func, Args&&... args) { diff --git a/src/crimson/common/interruptible_future.h b/src/crimson/common/interruptible_future.h index a1f53dafd18..643098624cc 100644 --- a/src/crimson/common/interruptible_future.h +++ b/src/crimson/common/interruptible_future.h @@ -265,25 +265,6 @@ public: using core_type::available; using core_type::failed; - template - [[gnu::always_inline]] - auto handle_interruption(Func&& func) { - return core_type::then_wrapped( - [func=std::move(func), - interrupt_condition=interrupt_cond](auto&& fut) mutable { - if (fut.failed()) { - std::exception_ptr ex = fut.get_exception(); - if (interrupt_condition->is_interruption(ex)) { - return seastar::futurize_invoke(std::move(func), std::move(ex)); - } else { - return seastar::make_exception_future(std::move(ex)); - } - } else { - return seastar::make_ready_future(fut.get()); - } - }); - } - template >>> @@ -410,6 +391,24 @@ public: return core_type::finally(std::forward(func)); } private: + template + [[gnu::always_inline]] + auto handle_interruption(Func&& func) { + return core_type::then_wrapped( + [func=std::move(func)](auto&& fut) mutable { + if (fut.failed()) { + std::exception_ptr ex = fut.get_exception(); + if (InterruptCond::is_interruption(ex)) { + return seastar::futurize_invoke(std::move(func), std::move(ex)); + } else { + return seastar::make_exception_future(std::move(ex)); + } + } else { + return seastar::make_ready_future(fut.get()); + } + }); + } + seastar::future to_future() { return static_cast(std::move(*this)); } @@ -428,24 +427,7 @@ private: std::move(fut)); }); } - template ::value, int>> - friend auto interruptor::do_for_each( - Iterator, Iterator, AsyncAction&&); - template ::value, int>> - friend auto interruptor::do_for_each( - Iterator, Iterator, AsyncAction&&); - template ::value, int>> - friend auto interruptor::repeat(AsyncAction&&); - template ::value, int>> - friend auto interruptor::repeat(AsyncAction&&); + friend interruptor; friend class interruptible_future_builder; template friend struct ::seastar::futurize; @@ -463,7 +445,6 @@ private: friend class ::crimson::maybe_handle_error_t; template friend class ::seastar::internal::extract_values_from_futures_vector; - friend class interruptor::parallel_for_each_state; template friend class interruptible_future_detail; template @@ -806,6 +787,7 @@ public: return (interrupt_futurize_t)(std::move(fut)); } +private: template [[gnu::always_inline]] auto handle_interruption(Func&& func) { @@ -823,7 +805,7 @@ public: -> typename futurator_t::type { if (fut.failed()) { std::exception_ptr ex = fut.get_exception(); - if (interrupt_condition->is_interruption(ex)) { + if (InterruptCond::is_interruption(ex)) { return futurator_t::invoke(std::move(func), std::move(ex)); } else { return futurator_t::make_exception_future(std::move(ex)); @@ -834,30 +816,12 @@ public: }); } -private: ErroratedFuture<::crimson::errorated_future_marker> to_future() { return static_cast(std::move(*this)); } - template ::value, int>> - friend auto interruptor::do_for_each( - Iterator, Iterator, AsyncAction&&); - template ::value, int>> - friend auto interruptor::do_for_each( - Iterator, Iterator, AsyncAction&&); - template ::value, int>> - friend auto interruptor::repeat(AsyncAction&&); - template ::value, int>> - friend auto interruptor::repeat(AsyncAction&&); + friend class interruptor; friend class interruptible_future_builder; template friend struct ::seastar::futurize; diff --git a/src/crimson/osd/pg_interval_interrupt_condition.h b/src/crimson/osd/pg_interval_interrupt_condition.h index 687f1d2b4cb..a59e479638c 100644 --- a/src/crimson/osd/pg_interval_interrupt_condition.h +++ b/src/crimson/osd/pg_interval_interrupt_condition.h @@ -41,7 +41,7 @@ public: std::is_same_v || std::is_same_v; - bool is_interruption(std::exception_ptr& eptr) { + static bool is_interruption(std::exception_ptr& eptr) { return (*eptr.__cxa_exception_type() == typeid(::crimson::common::actingset_changed) || *eptr.__cxa_exception_type() == diff --git a/src/test/crimson/test_interruptible_future.cc b/src/test/crimson/test_interruptible_future.cc index 2d927fc35d4..ddaeed80ed1 100644 --- a/src/test/crimson/test_interruptible_future.cc +++ b/src/test/crimson/test_interruptible_future.cc @@ -30,7 +30,7 @@ public: template static constexpr bool is_interruption_v = std::is_same_v; - bool is_interruption(std::exception_ptr& eptr) { + static bool is_interruption(std::exception_ptr& eptr) { if (*eptr.__cxa_exception_type() == typeid(test_interruption)) return true; return false; -- 2.39.5