From 7f047005fc72e1f37a45cde2d742bb2eb1e62881 Mon Sep 17 00:00:00 2001 From: Igor Fedotov Date: Fri, 28 Aug 2020 22:10:56 +0300 Subject: [PATCH] osd/pg: use next when calling collection_list for pg removal Signed-off-by: Igor Fedotov --- src/crimson/osd/pg.cc | 3 ++- src/crimson/osd/pg.h | 3 ++- src/osd/PG.cc | 31 ++++++++++++++++++++++++++----- src/osd/PG.h | 3 ++- src/osd/PeeringState.cc | 9 ++++++++- src/osd/PeeringState.h | 5 ++++- 6 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index b74c19e333c..34ff7478cc2 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -308,7 +308,8 @@ void PG::prepare_write(pg_info_t &info, } } -void PG::do_delete_work(ceph::os::Transaction &t) +ghobject_t PG::do_delete_work(ceph::os::Transaction &t, + ghobject_t _next) { // TODO shard_services.dec_pg_num(); diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h index 740e276e51d..b0403392c14 100644 --- a/src/crimson/osd/pg.h +++ b/src/crimson/osd/pg.h @@ -328,7 +328,8 @@ public: void on_removal(ceph::os::Transaction &t) final { // TODO } - void do_delete_work(ceph::os::Transaction &t) final; + ghobject_t do_delete_work(ceph::os::Transaction &t, + ghobject_t _next) final; // merge/split not ready void clear_ready_to_merge() final {} diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 9bd8aa5f1fe..9b66cb7cd4c 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -3869,7 +3869,8 @@ void PG::C_DeleteMore::complete(int r) { delete this; } -void PG::do_delete_work(ObjectStore::Transaction &t) +ghobject_t PG::do_delete_work(ObjectStore::Transaction &t, + ghobject_t _next) { dout(10) << __func__ << dendl; @@ -3895,25 +3896,45 @@ void PG::do_delete_work(ObjectStore::Transaction &t) osd->sleep_timer.add_event_at(delete_schedule_time, delete_requeue_callback); dout(20) << __func__ << " Delete scheduled at " << delete_schedule_time << dendl; - return; + return _next; } } delete_needs_sleep = true; + ghobject_t next; + vector olist; int max = std::min(osd->store->get_ideal_list_max(), (int)cct->_conf->osd_target_transaction_size); - ghobject_t next; + osd->store->collection_list( ch, - next, + _next, ghobject_t::get_max(), max, &olist, &next); dout(20) << __func__ << " " << olist << dendl; + // make sure we've removed everything + // by one more listing from the beginning + if (_next != ghobject_t() && olist.empty()) { + next = ghobject_t(); + osd->store->collection_list( + ch, + next, + ghobject_t::get_max(), + max, + &olist, + &next); + if (!olist.empty()) { + dout(0) << __func__ << " additional unexpected onode list" + <<" (new onodes has appeared since PG removal started" + << olist << dendl; + } + } + OSDriver::OSTransaction _t(osdriver.get_transaction(&t)); int64_t num = 0; for (auto& oid : olist) { @@ -3936,7 +3957,6 @@ void PG::do_delete_work(ObjectStore::Transaction &t) Context *fin = new C_DeleteMore(this, get_osdmap_epoch()); t.register_on_commit(fin); } else { - dout(20) << __func__ << " finished" << dendl; if (cct->_conf->osd_inject_failure_on_pg_removal) { _exit(1); } @@ -3971,6 +3991,7 @@ void PG::do_delete_work(ObjectStore::Transaction &t) osd->logger->dec(l_osd_pg_removing); } } + return next; } int PG::pg_stat_adjust(osd_stat_t *ns) diff --git a/src/osd/PG.h b/src/osd/PG.h index 3f7c1cd7c9b..5031861e816 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -457,7 +457,8 @@ public: return std::make_unique(this, &t); } - void do_delete_work(ObjectStore::Transaction &t) override; + ghobject_t do_delete_work(ObjectStore::Transaction &t, + ghobject_t _next) override; void clear_ready_to_merge() override; void set_not_ready_to_merge_target(pg_t pgid, pg_t src) override; diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc index c7637c004de..80208f1e772 100644 --- a/src/osd/PeeringState.cc +++ b/src/osd/PeeringState.cc @@ -6700,7 +6700,10 @@ PeeringState::Deleting::Deleting(my_context ctx) : my_base(ctx), NamedState(context< PeeringMachine >().state_history, "Started/ToDelete/Deleting") { + start = ceph::mono_clock::now(); + context< PeeringMachine >().log_enter(state_name); + DECLARE_LOCALS; ps->deleting = true; ObjectStore::Transaction &t = context().get_cur_transaction(); @@ -6721,7 +6724,8 @@ boost::statechart::result PeeringState::Deleting::react( const DeleteSome& evt) { DECLARE_LOCALS; - pl->do_delete_work(context().get_cur_transaction()); + next = pl->do_delete_work(context().get_cur_transaction(), + next); return discard_event(); } @@ -6731,6 +6735,9 @@ void PeeringState::Deleting::exit() DECLARE_LOCALS; ps->deleting = false; pl->cancel_local_background_io_reservation(); + psdout(20) << "Deleting::" << __func__ << this <<" finished in " + << ceph::mono_clock::now() - start + << dendl; } /*--------GetInfo---------*/ diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h index 06954865093..d036c1d00a6 100644 --- a/src/osd/PeeringState.h +++ b/src/osd/PeeringState.h @@ -377,7 +377,8 @@ public: /// Notification of removal complete, t must be populated to complete removal virtual void on_removal(ObjectStore::Transaction &t) = 0; /// Perform incremental removal work - virtual void do_delete_work(ObjectStore::Transaction &t) = 0; + virtual ghobject_t do_delete_work(ObjectStore::Transaction &t, + ghobject_t _next) = 0; // ======================= PG Merge ========================= virtual void clear_ready_to_merge() = 0; @@ -1242,6 +1243,8 @@ public: boost::statechart::custom_reaction< DeleteSome >, boost::statechart::transition > reactions; + ghobject_t next; + ceph::mono_clock::time_point start; explicit Deleting(my_context ctx); boost::statechart::result react(const DeleteSome &evt); void exit(); -- 2.39.5