]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
Revert "Merge pull request #12978 from asheplyakov/jewel-18581" 13280/head
authorSamuel Just <sjust@redhat.com>
Mon, 6 Feb 2017 18:20:55 +0000 (10:20 -0800)
committerSamuel Just <sjust@redhat.com>
Tue, 7 Feb 2017 00:28:17 +0000 (16:28 -0800)
See: http://tracker.ceph.com/issues/18809

This reverts commit 8e69580c97622abfcbda73f92d9b6b6780be031f, reversing
changes made to c05730ceb3387fb43c35937f0506297a34a44452.

Signed-off-by: Samuel Just <sjust@redhat.com>
src/osd/PGBackend.h
src/osd/ReplicatedBackend.cc
src/osd/ReplicatedBackend.h
src/osd/ReplicatedPG.h
src/osd/osd_types.h

index 88d2dd7b4921217d3dfbdde1d808977b7f8c9e05..f88c1a08af76441a523760c933d6bfed156eeb0f 100644 (file)
@@ -173,12 +173,6 @@ struct shard_info_wrapper;
        const hobject_t &hoid,
        map<string, bufferlist> &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;
 
index 90aecf374370d25c8cdaf52caf0445d886f81fcc..708ca7855984976c14fb6c40842c13cb2b063966 100644 (file)
@@ -118,8 +118,9 @@ void ReplicatedBackend::check_recovery_sources(const OSDMapRef osdmap)
       for (set<hobject_t, hobject_t::BitwiseComparator>::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<ThreadPool::TPHandle&> {
   ReplicatedBackend *bc;
-  list<ReplicatedBackend::pull_complete_info> to_continue;
+  list<hobject_t> 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<hobject_t>::iterator i =
+          to_continue.begin();
+        i != to_continue.end();
+        ++i) {
+      map<hobject_t, ReplicatedBackend::PullInfo, hobject_t::BitwiseComparator>::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<PullOp> replies(1);
   ObjectStore::Transaction t;
-  list<pull_complete_info> to_continue;
+  list<hobject_t> to_continue;
   for (vector<PushOp>::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<uint64_t>& data_subset,
-  map<hobject_t, interval_set<uint64_t>, hobject_t::BitwiseComparator>& clone_subsets,
-  ObcLockManager &manager)
+  map<hobject_t, interval_set<uint64_t>, 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<uint64_t>& data_subset,
-  map<hobject_t, interval_set<uint64_t>, hobject_t::BitwiseComparator>& clone_subsets,
-  ObcLockManager &manager)
+  map<hobject_t, interval_set<uint64_t>, 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, interval_set<uint64_t>, hobject_t::BitwiseComparator> clone_subsets;
   interval_set<uint64_t> 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<pg_shard_t, pg_info_t>::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<uint64_t> &data_subset,
   map<hobject_t, interval_set<uint64_t>, 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<pull_complete_info> *to_continue,
-  ObjectStore::Transaction *t)
+  list<hobject_t> *to_continue,
+  ObjectStore::Transaction *t
+  )
 {
   interval_set<uint64_t> 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<pull_complete_info> to_continue;
+    list<hobject_t> 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<hobject_t, PullInfo, hobject_t::BitwiseComparator>::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(
index bfc77126e49a59d3b2c85c09db74094fbfebe7ae..ea4d509971206e307f75af208ad9492ec554228e 100644 (file)
@@ -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<pg_shard_t, set<hobject_t, hobject_t::BitwiseComparator> > pull_from_peer;
-  void clear_pull(
-    map<hobject_t, PullInfo, hobject_t::BitwiseComparator>::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<pull_complete_info> *to_continue,
+    list<hobject_t> *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<uint64_t>& data_subset,
-    map<hobject_t, interval_set<uint64_t>, hobject_t::BitwiseComparator>& clone_subsets,
-    ObcLockManager &lock_manager);
+    map<hobject_t, interval_set<uint64_t>, 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<uint64_t> &data_subset,
-    map<hobject_t, interval_set<uint64_t>, 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<uint64_t>& data_subset,
-    map<hobject_t, interval_set<uint64_t>, 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<uint64_t> &data_subset,
+                map<hobject_t, interval_set<uint64_t>, 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<uint64_t>& data_subset,
+                        map<hobject_t, interval_set<uint64_t>, hobject_t::BitwiseComparator>& clone_subsets);
   ObjectRecoveryInfo recalc_subsets(
     const ObjectRecoveryInfo& recovery_info,
-    SnapSetContext *ssc,
-    ObcLockManager &lock_manager);
+    SnapSetContext *ssc
+    );
 
   /**
    * Client IO
index 06c2632436b6ea0c70101d71ed750cb5f1f1baef..149d709c98f8af34dc20700eb72e9d6091d53768 100644 (file)
@@ -378,29 +378,12 @@ public:
   const pg_pool_t &get_pool() const {
     return pool.info;
   }
-
   ObjectContextRef get_obc(
     const hobject_t &hoid,
     map<string, bufferlist> &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<pg_log_entry_t> &logv,
     boost::optional<pg_hit_set_history_t> &hset_history,
index 741c2a2418d4ec5b6cdd22560c5c38bd7a4c144f..1c5d71db308bea26439589c9aaaae99cf2644b7a 100644 (file)
@@ -3676,9 +3676,6 @@ public:
     }
     return false;
   }
-  bool try_get_read_lock() {
-    return rwstate.get_read_lock();
-  }
   void drop_recovery_read(list<OpRequestRef> *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<pair<hobject_t, list<OpRequestRef> > > *to_requeue,
     bool *requeue_recovery,