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 7f2275255678c9d583f7653b403b600fe7e98ae3..cb14e8d1b37eb621dad725bed7e30a9a38dfcef1 100644 (file)
--- 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 73455e91bd8e611d423bae9af876eb9fb86dc697..c7a2750cbfee5bf67d673ad96ab0449c8f18fc34 100644 (file)
--- 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);