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 <sjust@redhat.com>

diff --git a/src/crimson/common/exception.h b/src/crimson/common/exception.h
index dafb21a941976c115a8556768dccd6063d526e7f..682fef69b7e259471ad43649838999a402b33388 100644 (file)
--- a/src/crimson/common/exception.h
+++ b/src/crimson/common/exception.h
@@ -35,25 +35,6 @@ private:
   const bool still_primary;
 };
 
-template <typename InterruptCond, bool may_loop,
-         typename... InterruptExceptions,
-         typename OpFunc, typename OnInterrupt>
-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<InterruptCond>::template futurize<
-       std::result_of_t<OpFunc()>>::apply(std::move(opfunc), std::make_tuple())
-       .template handle_interruption<InterruptExceptions...>(std::move(efunc));
-    });
-  } else {
-      return ::crimson::interruptible::interruptor<InterruptCond>::template futurize<
-       std::result_of_t<OpFunc()>>::apply(std::move(opfunc), std::make_tuple())
-       .template handle_interruption<InterruptExceptions...>(std::move(efunc));
-  }
-}
-
 template<typename Func, typename... Args>
 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 a1f53dafd18d716ae5be2e92d0af2a12f71fe6e0..643098624cc6a7ef6a777390fd89f74db81b10fe 100644 (file)
--- 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 <typename Func>
-  [[gnu::always_inline]]
-  auto handle_interruption(Func&& func) {
-    return core_type::then_wrapped(
-      [func=std::move(func),
-      interrupt_condition=interrupt_cond<InterruptCond>](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<T>(std::move(ex));
-       }
-      } else {
-       return seastar::make_ready_future<T>(fut.get());
-      }
-    });
-  }
-
   template <typename Func,
            typename Result = interrupt_futurize_t<
                std::invoke_result_t<Func, seastar::future<T>>>>
@@ -410,6 +391,24 @@ public:
     return core_type::finally(std::forward<Func>(func));
   }
 private:
+  template <typename Func>
+  [[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<T>(std::move(ex));
+         }
+       } else {
+         return seastar::make_ready_future<T>(fut.get());
+       }
+      });
+  }
+
   seastar::future<T> to_future() {
     return static_cast<core_type&&>(std::move(*this));
   }
@@ -428,24 +427,7 @@ private:
                std::move(fut));
     });
   }
-  template <typename Iterator, typename AsyncAction, typename Result,
-           std::enable_if_t<
-             is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::do_for_each(
-           Iterator, Iterator, AsyncAction&&);
-  template <typename Iterator, typename AsyncAction, typename Result,
-           std::enable_if_t<
-             !is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::do_for_each(
-           Iterator, Iterator, AsyncAction&&);
-  template <typename AsyncAction, typename Result,
-           std::enable_if_t<
-             is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::repeat(AsyncAction&&);
-  template <typename AsyncAction, typename Result,
-           std::enable_if_t<
-             !is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::repeat(AsyncAction&&);
+  friend interruptor<InterruptCond>;
   friend class interruptible_future_builder<InterruptCond>;
   template <typename U>
   friend struct ::seastar::futurize;
@@ -463,7 +445,6 @@ private:
   friend class ::crimson::maybe_handle_error_t;
   template <typename>
   friend class ::seastar::internal::extract_values_from_futures_vector;
-  friend class interruptor<InterruptCond>::parallel_for_each_state;
   template <typename, typename>
   friend class interruptible_future_detail;
   template <typename ResolvedVectorTransform, typename Future>
@@ -806,6 +787,7 @@ public:
     return (interrupt_futurize_t<decltype(fut)>)(std::move(fut));
   }
 
+private:
   template <typename Func>
   [[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<T>>
   to_future() {
     return static_cast<core_type&&>(std::move(*this));
   }
 
-  template <typename Iterator, typename AsyncAction, typename Result,
-           std::enable_if_t<
-             is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::do_for_each(
-           Iterator, Iterator, AsyncAction&&);
-  template <typename Iterator, typename AsyncAction, typename Result,
-           std::enable_if_t<
-             !is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::do_for_each(
-           Iterator, Iterator, AsyncAction&&);
-  template <typename AsyncAction, typename Result,
-           std::enable_if_t<
-             is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::repeat(AsyncAction&&);
-  template <typename AsyncAction, typename Result,
-           std::enable_if_t<
-             !is_interruptible_future<Result>::value, int>>
-  friend auto interruptor<InterruptCond>::repeat(AsyncAction&&);
+  friend class interruptor<InterruptCond>;
   friend class interruptible_future_builder<InterruptCond>;
   template <typename U>
   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 687f1d2b4cb661d241db976b95ae5d3ba24997eb..a59e479638ca754dee6b0a8f3980325a53c8501d 100644 (file)
--- 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<T, ::crimson::common::actingset_changed>
     || std::is_same_v<T, ::crimson::common::system_shutdown_exception>;
 
-  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 2d927fc35d433b0f4e5542b29a95ce4831f58c92..ddaeed80ed13597e2cdd9e5071ded37c2cb04ef1 100644 (file)
--- a/src/test/crimson/test_interruptible_future.cc
+++ b/src/test/crimson/test_interruptible_future.cc
@@ -30,7 +30,7 @@ public:
   template <typename T>
   static constexpr bool is_interruption_v = std::is_same_v<T, test_interruption>;
 
-  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;