From f90af12de37820cf4dd7e089b3771c0f3f772790 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Tue, 23 Jul 2024 10:45:47 +0800 Subject: [PATCH] crimson/osd/osd_operations/client_request: check already complete in the "check_already_complete_get_obc" phase Otherwise, the replies can go out-of-order Signed-off-by: Xuehan Xu --- .../osd/osd_operation_external_tracking.h | 12 ++++++++++++ .../osd/osd_operations/client_request.cc | 17 +++++++++++------ src/crimson/osd/osd_operations/client_request.h | 1 + .../osd/osd_operations/common/pg_pipeline.h | 3 +++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/crimson/osd/osd_operation_external_tracking.h b/src/crimson/osd/osd_operation_external_tracking.h index 5d3bba58db0c2..530732ba71028 100644 --- a/src/crimson/osd/osd_operation_external_tracking.h +++ b/src/crimson/osd/osd_operation_external_tracking.h @@ -35,6 +35,7 @@ struct LttngBackend ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend, ClientRequest::PGPipeline::RecoverMissing:: BlockingEvent::ExitBarrierEvent::Backend, + ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent::Backend, ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend, ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend, ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend, @@ -111,6 +112,11 @@ struct LttngBackend const Operation& op) override { } + void handle(ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent& ev, + const Operation& op, + const ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc& blocker) override { + } + void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev, const Operation& op, const ClientRequest::PGPipeline::GetOBC& blocker) override { @@ -164,6 +170,7 @@ struct HistoricBackend ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend, ClientRequest::PGPipeline::RecoverMissing:: BlockingEvent::ExitBarrierEvent::Backend, + ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent::Backend, ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend, ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend, ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend, @@ -240,6 +247,11 @@ struct HistoricBackend const Operation& op) override { } + void handle(ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent& ev, + const Operation& op, + const ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc& blocker) override { + } + void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev, const Operation& op, const ClientRequest::PGPipeline::GetOBC& blocker) override { diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index 4c3594108717a..07570d39883a7 100644 --- a/src/crimson/osd/osd_operations/client_request.cc +++ b/src/crimson/osd/osd_operations/client_request.cc @@ -337,7 +337,6 @@ ClientRequest::process_op( std::set snaps = snaps_need_to_recover(); if (!snaps.empty()) { - // call with_obc() in order, but wait concurrently for loading. auto with_obc = pg->obc_loader.with_obc( m->get_hobj().get_head(), [&snaps, &ihref, pg, this](auto head, auto) { @@ -350,6 +349,16 @@ ClientRequest::process_op( } } + /** + * The previous stage of recover_missing is a concurrent phase. + * Checking for already_complete requests must done exclusively. + * Since get_obc is also an exclusive stage, we can merge both stages into + * a single stage and avoid stage switching overhead. + */ + DEBUGDPP("{}.{}: entering check_already_complete_get_obc", + *pg, *this, this_instance_id); + co_await ihref.enter_stage( + client_pp(*pg).check_already_complete_get_obc, *this); DEBUGDPP("{}.{}: checking already_complete", *pg, *this, this_instance_id); auto completed = co_await pg->already_complete(m->get_reqid()); @@ -368,11 +377,7 @@ ClientRequest::process_op( co_return; } - DEBUGDPP("{}.{}: not completed, entering get_obc stage", - *pg, *this, this_instance_id); - co_await ihref.enter_stage(client_pp(*pg).get_obc, *this); - - DEBUGDPP("{}.{}: entered get_obc stage, about to wait_scrub", + DEBUGDPP("{}.{}: not completed, about to wait_scrub", *pg, *this, this_instance_id); co_await ihref.enter_blocker( *this, pg->scrubber, &decltype(pg->scrubber)::wait_scrub, diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h index c5ab670c78bd3..ea7aade22ac75 100644 --- a/src/crimson/osd/osd_operations/client_request.h +++ b/src/crimson/osd/osd_operations/client_request.h @@ -103,6 +103,7 @@ public: PGActivationBlocker::BlockingEvent, PGPipeline::RecoverMissing::BlockingEvent, scrub::PGScrubber::BlockingEvent, + PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent, PGPipeline::GetOBC::BlockingEvent, PGPipeline::LockOBC::BlockingEvent, PGPipeline::Process::BlockingEvent, diff --git a/src/crimson/osd/osd_operations/common/pg_pipeline.h b/src/crimson/osd/osd_operations/common/pg_pipeline.h index 5984a1399baeb..2b2d03ae4b3ed 100644 --- a/src/crimson/osd/osd_operations/common/pg_pipeline.h +++ b/src/crimson/osd/osd_operations/common/pg_pipeline.h @@ -20,6 +20,9 @@ protected: struct RecoverMissing : OrderedConcurrentPhaseT { static constexpr auto type_name = "CommonPGPipeline::recover_missing"; } recover_missing; + struct CheckAlreadyCompleteGetObc : OrderedExclusivePhaseT { + static constexpr auto type_name = "CommonPGPipeline::check_already_complete_get_obc"; + } check_already_complete_get_obc; struct GetOBC : OrderedExclusivePhaseT { static constexpr auto type_name = "CommonPGPipeline::get_obc"; } get_obc; -- 2.39.5