From ab849a13782e5fe206e4206c8495e0c3e29266cf Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Tue, 8 Sep 2020 12:35:14 +0800 Subject: [PATCH 1/3] crimson/osd: rename is_valid_rep_op_reply to can_discard_replica_op Signed-off-by: Xuehan Xu --- src/crimson/osd/pg.cc | 21 +++++++++++---------- src/crimson/osd/pg.h | 3 ++- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index a2a4f528bb3..f1358ce4dfd 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -931,12 +931,13 @@ seastar::future<> PG::handle_rep_op(Ref req) void PG::handle_rep_op_reply(crimson::net::Connection* conn, const MOSDRepOpReply& m) { - if (is_valid_rep_op_reply(m)) { + if (!can_discard_replica_op(m)) { backend->got_rep_op_reply(m); } } -bool PG::is_valid_rep_op_reply(const MOSDRepOpReply& reply) const +template +bool PG::can_discard_replica_op(const MsgType& m) const { // if a repop is replied after a replica goes down in a new osdmap, and // before the pg advances to this new osdmap, the repop replies before this @@ -945,27 +946,27 @@ bool PG::is_valid_rep_op_reply(const MOSDRepOpReply& reply) const // resets the messenger sesssion when the replica reconnects. to avoid the // out-of-order replies, the messages from that replica should be discarded. const auto osdmap = peering_state.get_osdmap(); - const int from_osd = reply.get_source().num(); + const int from_osd = m.get_source().num(); if (osdmap->is_down(from_osd)) { - return false; + return true; } // Mostly, this overlaps with the old_peering_msg // condition. An important exception is pushes // sent by replicas not in the acting set, since // if such a replica goes down it does not cause // a new interval. - if (osdmap->get_down_at(from_osd) >= reply.map_epoch) { - return false; + if (osdmap->get_down_at(from_osd) >= m.map_epoch) { + return true; } // same pg? // if pg changes *at all*, we reset and repeer! if (epoch_t lpr = peering_state.get_last_peering_reset(); - lpr > reply.map_epoch) { + lpr > m.map_epoch) { logger().debug("{}: pg changed {} after {}, dropping", - __func__, get_info().history, reply.map_epoch); - return false; + __func__, get_info().history, m.map_epoch); + return true; } - return true; + return false; } seastar::future<> PG::stop() diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h index 0b6579c04ab..7bfc477b0a2 100644 --- a/src/crimson/osd/pg.h +++ b/src/crimson/osd/pg.h @@ -670,7 +670,8 @@ private: return seastar::make_ready_future(true); } - bool is_valid_rep_op_reply(const MOSDRepOpReply& reply) const; + template + bool can_discard_replica_op(const MsgType& m) const; bool is_missing_object(const hobject_t& soid) const { return peering_state.get_pg_log().get_missing().get_items().count(soid); } From e9ac113f4f5169ed4ca32f90426446e69a2c453c Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Tue, 8 Sep 2020 12:36:06 +0800 Subject: [PATCH 2/3] crimson/osd: drop repop if it can be discarded Signed-off-by: Xuehan Xu --- src/crimson/osd/pg.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index f1358ce4dfd..ca99a4b9996 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -908,6 +908,10 @@ seastar::future<> PG::handle_rep_op(Ref req) crimson::common::system_shutdown_exception()); } + if (can_discard_replica_op(*req)) { + return seastar::now(); + } + ceph::os::Transaction txn; auto encoded_txn = req->get_data().cbegin(); decode(txn, encoded_txn); From 2c17bc1933d3e06a4d966e6a074d66213d3c61ed Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Tue, 8 Sep 2020 12:36:41 +0800 Subject: [PATCH 3/3] crimson/osd: add can_discard_op method for detecting "discardable" client requests Signed-off-by: Xuehan Xu --- src/crimson/osd/osd_operations/client_request.cc | 4 +--- src/crimson/osd/pg.cc | 5 +++++ src/crimson/osd/pg.h | 1 + 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index a6e6d1e682a..a51cc7256d6 100644 --- a/src/crimson/osd/osd_operations/client_request.cc +++ b/src/crimson/osd/osd_operations/client_request.cc @@ -69,9 +69,7 @@ seastar::future<> ClientRequest::start() return with_blocking_future(osd.wait_for_pg(m->get_spg())); }).then([this, opref](Ref pgref) { PG &pg = *pgref; - if (__builtin_expect(m->get_map_epoch() - < pg.get_info().history.same_primary_since, - false)) { + if (pg.can_discard_op(*m)) { return osd.send_incremental_map(conn.get(), m->get_map_epoch()); } return with_blocking_future( diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index ca99a4b9996..3abe129c2fe 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -993,4 +993,9 @@ void PG::on_change(ceph::os::Transaction &t) { backend->on_actingset_changed({ is_primary() }); } +bool PG::can_discard_op(const MOSDOp& m) const { + return __builtin_expect(m.get_map_epoch() + < peering_state.get_info().history.same_primary_since, false); +} + } diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h index 7bfc477b0a2..7cdd054c33a 100644 --- a/src/crimson/osd/pg.h +++ b/src/crimson/osd/pg.h @@ -672,6 +672,7 @@ private: template bool can_discard_replica_op(const MsgType& m) const; + bool can_discard_op(const MOSDOp& m) const; bool is_missing_object(const hobject_t& soid) const { return peering_state.get_pg_log().get_missing().get_items().count(soid); }