From: Samuel Just Date: Fri, 2 Sep 2022 21:53:33 +0000 (+0000) Subject: crimson/osd: remove compound_peering_request X-Git-Tag: v18.1.0~639^2~19^2~15 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=998cb8c141bb89aafae298a9d5e130fbd78fe5f2;p=ceph-ci.git crimson/osd: remove compound_peering_request The state shared between sub events creates problems for multicore. The only user is MOSDPGCreate2, so the optimization isn't really worth salvaging. Signed-off-by: Samuel Just --- diff --git a/src/crimson/osd/CMakeLists.txt b/src/crimson/osd/CMakeLists.txt index 779a5beca53..52cc33e3f95 100644 --- a/src/crimson/osd/CMakeLists.txt +++ b/src/crimson/osd/CMakeLists.txt @@ -16,7 +16,6 @@ add_executable(crimson-osd osd_operation.cc osd_operations/client_request.cc osd_operations/client_request_common.cc - osd_operations/compound_peering_request.cc osd_operations/internal_client_request.cc osd_operations/peering_event.cc osd_operations/pg_advance_map.cc diff --git a/src/crimson/osd/osd.cc b/src/crimson/osd/osd.cc index 49f510d73a8..f3a91085d13 100644 --- a/src/crimson/osd/osd.cc +++ b/src/crimson/osd/osd.cc @@ -21,6 +21,7 @@ #include "messages/MOSDMarkMeDown.h" #include "messages/MOSDOp.h" #include "messages/MOSDPeeringOp.h" +#include "messages/MOSDPGCreate2.h" #include "messages/MOSDPGUpdateLogMissing.h" #include "messages/MOSDPGUpdateLogMissingReply.h" #include "messages/MOSDRepOpReply.h" @@ -48,7 +49,6 @@ #include "crimson/osd/pg_backend.h" #include "crimson/osd/pg_meta.h" #include "crimson/osd/osd_operations/client_request.h" -#include "crimson/osd/osd_operations/compound_peering_request.h" #include "crimson/osd/osd_operations/peering_event.h" #include "crimson/osd/osd_operations/pg_advance_map.h" #include "crimson/osd/osd_operations/recovery_subrequest.h" @@ -708,10 +708,8 @@ OSD::ms_dispatch(crimson::net::ConnectionRef conn, MessageRef m) case CEPH_MSG_OSD_OP: return handle_osd_op(conn, boost::static_pointer_cast(m)); case MSG_OSD_PG_CREATE2: - get_shard_services().start_operation( - pg_shard_manager, - conn, - m); + return handle_pg_create( + conn, boost::static_pointer_cast(m)); return seastar::now(); case MSG_COMMAND: return handle_command(conn, boost::static_pointer_cast(m)); @@ -1018,6 +1016,41 @@ seastar::future<> OSD::handle_osd_op(crimson::net::ConnectionRef conn, return seastar::now(); } +seastar::future<> OSD::handle_pg_create(crimson::net::ConnectionRef conn, + Ref m) +{ + for (auto& [pgid, when] : m->pgs) { + const auto &[created, created_stamp] = when; + auto q = m->pg_extra.find(pgid); + ceph_assert(q != m->pg_extra.end()); + auto& [history, pi] = q->second; + logger().debug( + "{}: {} e{} @{} " + "history {} pi {}", + __func__, pgid, created, created_stamp, + history, pi); + if (!pi.empty() && + m->epoch < pi.get_bounds().second) { + logger().error( + "got pg_create on {} epoch {} " + "unmatched past_intervals {} (history {})", + pgid, m->epoch, + pi, history); + } else { + std::ignore = pg_shard_manager.start_pg_operation( + conn, + pg_shard_t(), + pgid, + m->epoch, + m->epoch, + NullEvt(), + true, + new PGCreateInfo(pgid, m->epoch, history, pi, true)); + } + } + return seastar::now(); +} + seastar::future<> OSD::handle_update_log_missing( crimson::net::ConnectionRef conn, Ref m) diff --git a/src/crimson/osd/osd.h b/src/crimson/osd/osd.h index c0179c90cfc..04d3251312e 100644 --- a/src/crimson/osd/osd.h +++ b/src/crimson/osd/osd.h @@ -169,6 +169,8 @@ private: seastar::future<> handle_osd_map(crimson::net::ConnectionRef conn, Ref m); + seastar::future<> handle_pg_create(crimson::net::ConnectionRef conn, + Ref m); seastar::future<> handle_osd_op(crimson::net::ConnectionRef conn, Ref m); seastar::future<> handle_rep_op(crimson::net::ConnectionRef conn, diff --git a/src/crimson/osd/osd_operation_external_tracking.h b/src/crimson/osd/osd_operation_external_tracking.h index a2eec6d39d5..19b115b8563 100644 --- a/src/crimson/osd/osd_operation_external_tracking.h +++ b/src/crimson/osd/osd_operation_external_tracking.h @@ -7,7 +7,6 @@ #include "crimson/osd/osdmap_gate.h" #include "crimson/osd/osd_operations/background_recovery.h" #include "crimson/osd/osd_operations/client_request.h" -#include "crimson/osd/osd_operations/compound_peering_request.h" #include "crimson/osd/osd_operations/peering_event.h" #include "crimson/osd/osd_operations/pg_advance_map.h" #include "crimson/osd/osd_operations/recovery_subrequest.h" @@ -118,23 +117,6 @@ struct LttngBackend const Operation&) override {} }; -struct LttngBackendCompoundPeering - : CompoundPeeringRequest::StartEvent::Backend, - CompoundPeeringRequest::SubOpBlocker::BlockingEvent::Backend, - CompoundPeeringRequest::CompletionEvent::Backend -{ - void handle(CompoundPeeringRequest::StartEvent&, - const Operation&) override {} - - void handle(CompoundPeeringRequest::SubOpBlocker::BlockingEvent& ev, - const Operation& op, - const CompoundPeeringRequest::SubOpBlocker& blocker) override { - } - - void handle(CompoundPeeringRequest::CompletionEvent&, - const Operation&) override {} -}; - struct HistoricBackend : ClientRequest::StartEvent::Backend, ConnectionPipeline::AwaitActive::BlockingEvent::Backend, @@ -300,13 +282,6 @@ struct EventBackendRegistry { } }; -template <> -struct EventBackendRegistry { - static std::tuple get_backends() { - return { {} }; - } -}; - template <> struct EventBackendRegistry { static std::tuple<> get_backends() { diff --git a/src/crimson/osd/osd_operations/compound_peering_request.cc b/src/crimson/osd/osd_operations/compound_peering_request.cc deleted file mode 100644 index a30b03b1a01..00000000000 --- a/src/crimson/osd/osd_operations/compound_peering_request.cc +++ /dev/null @@ -1,158 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- -// vim: ts=8 sw=2 smarttab - -#include - -#include "osd/PeeringState.h" - -#include "messages/MOSDPGCreate2.h" - -#include "common/Formatter.h" - -#include "crimson/common/exception.h" -#include "crimson/osd/pg.h" -#include "crimson/osd/pg_shard_manager.h" -#include "crimson/osd/osd_operation_external_tracking.h" -#include "crimson/osd/osd_operations/compound_peering_request.h" - -namespace { - seastar::logger& logger() { - return crimson::get_logger(ceph_subsys_osd); - } -} - -namespace { -using namespace crimson::osd; - -struct compound_state { - seastar::promise promise; - // assuming crimson-osd won't need to be compatible with pre-octopus - // releases - BufferedRecoveryMessages ctx; - compound_state() = default; - ~compound_state() { - promise.set_value(std::move(ctx)); - } -}; -using compound_state_ref = seastar::lw_shared_ptr; - -class PeeringSubEvent : public RemotePeeringEvent { - compound_state_ref state; -public: - template - PeeringSubEvent(compound_state_ref state, Args &&... args) : - RemotePeeringEvent(std::forward(args)...), state(state) {} - - PeeringEvent::interruptible_future<> - complete_rctx( - ShardServices &shard_services, - Ref pg) final { - logger().debug("{}: submitting ctx transaction", *this); - state->ctx.accept_buffered_messages(ctx); - state = {}; - if (!pg) { - ceph_assert(ctx.transaction.empty()); - return seastar::now(); - } else { - return shard_services.dispatch_context_transaction( - pg->get_collection_ref(), ctx); - } - } -}; - -std::vector handle_pg_create( - PGShardManager &pg_shard_manager, - crimson::net::ConnectionRef conn, - compound_state_ref state, - Ref m) -{ - std::vector ret; - for (auto& [pgid, when] : m->pgs) { - const auto &[created, created_stamp] = when; - auto q = m->pg_extra.find(pgid); - ceph_assert(q != m->pg_extra.end()); - auto& [history, pi] = q->second; - logger().debug( - "{}: {} e{} @{} " - "history {} pi {}", - __func__, pgid, created, created_stamp, - history, pi); - if (!pi.empty() && - m->epoch < pi.get_bounds().second) { - logger().error( - "got pg_create on {} epoch {} " - "unmatched past_intervals {} (history {})", - pgid, m->epoch, - pi, history); - } else { - auto [op_id, fut] = pg_shard_manager.start_pg_operation( - state, - conn, - pg_shard_t(), - pgid, - m->epoch, - m->epoch, - NullEvt(), - true, - new PGCreateInfo(pgid, m->epoch, history, pi, true)); - ret.push_back(op_id); - } - } - return ret; -} - -} // namespace - -namespace crimson::osd { - -CompoundPeeringRequest::CompoundPeeringRequest( - PGShardManager &pg_shard_manager, - crimson::net::ConnectionRef conn, Ref m) - : pg_shard_manager(pg_shard_manager), - conn(conn), - m(m) -{} - -void CompoundPeeringRequest::print(std::ostream &lhs) const -{ - lhs << *m; -} - -void CompoundPeeringRequest::dump_detail(Formatter *f) const -{ - f->dump_stream("message") << *m; -} - -seastar::future<> CompoundPeeringRequest::start() -{ - logger().info("{}: starting", *this); - track_event(); - auto state = seastar::make_lw_shared(); - auto blocker = std::make_unique( - [&] { - assert((m->get_type() == MSG_OSD_PG_CREATE2)); - return handle_pg_create( - pg_shard_manager, - conn, - state, - boost::static_pointer_cast(m)); - }()); - - IRef ref = this; - logger().info("{}: about to fork future", *this); - return crimson::common::handle_system_shutdown( - [this, ref, blocker=std::move(blocker), state]() mutable { - return with_blocking_event([&] (auto&& trigger) { - return trigger.maybe_record_blocking(state->promise.get_future(), *blocker); - }).then([this, blocker=std::move(blocker)](auto &&ctx) { - logger().info("{}: sub events complete", *this); - return pg_shard_manager.get_shard_services( - ).dispatch_context_messages(std::move(ctx)); - }).then([this, ref=std::move(ref)] { - track_event(); - logger().info("{}: complete", *this); - }); - }); -} - -} // namespace crimson::osd diff --git a/src/crimson/osd/osd_operations/compound_peering_request.h b/src/crimson/osd/osd_operations/compound_peering_request.h deleted file mode 100644 index 4fbc3ffb278..00000000000 --- a/src/crimson/osd/osd_operations/compound_peering_request.h +++ /dev/null @@ -1,65 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- -// vim: ts=8 sw=2 smarttab - -#pragma once - -#include -#include - -#include "msg/MessageRef.h" - -#include "crimson/net/Connection.h" -#include "crimson/osd/osd_operation.h" - -namespace crimson::osd { - -class PGShardManager; -class PG; - -using osd_id_t = int; - -class CompoundPeeringRequest : public TrackableOperationT { - friend class LttngBackendCompoundPeering; -public: - static constexpr OperationTypeCode type = - OperationTypeCode::compound_peering_request; - - struct SubOpBlocker : crimson::BlockerT { - static constexpr const char * type_name = "CompoundOpBlocker"; - - std::vector subops; - SubOpBlocker(std::vector &&subops) - : subops(subops) {} - - virtual void dump_detail(Formatter *f) const { - f->open_array_section("dependent_operations"); - { - for (auto &i : subops) { - f->dump_unsigned("op_id", i); - } - } - f->close_section(); - } - }; - -private: - PGShardManager &pg_shard_manager; - crimson::net::ConnectionRef conn; - Ref m; - -public: - CompoundPeeringRequest( - PGShardManager &pg_shard_manager, crimson::net::ConnectionRef conn, Ref m); - - void print(std::ostream &) const final; - void dump_detail(Formatter *f) const final; - seastar::future<> start(); - - std::tuple< - StartEvent, - SubOpBlocker::BlockingEvent, - CompletionEvent - > tracking_events; -}; - -}