From 8776d6c076a0d282bb68e1c8da666d4b40024ad5 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 21 Aug 2019 16:03:43 -0500 Subject: [PATCH] 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 --- src/osd/OSD.cc | 23 ++++------------------- src/osd/OSD.h | 2 -- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 7f2275255678c..cb14e8d1b37eb 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -8603,25 +8603,23 @@ void OSD::_finish_splits(set& pgs) dout(10) << __func__ << " " << pgs << dendl; if (is_stopping()) return; - PeeringCtx rctx = create_context(); for (set::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 73455e91bd8e6..c7a2750cbfee5 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>& notify_list, OSDMapRef map); -- 2.39.5