From 0a83d956e546d7d04c55de34a788234533ed5293 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Thu, 19 Sep 2024 00:59:21 +0000 Subject: [PATCH] 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 --- .../osd/osd_operations/client_request.cc | 23 +++++++++++++------ .../osd/osd_operations/client_request.h | 7 +----- .../osd_operations/internal_client_request.cc | 2 ++ 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index 8e9a7c4d749..6eed04df6a5 100644 --- 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, 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>()); + }) ); co_await std::move(submitted); co_await ihref.enter_stage(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(); + }) + ); co_await ihref.enter_stage(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 331cedaadff..6ee57e9874c 100644 --- 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, int err); - - using do_process_iertr = - ::crimson::interruptible::interruptible_errorator< - ::crimson::osd::IOInterruptCondition, - ::crimson::errorator>; - do_process_iertr::future<> do_process( + interruptible_future<> do_process( instance_handle_t &ihref, Ref 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 2bfa4296b28..dabff1a33bd 100644 --- 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(); }) ); -- 2.39.5