From 0cf7a6133ee0d4609242d94088dd77e83665aa93 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Mon, 6 Feb 2017 10:20:55 -0800 Subject: [PATCH] Revert "Merge pull request #12978 from asheplyakov/jewel-18581" See: http://tracker.ceph.com/issues/18809 This reverts commit 8e69580c97622abfcbda73f92d9b6b6780be031f, reversing changes made to c05730ceb3387fb43c35937f0506297a34a44452. Signed-off-by: Samuel Just --- src/osd/PGBackend.h | 6 -- src/osd/ReplicatedBackend.cc | 142 ++++++++++++----------------------- src/osd/ReplicatedBackend.h | 59 +++++---------- src/osd/ReplicatedPG.h | 17 ----- src/osd/osd_types.h | 21 ------ 5 files changed, 68 insertions(+), 177 deletions(-) diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h index 88d2dd7b4921..f88c1a08af76 100644 --- a/src/osd/PGBackend.h +++ b/src/osd/PGBackend.h @@ -173,12 +173,6 @@ struct shard_info_wrapper; const hobject_t &hoid, map &attrs) = 0; - virtual bool try_lock_for_read( - const hobject_t &hoid, - ObcLockManager &manager) = 0; - - virtual void release_locks(ObcLockManager &manager) = 0; - virtual void op_applied( const eversion_t &applied_version) = 0; diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc index 90aecf374370..708ca7855984 100644 --- a/src/osd/ReplicatedBackend.cc +++ b/src/osd/ReplicatedBackend.cc @@ -118,8 +118,9 @@ void ReplicatedBackend::check_recovery_sources(const OSDMapRef osdmap) for (set::iterator j = i->second.begin(); j != i->second.end(); ++j) { + assert(pulling.count(*j) == 1); get_parent()->cancel_pull(*j); - clear_pull(pulling.find(*j), false); + pulling.erase(*j); } pull_from_peer.erase(i++); } else { @@ -225,16 +226,7 @@ bool ReplicatedBackend::handle_message( void ReplicatedBackend::clear_recovery_state() { // clear pushing/pulling maps - for (auto &&i: pushing) { - for (auto &&j: i.second) { - get_parent()->release_locks(j.second.lock_manager); - } - } pushing.clear(); - - for (auto &&i: pulling) { - get_parent()->release_locks(i.second.lock_manager); - } pulling.clear(); pull_from_peer.clear(); } @@ -874,18 +866,25 @@ void ReplicatedBackend::_do_push(OpRequestRef op) struct C_ReplicatedBackend_OnPullComplete : GenContext { ReplicatedBackend *bc; - list to_continue; + list to_continue; int priority; C_ReplicatedBackend_OnPullComplete(ReplicatedBackend *bc, int priority) : bc(bc), priority(priority) {} void finish(ThreadPool::TPHandle &handle) { ReplicatedBackend::RPGHandle *h = bc->_open_recovery_op(); - for (auto &&i: to_continue) { - if (!bc->start_pushes(i.hoid, i.obc, h)) { + for (list::iterator i = + to_continue.begin(); + i != to_continue.end(); + ++i) { + map::iterator j = + bc->pulling.find(*i); + assert(j != bc->pulling.end()); + if (!bc->start_pushes(*i, j->second.obc, h)) { bc->get_parent()->on_global_recover( - i.hoid, i.stat); + *i, j->second.stat); } + bc->pulling.erase(*i); handle.reset_tp_timeout(); } bc->run_recovery_op(h, priority); @@ -900,7 +899,7 @@ void ReplicatedBackend::_do_pull_response(OpRequestRef op) vector replies(1); ObjectStore::Transaction t; - list to_continue; + list to_continue; for (vector::iterator i = m->pushes.begin(); i != m->pushes.end(); ++i) { @@ -1262,8 +1261,7 @@ void ReplicatedBackend::calc_head_subsets( const pg_missing_t& missing, const hobject_t &last_backfill, interval_set& data_subset, - map, hobject_t::BitwiseComparator>& clone_subsets, - ObcLockManager &manager) + map, hobject_t::BitwiseComparator>& clone_subsets) { dout(10) << "calc_head_subsets " << head << " clone_overlap " << snapset.clone_overlap << dendl; @@ -1293,8 +1291,7 @@ void ReplicatedBackend::calc_head_subsets( c.snap = snapset.clones[j]; prev.intersection_of(snapset.clone_overlap[snapset.clones[j]]); if (!missing.is_missing(c) && - cmp(c, last_backfill, get_parent()->sort_bitwise()) < 0 && - get_parent()->try_lock_for_read(c, manager)) { + cmp(c, last_backfill, get_parent()->sort_bitwise()) < 0) { dout(10) << "calc_head_subsets " << head << " has prev " << c << " overlap " << prev << dendl; clone_subsets[c] = prev; @@ -1308,7 +1305,6 @@ void ReplicatedBackend::calc_head_subsets( if (cloning.num_intervals() > cct->_conf->osd_recover_clone_overlap_limit) { dout(10) << "skipping clone, too many holes" << dendl; - get_parent()->release_locks(manager); clone_subsets.clear(); cloning.clear(); } @@ -1326,8 +1322,7 @@ void ReplicatedBackend::calc_clone_subsets( const pg_missing_t& missing, const hobject_t &last_backfill, interval_set& data_subset, - map, hobject_t::BitwiseComparator>& clone_subsets, - ObcLockManager &manager) + map, hobject_t::BitwiseComparator>& clone_subsets) { dout(10) << "calc_clone_subsets " << soid << " clone_overlap " << snapset.clone_overlap << dendl; @@ -1361,8 +1356,7 @@ void ReplicatedBackend::calc_clone_subsets( c.snap = snapset.clones[j]; prev.intersection_of(snapset.clone_overlap[snapset.clones[j]]); if (!missing.is_missing(c) && - cmp(c, last_backfill, get_parent()->sort_bitwise()) < 0 && - get_parent()->try_lock_for_read(c, manager)) { + cmp(c, last_backfill, get_parent()->sort_bitwise()) < 0) { dout(10) << "calc_clone_subsets " << soid << " has prev " << c << " overlap " << prev << dendl; clone_subsets[c] = prev; @@ -1382,8 +1376,7 @@ void ReplicatedBackend::calc_clone_subsets( c.snap = snapset.clones[j]; next.intersection_of(snapset.clone_overlap[snapset.clones[j-1]]); if (!missing.is_missing(c) && - cmp(c, last_backfill, get_parent()->sort_bitwise()) < 0 && - get_parent()->try_lock_for_read(c, manager)) { + cmp(c, last_backfill, get_parent()->sort_bitwise()) < 0) { dout(10) << "calc_clone_subsets " << soid << " has next " << c << " overlap " << next << dendl; clone_subsets[c] = next; @@ -1396,7 +1389,6 @@ void ReplicatedBackend::calc_clone_subsets( if (cloning.num_intervals() > cct->_conf->osd_recover_clone_overlap_limit) { dout(10) << "skipping clone, too many holes" << dendl; - get_parent()->release_locks(manager); clone_subsets.clear(); cloning.clear(); } @@ -1458,7 +1450,6 @@ void ReplicatedBackend::prepare_pull( } ObjectRecoveryInfo recovery_info; - ObcLockManager lock_manager; if (soid.is_snap()) { assert(!get_parent()->get_local_missing().is_missing( @@ -1470,12 +1461,10 @@ void ReplicatedBackend::prepare_pull( SnapSetContext *ssc = headctx->ssc; assert(ssc); dout(10) << " snapset " << ssc->snapset << dendl; - calc_clone_subsets( - ssc->snapset, soid, get_parent()->get_local_missing(), - get_info().last_backfill, - recovery_info.copy_subset, - recovery_info.clone_subset, - lock_manager); + calc_clone_subsets(ssc->snapset, soid, get_parent()->get_local_missing(), + get_info().last_backfill, + recovery_info.copy_subset, + recovery_info.clone_subset); // FIXME: this may overestimate if we are pulling multiple clones in parallel... dout(10) << " pulling " << recovery_info << dendl; @@ -1503,13 +1492,10 @@ void ReplicatedBackend::prepare_pull( assert(!pulling.count(soid)); pull_from_peer[fromshard].insert(soid); PullInfo &pi = pulling[soid]; - pi.from = fromshard; - pi.soid = soid; pi.head_ctx = headctx; pi.recovery_info = op.recovery_info; pi.recovery_progress = op.recovery_progress; pi.cache_dont_need = h->cache_dont_need; - pi.lock_manager = std::move(lock_manager); } /* @@ -1529,7 +1515,6 @@ void ReplicatedBackend::prep_push_to_replica( map, hobject_t::BitwiseComparator> clone_subsets; interval_set data_subset; - ObcLockManager lock_manager; // are we doing a clone on the replica? if (soid.snap && soid.snap < CEPH_NOSNAP) { hobject_t head = soid; @@ -1558,12 +1543,10 @@ void ReplicatedBackend::prep_push_to_replica( map::const_iterator pi = get_parent()->get_shard_info().find(peer); assert(pi != get_parent()->get_shard_info().end()); - calc_clone_subsets( - ssc->snapset, soid, - pm->second, - pi->second.last_backfill, - data_subset, clone_subsets, - lock_manager); + calc_clone_subsets(ssc->snapset, soid, + pm->second, + pi->second.last_backfill, + data_subset, clone_subsets); } else if (soid.snap == CEPH_NOSNAP) { // pushing head or unversioned object. // base this on partially on replica's clones? @@ -1574,20 +1557,10 @@ void ReplicatedBackend::prep_push_to_replica( obc, ssc->snapset, soid, get_parent()->get_shard_missing().find(peer)->second, get_parent()->get_shard_info().find(peer)->second.last_backfill, - data_subset, clone_subsets, - lock_manager); + data_subset, clone_subsets); } - prep_push( - obc, - soid, - peer, - oi.version, - data_subset, - clone_subsets, - pop, - cache_dont_need, - std::move(lock_manager)); + prep_push(obc, soid, peer, oi.version, data_subset, clone_subsets, pop, cache_dont_need); } void ReplicatedBackend::prep_push(ObjectContextRef obc, @@ -1601,7 +1574,7 @@ void ReplicatedBackend::prep_push(ObjectContextRef obc, prep_push(obc, soid, peer, obc->obs.oi.version, data_subset, clone_subsets, - pop, cache_dont_need, ObcLockManager()); + pop, cache_dont_need); } void ReplicatedBackend::prep_push( @@ -1611,8 +1584,7 @@ void ReplicatedBackend::prep_push( interval_set &data_subset, map, hobject_t::BitwiseComparator>& clone_subsets, PushOp *pop, - bool cache_dont_need, - ObcLockManager &&lock_manager) + bool cache_dont_need) { get_parent()->begin_peer_recover(peer, soid); // take note. @@ -1628,7 +1600,6 @@ void ReplicatedBackend::prep_push( pi.recovery_progress.data_recovered_to = 0; pi.recovery_progress.data_complete = 0; pi.recovery_progress.omap_complete = 0; - pi.lock_manager = std::move(lock_manager); ObjectRecoveryProgress new_progress; int r = build_push_op(pi.recovery_info, @@ -1759,8 +1730,7 @@ void ReplicatedBackend::submit_push_complete(ObjectRecoveryInfo &recovery_info, ObjectRecoveryInfo ReplicatedBackend::recalc_subsets( const ObjectRecoveryInfo& recovery_info, - SnapSetContext *ssc, - ObcLockManager &manager) + SnapSetContext *ssc) { if (!recovery_info.soid.snap || recovery_info.soid.snap >= CEPH_NOSNAP) return recovery_info; @@ -1768,19 +1738,17 @@ ObjectRecoveryInfo ReplicatedBackend::recalc_subsets( new_info.copy_subset.clear(); new_info.clone_subset.clear(); assert(ssc); - get_parent()->release_locks(manager); // might already have locks - calc_clone_subsets( - ssc->snapset, new_info.soid, get_parent()->get_local_missing(), - get_info().last_backfill, - new_info.copy_subset, new_info.clone_subset, - manager); + calc_clone_subsets(ssc->snapset, new_info.soid, get_parent()->get_local_missing(), + get_info().last_backfill, + new_info.copy_subset, new_info.clone_subset); return new_info; } bool ReplicatedBackend::handle_pull_response( pg_shard_t from, PushOp &pop, PullOp *response, - list *to_continue, - ObjectStore::Transaction *t) + list *to_continue, + ObjectStore::Transaction *t + ) { interval_set data_included = pop.data_included; bufferlist data; @@ -1825,10 +1793,7 @@ bool ReplicatedBackend::handle_pull_response( } pi.obc = get_parent()->get_obc(pi.recovery_info.soid, pop.attrset); pi.recovery_info.oi = pi.obc->obs.oi; - pi.recovery_info = recalc_subsets( - pi.recovery_info, - pi.obc->ssc, - pi.lock_manager); + pi.recovery_info = recalc_subsets(pi.recovery_info, pi.obc->ssc); } @@ -1864,10 +1829,12 @@ bool ReplicatedBackend::handle_pull_response( if (complete) { pi.stat.num_objects_recovered++; - to_continue->push_back({hoid, pi.obc, pi.stat}); + to_continue->push_back(hoid); get_parent()->on_local_recover( hoid, pi.recovery_info, pi.obc, t); - clear_pull(pulling.find(hoid)); + pull_from_peer[from].erase(hoid); + if (pull_from_peer[from].empty()) + pull_from_peer.erase(from); return false; } else { response->soid = pop.soid; @@ -2205,7 +2172,6 @@ bool ReplicatedBackend::handle_push_reply(pg_shard_t peer, PushReplyOp &op, Push stat.num_keys_recovered = reply->omap_entries.size(); stat.num_objects_recovered = 1; - get_parent()->release_locks(pi->lock_manager); pushing[soid].erase(peer); pi = NULL; @@ -2352,7 +2318,7 @@ void ReplicatedBackend::sub_op_push(OpRequestRef op) if (is_primary()) { PullOp resp; RPGHandle *h = _open_recovery_op(); - list to_continue; + list to_continue; bool more = handle_pull_response( m->from, pop, &resp, &to_continue, &t); @@ -2392,22 +2358,10 @@ void ReplicatedBackend::sub_op_push(OpRequestRef op) void ReplicatedBackend::_failed_push(pg_shard_t from, const hobject_t &soid) { get_parent()->failed_push(from, soid); - - clear_pull(pulling.find(soid)); -} - -void ReplicatedBackend::clear_pull( - map::iterator piter, - bool clear_pull_from_peer) -{ - auto from = piter->second.from; - if (clear_pull_from_peer) { - pull_from_peer[from].erase(piter->second.soid); - if (pull_from_peer[from].empty()) - pull_from_peer.erase(from); - } - get_parent()->release_locks(piter->second.lock_manager); - pulling.erase(piter); + pull_from_peer[from].erase(soid); + if (pull_from_peer[from].empty()) + pull_from_peer.erase(from); + pulling.erase(soid); } int ReplicatedBackend::start_pushes( diff --git a/src/osd/ReplicatedBackend.h b/src/osd/ReplicatedBackend.h index bfc77126e49a..ea4d50997120 100644 --- a/src/osd/ReplicatedBackend.h +++ b/src/osd/ReplicatedBackend.h @@ -170,7 +170,6 @@ private: ObjectRecoveryInfo recovery_info; ObjectContextRef obc; object_stat_sum_t stat; - ObcLockManager lock_manager; void dump(Formatter *f) const { { @@ -189,15 +188,12 @@ private: // pull struct PullInfo { - pg_shard_t from; - hobject_t soid; ObjectRecoveryProgress recovery_progress; ObjectRecoveryInfo recovery_info; ObjectContextRef head_ctx; ObjectContextRef obc; object_stat_sum_t stat; bool cache_dont_need; - ObcLockManager lock_manager; void dump(Formatter *f) const { { @@ -221,9 +217,6 @@ private: // Reverse mapping from osd peer to objects beging pulled from that peer map > pull_from_peer; - void clear_pull( - map::iterator piter, - bool clear_pull_from_peer = true); void sub_op_push(OpRequestRef op); void sub_op_push_reply(OpRequestRef op); @@ -243,15 +236,9 @@ private: bool handle_push_reply(pg_shard_t peer, PushReplyOp &op, PushOp *reply); void handle_pull(pg_shard_t peer, PullOp &op, PushOp *reply); - - struct pull_complete_info { - hobject_t hoid; - ObjectContextRef obc; - object_stat_sum_t stat; - }; bool handle_pull_response( pg_shard_t from, PushOp &op, PullOp *response, - list *to_continue, + list *to_continue, ObjectStore::Transaction *t); void handle_push(pg_shard_t from, PushOp &op, PushReplyOp *response, ObjectStore::Transaction *t); @@ -297,8 +284,7 @@ private: SnapSet& snapset, const hobject_t& poid, const pg_missing_t& missing, const hobject_t &last_backfill, interval_set& data_subset, - map, hobject_t::BitwiseComparator>& clone_subsets, - ObcLockManager &lock_manager); + map, hobject_t::BitwiseComparator>& clone_subsets); void prepare_pull( eversion_t v, const hobject_t& soid, @@ -311,31 +297,26 @@ private: void prep_push_to_replica( ObjectContextRef obc, const hobject_t& soid, pg_shard_t peer, PushOp *pop, bool cache_dont_need = true); - void prep_push( - ObjectContextRef obc, - const hobject_t& oid, pg_shard_t dest, - PushOp *op, - bool cache_dont_need); - void prep_push( - ObjectContextRef obc, - const hobject_t& soid, pg_shard_t peer, - eversion_t version, - interval_set &data_subset, - map, hobject_t::BitwiseComparator>& clone_subsets, - PushOp *op, - bool cache, - ObcLockManager &&lock_manager); - void calc_head_subsets( - ObjectContextRef obc, SnapSet& snapset, const hobject_t& head, - const pg_missing_t& missing, - const hobject_t &last_backfill, - interval_set& data_subset, - map, hobject_t::BitwiseComparator>& clone_subsets, - ObcLockManager &lock_manager); + void prep_push(ObjectContextRef obc, + const hobject_t& oid, pg_shard_t dest, + PushOp *op, + bool cache_dont_need); + void prep_push(ObjectContextRef obc, + const hobject_t& soid, pg_shard_t peer, + eversion_t version, + interval_set &data_subset, + map, hobject_t::BitwiseComparator>& clone_subsets, + PushOp *op, + bool cache = false); + void calc_head_subsets(ObjectContextRef obc, SnapSet& snapset, const hobject_t& head, + const pg_missing_t& missing, + const hobject_t &last_backfill, + interval_set& data_subset, + map, hobject_t::BitwiseComparator>& clone_subsets); ObjectRecoveryInfo recalc_subsets( const ObjectRecoveryInfo& recovery_info, - SnapSetContext *ssc, - ObcLockManager &lock_manager); + SnapSetContext *ssc + ); /** * Client IO diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h index 06c2632436b6..149d709c98f8 100644 --- a/src/osd/ReplicatedPG.h +++ b/src/osd/ReplicatedPG.h @@ -378,29 +378,12 @@ public: const pg_pool_t &get_pool() const { return pool.info; } - ObjectContextRef get_obc( const hobject_t &hoid, map &attrs) { return get_object_context(hoid, true, &attrs); } - bool try_lock_for_read( - const hobject_t &hoid, - ObcLockManager &manager) override { - if (is_missing_object(hoid)) - return false; - auto obc = get_object_context(hoid, false, nullptr); - if (!obc) - return false; - return manager.try_get_read_lock(hoid, obc); - } - - void release_locks(ObcLockManager &manager) { - release_object_locks(manager); - } - - void log_operation( const vector &logv, boost::optional &hset_history, diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index 741c2a2418d4..1c5d71db308b 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -3676,9 +3676,6 @@ public: } return false; } - bool try_get_read_lock() { - return rwstate.get_read_lock(); - } void drop_recovery_read(list *ls) { assert(rwstate.recovery_read_marker); rwstate.put_read(ls); @@ -3838,7 +3835,6 @@ public: ObcLockManager() = default; ObcLockManager(ObcLockManager &&) = default; ObcLockManager(const ObcLockManager &) = delete; - ObcLockManager &operator=(ObcLockManager &&) = default; bool empty() const { return locks.empty(); } @@ -3898,23 +3894,6 @@ public: return false; } } - - /// try get read lock - bool try_get_read_lock( - const hobject_t &hoid, - ObjectContextRef obc) { - assert(locks.find(hoid) == locks.end()); - if (obc->try_get_read_lock()) { - locks.insert( - make_pair( - hoid, - ObjectLockState(obc, ObjectContext::RWState::RWREAD))); - return true; - } else { - return false; - } - } - void put_locks( list > > *to_requeue, bool *requeue_recovery, -- 2.47.3