From 2fbe40e72e8352a3bf47190d7bc8c80bb60eb7bd Mon Sep 17 00:00:00 2001 From: Leonid Usov Date: Tue, 27 Feb 2024 23:25:20 +0200 Subject: [PATCH] messages: avoid using mutable members in MMDSQuiesce* Signed-off-by: Leonid Usov --- src/mds/MDSRankQuiesce.cc | 81 ++++++++++++++++++----------- src/messages/MMDSQuiesceDbAck.h | 17 +++--- src/messages/MMDSQuiesceDbListing.h | 18 ++++--- 3 files changed, 73 insertions(+), 43 deletions(-) diff --git a/src/mds/MDSRankQuiesce.cc b/src/mds/MDSRankQuiesce.cc index 820cd33fcff..5d92c0bd818 100644 --- a/src/mds/MDSRankQuiesce.cc +++ b/src/mds/MDSRankQuiesce.cc @@ -288,9 +288,9 @@ void MDSRank::quiesce_cluster_update() { } auto addrs = mdsmap->get_info_gid(leader).addrs; - auto ack_msg = make_message(me); + auto ack_msg = make_message(); dout(10) << "sending ack " << ack << " to the leader " << leader << dendl; - ack_msg->diff_map = std::move(ack); + ack_msg->encode_payload_from(me, ack); return send_message_mds(ack_msg, addrs); } }; @@ -302,9 +302,9 @@ void MDSRank::quiesce_cluster_update() { return -ENOENT; } auto addrs = mdsmap->get_info_gid(to).addrs; - auto listing_msg = make_message(me); + auto listing_msg = make_message(); dout(10) << "sending listing " << db << " to the peer " << to << dendl; - listing_msg->db_listing = std::move(db); + listing_msg->encode_payload_from(me, db); return send_message_mds(listing_msg, addrs); }; } @@ -374,38 +374,59 @@ void MDSRank::quiesce_cluster_update() { } bool MDSRank::quiesce_dispatch(const cref_t &m) { - switch(m->get_type()) { - case MSG_MDS_QUIESCE_DB_LISTING: - { - const auto& req = ref_cast(m); - if (quiesce_db_manager) { - dout(10) << "got " << req->db_listing << " from peer " << req->gid << dendl; - int result = quiesce_db_manager->submit_listing_from(req->gid, std::move(req->db_listing)); - if (result != 0) { - dout(3) << "error (" << result << ") submitting " << req->db_listing << " from peer " << req->gid << dendl; + try { + switch(m->get_type()) { + case MSG_MDS_QUIESCE_DB_LISTING: + { + const auto& req = ref_cast(m); + mds_gid_t gid; + QuiesceDbListing db_listing; + req->decode_payload_into(gid, db_listing); + if (quiesce_db_manager) { + dout(10) << "got " << db_listing << " from peer " << gid << dendl; + int result = quiesce_db_manager->submit_listing_from(gid, std::move(db_listing)); + if (result != 0) { + dout(3) << "error (" << result << ") submitting " << db_listing << " from peer " << gid << dendl; + } + } else { + dout(5) << "no db manager to process " << db_listing << dendl; } - } else { - dout(5) << "no db manager to process " << req->db_listing << dendl; + return true; } - return true; - } - case MSG_MDS_QUIESCE_DB_ACK: - { - const auto& req = ref_cast(m); - if (quiesce_db_manager) { - dout(10) << "got ack " << req->diff_map << " from peer " << req->gid << dendl; - int result = quiesce_db_manager->submit_ack_from(req->gid, std::move(req->diff_map)); - if (result != 0) { - dout(3) << "error (" << result << ") submitting an ack from peer " << req->gid << dendl; + case MSG_MDS_QUIESCE_DB_ACK: + { + const auto& req = ref_cast(m); + mds_gid_t gid; + QuiesceMap diff_map; + req->decode_payload_into(gid, diff_map); + if (quiesce_db_manager) { + dout(10) << "got ack " << diff_map << " from peer " << gid << dendl; + int result = quiesce_db_manager->submit_ack_from(gid, std::move(diff_map)); + if (result != 0) { + dout(3) << "error (" << result << ") submitting an ack from peer " << gid << dendl; + } + } else { + dout(5) << "no db manager to process an ack: " << diff_map << dendl; } - } else { - dout(5) << "no db manager to process an ack: " << req->diff_map << dendl; + return true; + } + default: break; + } + } + catch (const ceph::buffer::error &e) { + if (cct) { + dout(-1) << "failed to decode message of type " << m->get_type() + << " v" << m->get_header().version + << ": " << e.what() << dendl; + dout(10) << "dump: \n"; + m->get_payload().hexdump(*_dout); + *_dout << dendl; + if (cct->_conf->ms_die_on_bad_msg) { + ceph_abort(); } - return true; } - default: - return false; } + return false; } void MDSRank::quiesce_agent_setup() { diff --git a/src/messages/MMDSQuiesceDbAck.h b/src/messages/MMDSQuiesceDbAck.h index 907db239bd1..1d56451e89b 100644 --- a/src/messages/MMDSQuiesceDbAck.h +++ b/src/messages/MMDSQuiesceDbAck.h @@ -19,13 +19,8 @@ #include "mds/QuiesceDbEncoding.h" class MMDSQuiesceDbAck final : public MMDSOp { -public: - mds_gid_t gid; - mutable QuiesceMap diff_map; - protected: - MMDSQuiesceDbAck(mds_gid_t gid) : MMDSOp{MSG_MDS_QUIESCE_DB_ACK}, gid(gid) {} - MMDSQuiesceDbAck() : MMDSQuiesceDbAck(MDS_GID_NONE) {} + MMDSQuiesceDbAck() : MMDSOp{MSG_MDS_QUIESCE_DB_ACK} {} ~MMDSQuiesceDbAck() final {} public: @@ -35,6 +30,11 @@ public: } void encode_payload(uint64_t features) override + { + // noop to prevent unnecessary overheads + } + + void encode_payload_from(mds_gid_t const&gid, QuiesceMap const&diff_map) { using ceph::encode; @@ -47,6 +47,11 @@ public: } void decode_payload() override { + // noop to prevent unnecessary overheads + } + + void decode_payload_into(mds_gid_t &gid, QuiesceMap &diff_map) const + { using ceph::decode; auto p = payload.cbegin(); DECODE_START(1, p); diff --git a/src/messages/MMDSQuiesceDbListing.h b/src/messages/MMDSQuiesceDbListing.h index 9d3ce20fdab..f57de50e22f 100644 --- a/src/messages/MMDSQuiesceDbListing.h +++ b/src/messages/MMDSQuiesceDbListing.h @@ -19,13 +19,8 @@ #include "mds/QuiesceDbEncoding.h" class MMDSQuiesceDbListing final : public MMDSOp { -public: - mds_gid_t gid; - mutable QuiesceDbListing db_listing; - protected: - MMDSQuiesceDbListing(mds_gid_t gid) : MMDSOp{MSG_MDS_QUIESCE_DB_LISTING}, gid(gid) {} - MMDSQuiesceDbListing() : MMDSQuiesceDbListing(MDS_GID_NONE) {} + MMDSQuiesceDbListing() : MMDSOp{MSG_MDS_QUIESCE_DB_LISTING} {} ~MMDSQuiesceDbListing() final {} public: @@ -34,7 +29,11 @@ public: } - void encode_payload(uint64_t features) override + void encode_payload(uint64_t features) override { + // noop to prevent unnecessary overheads + } + + void encode_payload_from(mds_gid_t const& gid, QuiesceDbListing const& db_listing) { using ceph::encode; @@ -47,6 +46,11 @@ public: } void decode_payload() override { + // noop to prevent unnecessary overheads + } + + void decode_payload_into(mds_gid_t &gid, QuiesceDbListing &db_listing) const + { using ceph::decode; auto p = payload.cbegin(); DECODE_START(1, p); -- 2.39.5