From 85dbe8ec90b228de590f7542fd0f7edfa0f245e1 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Fri, 12 Apr 2019 16:34:55 -0700 Subject: [PATCH] PG: begin cleaning up scrub stat and history mutations Signed-off-by: sjust@redhat.com --- src/osd/OSD.cc | 3 -- src/osd/PG.cc | 100 ++++++++++++++++++++-------------------- src/osd/PG.h | 26 +++++++++-- src/osd/PeeringState.cc | 14 ++++++ src/osd/PeeringState.h | 4 ++ src/osd/PrimaryLogPG.cc | 2 +- 6 files changed, 91 insertions(+), 58 deletions(-) diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 8fa0e079663..d1a8e7b754e 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -5613,7 +5613,6 @@ void TestOpsSocketHook::test_ops(OSDService *service, ObjectStore *store, } if (pg->is_primary()) { - pg->unreg_next_scrub(); const pg_pool_t *p = curmap->get_pg_pool(pgid.pool()); double pool_scrub_max_interval = 0; double scrub_max_interval; @@ -5638,8 +5637,6 @@ void TestOpsSocketHook::test_ops(OSDService *service, ObjectStore *store, } else { pg->set_last_scrub_stamp(stamp); } - pg->reg_next_scrub(); - pg->publish_stats_to_osd(); ss << "ok - set" << (deep ? " deep" : "" ) << " stamp " << stamp; } else { ss << "Not primary"; diff --git a/src/osd/PG.cc b/src/osd/PG.cc index d1256e505de..a42768cdde8 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -3384,15 +3384,6 @@ void PG::scrub_finish() osd->clog->debug(oss); } - // finish up - unreg_next_scrub(); - utime_t now = ceph_clock_now(); - info.history.last_scrub = info.last_update; - info.history.last_scrub_stamp = now; - if (scrubber.deep) { - info.history.last_deep_scrub = info.last_update; - info.history.last_deep_scrub_stamp = now; - } // Since we don't know which errors were fixed, we can only clear them // when every one has been fixed. if (repair) { @@ -3412,54 +3403,63 @@ void PG::scrub_finish() << " error(s) present with no repair possible" << dendl; } } - if (deep_scrub) { - if ((scrubber.shallow_errors == 0) && (scrubber.deep_errors == 0)) - info.history.last_clean_scrub_stamp = now; - info.stats.stats.sum.num_shallow_scrub_errors = scrubber.shallow_errors; - info.stats.stats.sum.num_deep_scrub_errors = scrubber.deep_errors; - info.stats.stats.sum.num_large_omap_objects = scrubber.omap_stats.large_omap_objects; - info.stats.stats.sum.num_omap_bytes = scrubber.omap_stats.omap_bytes; - info.stats.stats.sum.num_omap_keys = scrubber.omap_stats.omap_keys; - dout(25) << __func__ << " shard " << pg_whoami << " num_omap_bytes = " - << info.stats.stats.sum.num_omap_bytes << " num_omap_keys = " - << info.stats.stats.sum.num_omap_keys << dendl; - } else { - info.stats.stats.sum.num_shallow_scrub_errors = scrubber.shallow_errors; - // XXX: last_clean_scrub_stamp doesn't mean the pg is not inconsistent - // because of deep-scrub errors - if (scrubber.shallow_errors == 0) - info.history.last_clean_scrub_stamp = now; - } - info.stats.stats.sum.num_scrub_errors = - info.stats.stats.sum.num_shallow_scrub_errors + - info.stats.stats.sum.num_deep_scrub_errors; - if (scrubber.check_repair) { - scrubber.check_repair = false; - if (info.stats.stats.sum.num_scrub_errors) { - state_set(PG_STATE_FAILED_REPAIR); - dout(10) << __func__ << " " << info.stats.stats.sum.num_scrub_errors - << " error(s) still present after re-scrub" << dendl; - } - } - publish_stats_to_osd(); - if (do_deep_scrub) { - // XXX: Auto scrub won't activate if must_scrub is set, but - // setting the scrub stamps affects what users see. - utime_t stamp = utime_t(0,1); - set_last_scrub_stamp(stamp); - set_last_deep_scrub_stamp(stamp); - } - reg_next_scrub(); { + // finish up ObjectStore::Transaction t; - dirty_info = true; - write_if_dirty(t); + recovery_state.update_stats( + [this, do_deep_scrub, deep_scrub](auto &history, auto &stats) { + utime_t now = ceph_clock_now(); + history.last_scrub = recovery_state.get_info().last_update; + history.last_scrub_stamp = now; + if (scrubber.deep) { + history.last_deep_scrub = recovery_state.get_info().last_update; + history.last_deep_scrub_stamp = now; + } + + if (deep_scrub) { + if ((scrubber.shallow_errors == 0) && (scrubber.deep_errors == 0)) + history.last_clean_scrub_stamp = now; + stats.stats.sum.num_shallow_scrub_errors = scrubber.shallow_errors; + stats.stats.sum.num_deep_scrub_errors = scrubber.deep_errors; + stats.stats.sum.num_large_omap_objects = scrubber.omap_stats.large_omap_objects; + stats.stats.sum.num_omap_bytes = scrubber.omap_stats.omap_bytes; + stats.stats.sum.num_omap_keys = scrubber.omap_stats.omap_keys; + dout(25) << "scrub_finish shard " << pg_whoami << " num_omap_bytes = " + << stats.stats.sum.num_omap_bytes << " num_omap_keys = " + << stats.stats.sum.num_omap_keys << dendl; + } else { + stats.stats.sum.num_shallow_scrub_errors = scrubber.shallow_errors; + // XXX: last_clean_scrub_stamp doesn't mean the pg is not inconsistent + // because of deep-scrub errors + if (scrubber.shallow_errors == 0) + history.last_clean_scrub_stamp = now; + } + stats.stats.sum.num_scrub_errors = + stats.stats.sum.num_shallow_scrub_errors + + stats.stats.sum.num_deep_scrub_errors; + if (scrubber.check_repair) { + scrubber.check_repair = false; + if (info.stats.stats.sum.num_scrub_errors) { + state_set(PG_STATE_FAILED_REPAIR); + dout(10) << "scrub_finish " << info.stats.stats.sum.num_scrub_errors + << " error(s) still present after re-scrub" << dendl; + } + } + if (do_deep_scrub) { + // XXX: Auto scrub won't activate if must_scrub is set, but + // setting the scrub stamps affects what users see. + utime_t stamp = utime_t(0,1); + set_last_scrub_stamp(stamp, history, stats); + set_last_deep_scrub_stamp(stamp, history, stats); + } + return true; + }, + &t); int tr = osd->store->queue_transaction(ch, std::move(t), NULL); ceph_assert(tr == 0); } - if (has_error) { queue_peering_event( PGPeeringEventRef( diff --git a/src/osd/PG.h b/src/osd/PG.h index 72fb190c424..1327358d5e9 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -282,14 +282,32 @@ public: return info.history.same_interval_since; } + static void set_last_scrub_stamp( + utime_t t, pg_history_t &history, pg_stat_t &stats) { + stats.last_scrub_stamp = t; + history.last_scrub_stamp = t; + } + void set_last_scrub_stamp(utime_t t) { - info.stats.last_scrub_stamp = t; - info.history.last_scrub_stamp = t; + recovery_state.update_stats( + [=](auto &history, auto &stats) { + set_last_scrub_stamp(t, history, stats); + return true; + }); + } + + static void set_last_deep_scrub_stamp( + utime_t t, pg_history_t &history, pg_stat_t &stats) { + stats.last_deep_scrub_stamp = t; + history.last_deep_scrub_stamp = t; } void set_last_deep_scrub_stamp(utime_t t) { - info.stats.last_deep_scrub_stamp = t; - info.history.last_deep_scrub_stamp = t; + recovery_state.update_stats( + [=](auto &history, auto &stats) { + set_last_deep_scrub_stamp(t, history, stats); + return true; + }); } bool is_deleting() const { diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc index 6be60577410..fb4f7090617 100644 --- a/src/osd/PeeringState.cc +++ b/src/osd/PeeringState.cc @@ -3393,6 +3393,20 @@ void PeeringState::dump_peering_state(Formatter *f) } } +void PeeringState::update_stats( + std::function f, + ObjectStore::Transaction *t) { + if (f(info.history, info.stats)) { + pl->publish_stats_to_osd(); + } + pl->on_info_history_change(); + + if (t) { + dirty_info = true; + write_if_dirty(*t); + } +} + /*------------ Peering State Machine----------------*/ #undef dout_prefix #define dout_prefix (context< PeeringMachine >().dpp->gen_prefix(*_dout) \ diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h index a2c8de0c001..e1f0b611b2d 100644 --- a/src/osd/PeeringState.h +++ b/src/osd/PeeringState.h @@ -1488,6 +1488,10 @@ public: unsigned split_bits, const pg_merge_meta_t& last_pg_merge_meta); + void update_stats( + std::function f, + ObjectStore::Transaction *t = nullptr); + std::optional prepare_stats_for_publish( bool pg_stats_publish_valid, const pg_stat_t &pg_stats_publish, diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index 794a8bd58ae..7ae79317a57 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -15323,7 +15323,7 @@ boost::statechart::result PrimaryLogPG::AwaitAsyncWork::react(const DoSnapWork&) << pg->snap_trimq << dendl; ObjectStore::Transaction t; - pg->dirty_big_info = true; + pg->recovery_state.dirty_big_info = true; pg->write_if_dirty(t); int tr = pg->osd->store->queue_transaction(pg->ch, std::move(t), NULL); ceph_assert(tr == 0); -- 2.39.5