From: Sage Weil <sage@redhat.com>
Date: Wed, 21 Aug 2019 21:03:43 +0000 (-0500)
Subject: osd: dispatch_context inside PG lock
X-Git-Tag: v15.1.0~1584^2~13
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=8776d6c076a0d282bb68e1c8da666d4b40024ad5;p=ceph.git

osd: dispatch_context inside PG lock

We have been using PeeringCtx (previously RecoveryCtx) to batch up
query, notify, and info messages and send them to other OSDs.  This is
optimizing for small clusters in a way that doesn't apply to large
clusters and complicates the code.

(FWIW I also don't think the batching was working very well anyway.)

Instead, dispatch everything *inside* the PG lock, and do not accumulate
work across multiple PGs.  Avoid re-using the RecoveryCtx at all by
moving it into the loops.

Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 7f2275255678..cb14e8d1b37e 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -8603,25 +8603,23 @@ void OSD::_finish_splits(set<PGRef>& pgs)
   dout(10) << __func__ << " " << pgs << dendl;
   if (is_stopping())
     return;
-  PeeringCtx rctx = create_context();
   for (set<PGRef>::iterator i = pgs.begin();
        i != pgs.end();
        ++i) {
     PG *pg = i->get();
 
+    PeeringCtx rctx = create_context();
     pg->lock();
     dout(10) << __func__ << " " << *pg << dendl;
     epoch_t e = pg->get_osdmap_epoch();
     pg->handle_initialize(rctx);
     pg->queue_null(e, e);
-    dispatch_context_transaction(rctx, pg);
+    dispatch_context(rctx, pg, service.get_osdmap());
     pg->unlock();
 
     unsigned shard_index = pg->pg_id.hash_to_shard(num_shards);
     shards[shard_index]->register_and_wake_split_child(pg);
   }
-
-  dispatch_context(rctx, 0, service.get_osdmap());
 };
 
 bool OSD::add_merge_waiter(OSDMapRef nextmap, spg_t target, PGRef src,
@@ -8679,7 +8677,7 @@ bool OSD::advance_pg(
 		  << " is merge source, target is " << parent
 		   << dendl;
 	  pg->write_if_dirty(rctx);
-	  dispatch_context_transaction(rctx, pg, &handle);
+	  dispatch_context(rctx, pg, pg->get_osdmap(), &handle);
 	  pg->ch->flush();
 	  pg->on_shutdown();
 	  OSDShard *sdata = pg->osd_shard;
@@ -9256,18 +9254,6 @@ PeeringCtx OSD::create_context()
   return PeeringCtx();
 }
 
-void OSD::dispatch_context_transaction(PeeringCtx &ctx, PG *pg,
-                                       ThreadPool::TPHandle *handle)
-{
-  if (!ctx.transaction.empty() || ctx.transaction.has_contexts()) {
-    int tr = store->queue_transaction(
-      pg->ch,
-      std::move(ctx.transaction), TrackedOpRef(), handle);
-    ceph_assert(tr == 0);
-    ctx.reset_transaction();
-  }
-}
-
 void OSD::dispatch_context(PeeringCtx &ctx, PG *pg, OSDMapRef curmap,
                            ThreadPool::TPHandle *handle)
 {
@@ -9938,7 +9924,7 @@ void OSD::dequeue_peering_evt(
       pg->unlock();
       return;
     }
-    dispatch_context_transaction(rctx, pg, &handle);
+    dispatch_context(rctx, pg, curmap, &handle);
     need_up_thru = pg->get_need_up_thru();
     same_interval_since = pg->get_same_interval_since();
     pg->unlock();
@@ -9947,7 +9933,6 @@ void OSD::dequeue_peering_evt(
   if (need_up_thru) {
     queue_want_up_thru(same_interval_since);
   }
-  dispatch_context(rctx, pg, curmap, &handle);
 
   service.send_pg_temp();
 }
diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index 73455e91bd8e..c7a2750cbfee 100644
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1915,8 +1915,6 @@ protected:
   PeeringCtx create_context();
   void dispatch_context(PeeringCtx &ctx, PG *pg, OSDMapRef curmap,
                         ThreadPool::TPHandle *handle = NULL);
-  void dispatch_context_transaction(PeeringCtx &ctx, PG *pg,
-                                    ThreadPool::TPHandle *handle = NULL);
   void discard_context(PeeringCtx &ctx);
   void do_notifies(map<int,vector<pg_notify_t>>& notify_list,
 		   OSDMapRef map);