From 9d7844b305cbc78fbc1ca66295f48909f50b2e4f Mon Sep 17 00:00:00 2001 From: myoungwon oh Date: Fri, 12 Mar 2021 22:01:23 +0900 Subject: [PATCH] osd: avoid for the two copy to cancel each other add the op to blocked_list, then return early if the destination of copy-from already exists and the head object is same fixes: https://tracker.ceph.com/issues/49726 Signed-off-by: Myoungwon Oh --- src/osd/PrimaryLogPG.cc | 43 +++++++++++++++++++++++++++++------------ src/osd/PrimaryLogPG.h | 3 ++- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index 94d74a15db47f..4f69fbe6f39e1 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -711,6 +711,18 @@ void PrimaryLogPG::block_write_on_snap_rollback( // otherwise, we'd have blocked in do_op ceph_assert(oid.is_head()); ceph_assert(objects_blocked_on_snap_promotion.count(oid) == 0); + /* + * We block the head object here. + * + * Let's assume that there is racing read When the head object is being rollbacked. + * Since the two different ops can trigger promote_object() with the same source, + * infinite loop happens by canceling ops each other. + * To avoid this, we block the head object during rollback. + * So, the racing read will be blocked until the rollback is completed. + * see also: https://tracker.ceph.com/issues/49726 + */ + ObjectContextRef head_obc = get_object_context(oid, false); + head_obc->start_block(); objects_blocked_on_snap_promotion[oid] = obc; wait_for_blocked_object(obc->obs.oi.soid, op); } @@ -10142,6 +10154,7 @@ void PrimaryLogPG::cancel_copy(CopyOpRef cop, bool requeue, copy_ops.erase(cop->obc->obs.oi.soid); cop->obc->stop_block(); + kick_object_context_blocked(cop->obc); cop->results.should_requeue = requeue; CopyCallbackResults result(-ECANCELED, &cop->results); cop->cb->complete(result); @@ -10152,15 +10165,13 @@ void PrimaryLogPG::cancel_copy(CopyOpRef cop, bool requeue, cop->obc = ObjectContextRef(); } -void PrimaryLogPG::cancel_and_kick_copy_ops(bool requeue, vector *tids) +void PrimaryLogPG::cancel_copy_ops(bool requeue, vector *tids) { dout(10) << __func__ << dendl; map::iterator p = copy_ops.begin(); while (p != copy_ops.end()) { - ObjectContextRef obc = p->second->obc; // requeue this op? can I queue up all of them? cancel_copy((p++)->second, requeue, tids); - kick_object_context_blocked(obc); } } @@ -11866,6 +11877,16 @@ void PrimaryLogPG::add_object_context_to_pg_stat(ObjectContextRef obc, pg_stat_t pgstat->stats.sum.add(stat); } +void PrimaryLogPG::requeue_op_blocked_by_object(const hobject_t &soid) { + map>::iterator p = waiting_for_blocked_object.find(soid); + if (p != waiting_for_blocked_object.end()) { + list& ls = p->second; + dout(10) << __func__ << " " << soid << " requeuing " << ls.size() << " requests" << dendl; + requeue_ops(ls); + waiting_for_blocked_object.erase(p); + } +} + void PrimaryLogPG::kick_object_context_blocked(ObjectContextRef obc) { const hobject_t& soid = obc->obs.oi.soid; @@ -11874,18 +11895,16 @@ void PrimaryLogPG::kick_object_context_blocked(ObjectContextRef obc) return; } - map>::iterator p = waiting_for_blocked_object.find(soid); - if (p != waiting_for_blocked_object.end()) { - list& ls = p->second; - dout(10) << __func__ << " " << soid << " requeuing " << ls.size() << " requests" << dendl; - requeue_ops(ls); - waiting_for_blocked_object.erase(p); - } + requeue_op_blocked_by_object(soid); map::iterator i = objects_blocked_on_snap_promotion.find(obc->obs.oi.soid.get_head()); if (i != objects_blocked_on_snap_promotion.end()) { ceph_assert(i->second == obc); + ObjectContextRef head_obc = get_object_context(i->first, false); + head_obc->stop_block(); + // kick blocked ops (head) + requeue_op_blocked_by_object(i->first); objects_blocked_on_snap_promotion.erase(i); } @@ -12564,7 +12583,7 @@ void PrimaryLogPG::on_shutdown() m_scrubber->unreg_next_scrub(); vector tids; - cancel_and_kick_copy_ops(false, &tids); + cancel_copy_ops(false, &tids); cancel_flush_ops(false, &tids); cancel_proxy_ops(false, &tids); cancel_manifest_ops(false, &tids); @@ -12681,7 +12700,7 @@ void PrimaryLogPG::on_change(ObjectStore::Transaction &t) requeue_ops(waiting_for_readable); vector tids; - cancel_and_kick_copy_ops(is_primary(), &tids); + cancel_copy_ops(is_primary(), &tids); cancel_flush_ops(is_primary(), &tids); cancel_proxy_ops(is_primary(), &tids); cancel_manifest_ops(is_primary(), &tids); diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h index f4fb347c87e60..c922e36485aa8 100644 --- a/src/osd/PrimaryLogPG.h +++ b/src/osd/PrimaryLogPG.h @@ -1329,7 +1329,7 @@ protected: void finish_copyfrom(CopyFromCallback *cb); void finish_promote(int r, CopyResults *results, ObjectContextRef obc); void cancel_copy(CopyOpRef cop, bool requeue, std::vector *tids); - void cancel_and_kick_copy_ops(bool requeue, std::vector *tids); + void cancel_copy_ops(bool requeue, std::vector *tids); friend struct C_Copyfrom; @@ -1844,6 +1844,7 @@ public: bool maybe_await_blocked_head(const hobject_t &soid, OpRequestRef op); void wait_for_blocked_object(const hobject_t& soid, OpRequestRef op); void kick_object_context_blocked(ObjectContextRef obc); + void requeue_op_blocked_by_object(const hobject_t &soid); void maybe_force_recovery(); -- 2.39.5