crimson: remove the eagain error from PG::do_osd_ops

The idea here is that PG::do_osd_ops propogates an eagain after starting
a repair upon encountering an eio to indicate that the op should restart
from the top of ClientRequest::process_op.

However, InternalClientRequest's handler for this error simply ignores
it.  ClientRequest's handling, while superficially reasonable, doesn't
actually work.  Re-calling process_op would mean reentering previous
stages.  This is problematic for at least a few reasons:
1. Reentering a prior stage with the same handler doesn't actually work
   since the corresponding event entries will already be populated.
2. There might be other ops on the same object waiting on the process
   stage.  They'd need to be sent back as well in order to preserve
   ordering.

Because this mechanism doesn't really seem to be fully baked, let's
remove it for now and try to reintroduce it later after
do_osd_ops[_execute] are a bit simpler.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc
index 8e9a7c4d7490c69af82acc0b06fb14b9be878e4b..6eed04df6a5acb5388b716ef53dba44d540ea1d1 100644 (file)
--- a/src/crimson/osd/osd_operations/client_request.cc
+++ b/src/crimson/osd/osd_operations/client_request.cc
@@ -403,11 +403,6 @@ ClientRequest::process_op(
                   *pg, *this, this_instance_id);
          return do_process(
            ihref, pg, obc, this_instance_id
-         ).handle_error_interruptible(
-           crimson::ct_error::eagain::handle(
-             [this, pg, this_instance_id, &ihref]() mutable {
-               return process_op(ihref, pg, this_instance_id);
-             })
          );
        }
       );
@@ -437,7 +432,7 @@ ClientRequest::process_op(
   co_await std::move(process);
 }
 
-ClientRequest::do_process_iertr::future<>
+ClientRequest::interruptible_future<>
 ClientRequest::do_process(
   instance_handle_t &ihref,
   Ref<PG> pg, crimson::osd::ObjectContextRef obc,
@@ -509,12 +504,26 @@ ClientRequest::do_process(
 
   auto [submitted, all_completed] = co_await pg->do_osd_ops(
     m, r_conn, obc, op_info, snapc
+  ).handle_error_interruptible(
+    crimson::ct_error::eagain::handle([] {
+      ceph_assert(0 == "not handled");
+      return std::make_tuple(
+       interruptor::now(),
+       PG::do_osd_ops_iertr::make_ready_future<MURef<MOSDOpReply>>());
+    })
   );
   co_await std::move(submitted);
 
   co_await ihref.enter_stage<interruptor>(client_pp(*pg).wait_repop, *this);
 
-  auto reply = co_await std::move(all_completed);
+  auto reply = co_await std::move(
+    all_completed
+  ).handle_error_interruptible(
+    crimson::ct_error::eagain::handle([] {
+      ceph_assert(0 == "not handled");
+      return MURef<MOSDOpReply>();
+    })
+  );
 
   co_await ihref.enter_stage<interruptor>(client_pp(*pg).send_reply, *this);
   DEBUGDPP("{}.{}: sending response",

diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h
index 331cedaadfff2a3788c5c8024bcb477acbadc6a3..6ee57e9874cd1c3d39858a1e17df3eed7ab347b4 100644 (file)
--- a/src/crimson/osd/osd_operations/client_request.h
+++ b/src/crimson/osd/osd_operations/client_request.h
@@ -274,12 +274,7 @@ private:
   interruptible_future<> with_sequencer(FuncT&& func);
   interruptible_future<> reply_op_error(const Ref<PG>& pg, int err);
 
-
-  using do_process_iertr =
-    ::crimson::interruptible::interruptible_errorator<
-      ::crimson::osd::IOInterruptCondition,
-      ::crimson::errorator<crimson::ct_error::eagain>>;
-  do_process_iertr::future<> do_process(
+  interruptible_future<> do_process(
     instance_handle_t &ihref,
     Ref<PG> pg,
     crimson::osd::ObjectContextRef obc,

diff --git a/src/crimson/osd/osd_operations/internal_client_request.cc b/src/crimson/osd/osd_operations/internal_client_request.cc
index 2bfa4296b28291096d6b400efbd3e87d6fb82c04..dabff1a33bdb684d613b06690c77a5e6a5f40a6b 100644 (file)
--- a/src/crimson/osd/osd_operations/internal_client_request.cc
+++ b/src/crimson/osd/osd_operations/internal_client_request.cc
@@ -103,9 +103,11 @@ InternalClientRequest::with_interruption()
            [](auto submitted, auto all_completed) {
              return all_completed.handle_error_interruptible(
                crimson::ct_error::eagain::handle([] {
+                 ceph_assert(0 == "not handled");
                  return seastar::now();
                }));
            }, crimson::ct_error::eagain::handle([] {
+             ceph_assert(0 == "not handled");
              return interruptor::now();
            })
          );