From: Sage Weil Date: Wed, 7 Feb 2018 23:05:46 +0000 (-0600) Subject: osd/PGBackend: drop on_applied callback for submit_transaction X-Git-Tag: wip-pdonnell-testing-20180317.202121~265^2~9 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=01b68f969cdf6925275a90a31c2bba4e7951ff43;p=ceph-ci.git osd/PGBackend: drop on_applied callback for submit_transaction This removes a ton of tracking for ReplicatedBackend. ECBackend needs to keep most of it so that it can track in-flight applies on legacy peer OSDs. We can remove this post-nautilus. Signed-off-by: Sage Weil --- diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc index 1b65ac3568f..9ee4107e3b1 100644 --- a/src/osd/ECBackend.cc +++ b/src/osd/ECBackend.cc @@ -1071,12 +1071,6 @@ void ECBackend::handle_sub_write_reply( i->second.pending_apply.erase(from); } - if (i->second.pending_apply.empty() && i->second.on_all_applied) { - dout(10) << __func__ << " Calling on_all_applied on " << i->second << dendl; - i->second.on_all_applied->complete(0); - i->second.on_all_applied = 0; - i->second.trace.event("ec write all applied"); - } if (i->second.pending_commit.empty() && i->second.on_all_commit) { dout(10) << __func__ << " Calling on_all_commit on " << i->second << dendl; i->second.on_all_commit->complete(0); @@ -1413,7 +1407,6 @@ void ECBackend::submit_transaction( const eversion_t &roll_forward_to, const vector &log_entries, boost::optional &hset_history, - Context *on_all_applied, Context *on_all_commit, ceph_tid_t tid, osd_reqid_t reqid, @@ -1429,7 +1422,6 @@ void ECBackend::submit_transaction( op->roll_forward_to = std::max(roll_forward_to, committed_to); op->log_entries = log_entries; std::swap(op->updated_hit_set_history, hset_history); - op->on_all_applied = on_all_applied; op->on_all_commit = on_all_commit; op->tid = tid; op->reqid = reqid; diff --git a/src/osd/ECBackend.h b/src/osd/ECBackend.h index 55f07866374..841493d77c4 100644 --- a/src/osd/ECBackend.h +++ b/src/osd/ECBackend.h @@ -105,7 +105,6 @@ public: const eversion_t &roll_forward_to, const vector &log_entries, boost::optional &hset_history, - Context *on_all_applied, Context *on_all_commit, ceph_tid_t tid, osd_reqid_t reqid, @@ -484,8 +483,11 @@ public: return !remote_read.empty() && remote_read_result.empty(); } - /// In progress write state + /// In progress write state. set pending_commit; + // we need pending_apply for pre-mimic peers so that we don't issue a + // read on a remote shard before it has applied a previous write. We can + // remove this after nautilus. set pending_apply; bool write_in_progress() const { return !pending_commit.empty() || !pending_apply.empty(); @@ -498,10 +500,8 @@ public: ExtentCache::write_pin pin; /// Callbacks - Context *on_all_applied = nullptr; Context *on_all_commit = nullptr; ~Op() { - delete on_all_applied; delete on_all_commit; } }; diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h index 158c358a714..68da2e89b6f 100644 --- a/src/osd/PGBackend.h +++ b/src/osd/PGBackend.h @@ -216,9 +216,6 @@ typedef ceph::shared_ptr OSDMapRef; virtual void release_locks(ObcLockManager &manager) = 0; - virtual void op_applied( - const eversion_t &applied_version) = 0; - virtual bool should_send_op( pg_shard_t peer, const hobject_t &hoid) = 0; @@ -438,7 +435,6 @@ typedef ceph::shared_ptr OSDMapRef; const vector &log_entries, ///< [in] log entries for t /// [in] hitset history (if updated with this transaction) boost::optional &hset_history, - Context *on_all_applied, ///< [in] called when all acked Context *on_all_commit, ///< [in] called when all commit ceph_tid_t tid, ///< [in] tid osd_reqid_t reqid, ///< [in] reqid diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index 6b62ae3f984..dccd8cc164f 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -10022,7 +10022,6 @@ void PrimaryLogPG::issue_repop(RepGather *repop, OpContext *ctx) min_last_complete_ondisk, ctx->log, ctx->updated_hset_history, - nullptr, on_all_commit, repop->rep_tid, ctx->reqid, diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc index 5138627ffd7..83e5d5f7b54 100644 --- a/src/osd/ReplicatedBackend.cc +++ b/src/osd/ReplicatedBackend.cc @@ -237,7 +237,6 @@ void ReplicatedBackend::on_change() dout(10) << __func__ << dendl; for (auto& op : in_progress_ops) { delete op.second.on_commit; - delete op.second.on_applied; } in_progress_ops.clear(); clear_recovery_state(); @@ -428,7 +427,6 @@ void ReplicatedBackend::submit_transaction( const eversion_t &roll_forward_to, const vector &_log_entries, boost::optional &hset_history, - Context *on_all_acked, Context *on_all_commit, ceph_tid_t tid, osd_reqid_t reqid, @@ -456,16 +454,13 @@ void ReplicatedBackend::submit_transaction( make_pair( tid, InProgressOp( - tid, on_all_commit, on_all_acked, + tid, on_all_commit, orig_op, at_version) ) ); assert(insert_res.second); InProgressOp &op = insert_res.first->second; - op.waiting_for_applied.insert( - parent->get_actingbackfill_shards().begin(), - parent->get_actingbackfill_shards().end()); op.waiting_for_commit.insert( parent->get_actingbackfill_shards().begin(), parent->get_actingbackfill_shards().end()); @@ -503,30 +498,6 @@ void ReplicatedBackend::submit_transaction( tls.push_back(std::move(op_t)); parent->queue_transactions(tls, op.op); - op_applied(&op); -} - -void ReplicatedBackend::op_applied( - InProgressOp *op) -{ - FUNCTRACE(cct); - OID_EVENT_TRACE_WITH_MSG((op && op->op) ? op->op->get_req() : NULL, "OP_APPLIED_BEGIN", true); - dout(10) << __func__ << ": " << op->tid << dendl; - if (op->op) { - op->op->mark_event("op_applied"); - op->op->pg_trace.event("op applied"); - } - - op->waiting_for_applied.erase(get_parent()->whoami_shard()); - - if (op->waiting_for_applied.empty()) { - op->on_applied->complete(0); - op->on_applied = 0; - } - if (op->done()) { - assert(!op->on_commit && !op->on_applied); - in_progress_ops.erase(op->tid); - } } void ReplicatedBackend::op_commit( @@ -545,9 +516,7 @@ void ReplicatedBackend::op_commit( if (op->waiting_for_commit.empty()) { op->on_commit->complete(0); op->on_commit = 0; - } - if (op->done()) { - assert(!op->on_commit && !op->on_applied); + assert(!op->on_commit); in_progress_ops.erase(op->tid); } } @@ -594,32 +563,17 @@ void ReplicatedBackend::do_repop_reply(OpRequestRef op) ip_op.op->pg_trace.event("sub_op_commit_rec"); } } else { - assert(ip_op.waiting_for_applied.count(from)); - if (ip_op.op) { - ostringstream ss; - ss << "sub_op_applied_rec from " << from; - ip_op.op->mark_event_string(ss.str()); - ip_op.op->pg_trace.event("sub_op_applied_rec"); - } + // legacy peer; ignore } - ip_op.waiting_for_applied.erase(from); parent->update_peer_last_complete_ondisk( from, r->get_last_complete_ondisk()); - if (ip_op.waiting_for_applied.empty() && - ip_op.on_applied) { - ip_op.on_applied->complete(0); - ip_op.on_applied = 0; - } if (ip_op.waiting_for_commit.empty() && ip_op.on_commit) { ip_op.on_commit->complete(0); - ip_op.on_commit= 0; - } - if (ip_op.done()) { - assert(!ip_op.on_commit && !ip_op.on_applied); + ip_op.on_commit = 0; in_progress_ops.erase(iter); } } @@ -1120,35 +1074,9 @@ void ReplicatedBackend::do_repop(OpRequestRef op) tls.push_back(std::move(rm->localt)); tls.push_back(std::move(rm->opt)); parent->queue_transactions(tls, op); - repop_applied(rm); // op is cleaned up by oncommit/onapply when both are executed } -void ReplicatedBackend::repop_applied(RepModifyRef rm) -{ - rm->op->mark_event("sub_op_applied"); - rm->applied = true; - rm->op->pg_trace.event("sup_op_applied"); - - dout(10) << __func__ << " on " << rm << " op " - << *rm->op->get_req() << dendl; - const Message *m = rm->op->get_req(); - const MOSDRepOp *req = static_cast(m); - eversion_t version = req->version; - - // send ack to acker only if we haven't sent a commit already - if (!rm->committed) { - Message *ack = new MOSDRepOpReply( - req, parent->whoami_shard(), - 0, get_osdmap()->get_epoch(), req->min_epoch, CEPH_OSD_FLAG_ACK); - ack->set_priority(CEPH_MSG_PRIO_HIGH); // this better match commit priority! - ack->trace = rm->op->pg_trace; - get_parent()->send_message_osd_cluster( - rm->ackerosd, ack, get_osdmap()->get_epoch()); - } - -} - void ReplicatedBackend::repop_commit(RepModifyRef rm) { rm->op->mark_commit_sent(); diff --git a/src/osd/ReplicatedBackend.h b/src/osd/ReplicatedBackend.h index 0beae0c9a7d..a1325a94847 100644 --- a/src/osd/ReplicatedBackend.h +++ b/src/osd/ReplicatedBackend.h @@ -330,25 +330,21 @@ private: struct InProgressOp { ceph_tid_t tid; set waiting_for_commit; - set waiting_for_applied; Context *on_commit; - Context *on_applied; OpRequestRef op; eversion_t v; InProgressOp( - ceph_tid_t tid, Context *on_commit, Context *on_applied, + ceph_tid_t tid, Context *on_commit, OpRequestRef op, eversion_t v) - : tid(tid), on_commit(on_commit), on_applied(on_applied), + : tid(tid), on_commit(on_commit), op(op), v(v) {} bool done() const { - return waiting_for_commit.empty() && - waiting_for_applied.empty(); + return waiting_for_commit.empty(); } }; map in_progress_ops; public: friend class C_OSD_OnOpCommit; - friend class C_OSD_OnOpApplied; void call_write_ordered(std::function &&cb) override { // ReplicatedBackend submits writes inline in submit_transaction, so @@ -365,7 +361,6 @@ public: const eversion_t &roll_forward_to, const vector &log_entries, boost::optional &hset_history, - Context *on_all_applied, Context *on_all_commit, ceph_tid_t tid, osd_reqid_t reqid, @@ -400,29 +395,26 @@ private: boost::optional &hset_history, InProgressOp *op, ObjectStore::Transaction &op_t); - void op_applied(InProgressOp *op); void op_commit(InProgressOp *op); void do_repop_reply(OpRequestRef op); void do_repop(OpRequestRef op); struct RepModify { OpRequestRef op; - bool applied, committed; + bool committed; int ackerosd; eversion_t last_complete; epoch_t epoch_started; ObjectStore::Transaction opt, localt; - RepModify() : applied(false), committed(false), ackerosd(-1), + RepModify() : committed(false), ackerosd(-1), epoch_started(0) {} }; typedef ceph::shared_ptr RepModifyRef; - struct C_OSD_RepModifyApply; struct C_OSD_RepModifyCommit; - void repop_applied(RepModifyRef rm); void repop_commit(RepModifyRef rm); bool auto_repair_supported() const override { return false; }