From 60842eb1a96fee96104c89ee801118bff823169b Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Mon, 1 Nov 2021 12:08:57 +0000 Subject: [PATCH] osd/scrub: scrubbing schedule - minor related cleanups Signed-off-by: Ronen Friedman --- .gitignore | 1 + src/mon/PGMap.cc | 18 +++++++++--------- src/mon/PGMap.h | 4 ++-- src/osd/OSD.cc | 1 + src/osd/PeeringState.cc | 3 +-- src/osd/PrimaryLogPG.cc | 4 ++-- src/osd/PrimaryLogPG.h | 2 +- src/osd/osd_types.cc | 3 ++- src/osd/osd_types.h | 2 +- src/osd/scrubber/PrimaryLogScrub.cc | 1 - src/osd/scrubber/pg_scrubber.cc | 13 +++++++------ src/osd/scrubber/pg_scrubber.h | 6 +++--- 12 files changed, 30 insertions(+), 28 deletions(-) diff --git a/.gitignore b/.gitignore index 4c9ff3b26deb6..b01aef839bef6 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,4 @@ GTAGS # Python building things where it shouldn't /src/python-common/build/ +.cache diff --git a/src/mon/PGMap.cc b/src/mon/PGMap.cc index d773c2fd3861f..ea90270a9ef8f 100644 --- a/src/mon/PGMap.cc +++ b/src/mon/PGMap.cc @@ -1608,7 +1608,7 @@ void PGMap::dump_osd_stats(ceph::Formatter *f, bool with_net) const void PGMap::dump_osd_ping_times(ceph::Formatter *f) const { f->open_array_section("osd_ping_times"); - for (auto& [osd, stat] : osd_stat) { + for (const auto& [osd, stat] : osd_stat) { f->open_object_section("osd_ping_time"); f->dump_int("osd", osd); stat.dump_ping_time(f); @@ -1617,10 +1617,11 @@ void PGMap::dump_osd_ping_times(ceph::Formatter *f) const f->close_section(); } +// note: dump_pg_stats_plain() is static void PGMap::dump_pg_stats_plain( ostream& ss, const mempool::pgmap::unordered_map& pg_stats, - bool brief) const + bool brief) { TextTable tab; @@ -1661,11 +1662,9 @@ void PGMap::dump_pg_stats_plain( tab.define_column("SCRUB_SCHEDULING", TextTable::LEFT, TextTable::LEFT); } - for (auto i = pg_stats.begin(); - i != pg_stats.end(); ++i) { - const pg_stat_t &st(i->second); + for (const auto& [pg, st] : pg_stats) { if (brief) { - tab << i->first + tab << pg << pg_state_string(st.state) << st.up << st.up_primary @@ -1676,7 +1675,7 @@ void PGMap::dump_pg_stats_plain( ostringstream reported; reported << st.reported_epoch << ":" << st.reported_seq; - tab << i->first + tab << pg << st.stats.sum.num_objects << st.stats.sum.num_objects_missing_on_primary << st.stats.sum.num_objects_degraded @@ -1700,7 +1699,8 @@ void PGMap::dump_pg_stats_plain( << st.last_deep_scrub << st.last_deep_scrub_stamp << st.snaptrimq_len - << st.scrub_duration + << st.last_scrub_duration + << st.dump_scrub_schedule() << TextTable::endrow; } } @@ -2271,7 +2271,7 @@ void PGMap::dump_filtered_pg_stats(ostream& ss, set& pgs) const void PGMap::dump_pool_stats_and_io_rate(int64_t poolid, const OSDMap &osd_map, ceph::Formatter *f, stringstream *rs) const { - string pool_name = osd_map.get_pool_name(poolid); + const string& pool_name = osd_map.get_pool_name(poolid); if (f) { f->open_object_section("pool"); f->dump_string("pool_name", pool_name.c_str()); diff --git a/src/mon/PGMap.h b/src/mon/PGMap.h index c592f338ecb33..d55d2397c1b98 100644 --- a/src/mon/PGMap.h +++ b/src/mon/PGMap.h @@ -460,10 +460,10 @@ class PGMap : public PGMapDigest { void dump_pool_stats_and_io_rate(int64_t poolid, const OSDMap &osd_map, ceph::Formatter *f, std::stringstream *ss) const; - void dump_pg_stats_plain( + static void dump_pg_stats_plain( std::ostream& ss, const mempool::pgmap::unordered_map& pg_stats, - bool brief) const; + bool brief); void get_stuck_stats( int types, const utime_t cutoff, mempool::pgmap::unordered_map& stuck_pgs) const; diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 67664389c879b..ca1dc472b2197 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -7592,6 +7592,7 @@ void OSD::resched_all_scrubs() MPGStats* OSD::collect_pg_stats() { + dout(15) << __func__ << dendl; // This implementation unconditionally sends every is_primary PG's // stats every time we're called. This has equivalent cost to the // previous implementation's worst case where all PGs are busy and diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc index ae9344ec4ab49..b5e6b69573e4c 100644 --- a/src/osd/PeeringState.cc +++ b/src/osd/PeeringState.cc @@ -3810,7 +3810,7 @@ std::optional PeeringState::prepare_stats_for_publish( if (info.stats.state != state) { info.stats.last_change = now; // Optimistic estimation, if we just find out an inactive PG, - // assumt it is active till now. + // assume it is active till now. if (!(state & PG_STATE_ACTIVE) && (info.stats.state & PG_STATE_ACTIVE)) info.stats.last_active = now; @@ -3990,7 +3990,6 @@ void PeeringState::update_stats_wo_resched( f(info.history, info.stats); } - bool PeeringState::append_log_entries_update_missing( const mempool::osd_pglog::list &entries, ObjectStore::Transaction &t, std::optional trim_to, diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index 6c2343ba34f4f..298716a1a0c71 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -947,7 +947,7 @@ PrimaryLogPG::get_pgls_filter(bufferlist::const_iterator& iter) if (type.compare("plain") == 0) { filter = std::make_unique(); } else { - std::size_t dot = type.find("."); + std::size_t dot = type.find('.'); if (dot == std::string::npos || dot == 0 || dot == type.size() - 1) { return { -EINVAL, nullptr }; } @@ -1179,7 +1179,7 @@ void PrimaryLogPG::do_command( if (deep) { set_last_deep_scrub_stamp(stamp); } else { - set_last_scrub_stamp(stamp); + set_last_scrub_stamp(stamp); // also for 'deep', as we use this value to order scrubs } f->open_object_section("result"); f->dump_bool("deep", deep); diff --git a/src/osd/PrimaryLogPG.h b/src/osd/PrimaryLogPG.h index 23ecdffda6396..b92c46bf4a926 100644 --- a/src/osd/PrimaryLogPG.h +++ b/src/osd/PrimaryLogPG.h @@ -196,7 +196,7 @@ class PrimaryLogPG : public PG, public PGBackend::Listener { friend struct CopyFromFinisher; friend class PromoteCallback; friend struct PromoteFinisher; - friend class C_gather; + friend struct C_gather; struct ProxyReadOp { OpRequestRef op; diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc index 9e260de9426ea..d439cdb2e8d71 100644 --- a/src/osd/osd_types.cc +++ b/src/osd/osd_types.cc @@ -6397,8 +6397,9 @@ void object_info_t::dump(Formatter *f) const f->dump_unsigned("lost", (int)is_lost()); vector sv = get_flag_vector(flags); f->open_array_section("flags"); - for (auto str: sv) + for (const auto& str: sv) { f->dump_string("flags", str); + } f->close_section(); f->dump_unsigned("truncate_seq", truncate_seq); f->dump_unsigned("truncate_size", truncate_size); diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index 1bb384548d6d6..65c0cf1f4109d 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -2259,7 +2259,7 @@ struct pg_stat_t { int32_t acting_primary; // snaptrimq.size() is 64bit, but let's be serious - anything over 50k is - // absurd already, so cap it to 2^32 and save 4 bytes at the same time + // absurd already, so cap it to 2^32 and save 4 bytes at the same time uint32_t snaptrimq_len; pg_scrubbing_status_t scrub_sched_status; diff --git a/src/osd/scrubber/PrimaryLogScrub.cc b/src/osd/scrubber/PrimaryLogScrub.cc index 2be7b56f61b03..06dce06f5457f 100644 --- a/src/osd/scrubber/PrimaryLogScrub.cc +++ b/src/osd/scrubber/PrimaryLogScrub.cc @@ -23,7 +23,6 @@ template static ostream& _prefix(std::ostream* _dout, T* t) } using namespace Scrub; -using Scrub::ScrubMachine; bool PrimaryLogScrub::get_store_errors(const scrub_ls_arg_t& arg, scrub_ls_result_t& res_inout) const diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index 9e3256163e901..038d9392c6271 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -3,6 +3,7 @@ #include "./pg_scrubber.h" // the '.' notation used to affect clang-format order +#include #include #include @@ -520,7 +521,7 @@ void PgScrubber::update_scrub_job(const requested_scrub_t& request_flags) } ScrubQueue::sched_params_t -PgScrubber::determine_scrub_time(const requested_scrub_t& request_flags) +PgScrubber::determine_scrub_time(const requested_scrub_t& request_flags) const { ScrubQueue::sched_params_t res; @@ -795,7 +796,7 @@ void PgScrubber::add_delayed_scheduling() if (m_needs_sleep) { double scrub_sleep = 1000.0 * m_osds->get_scrub_services().scrub_sleep_time(m_flags.required); - sleep_time = milliseconds{long(scrub_sleep)}; + sleep_time = milliseconds{int64_t(scrub_sleep)}; } dout(15) << __func__ << " sleep: " << sleep_time.count() << "ms. needed? " << m_needs_sleep << dendl; @@ -1348,7 +1349,7 @@ void PgScrubber::set_op_parameters(requested_scrub_t& request) // not calling update_op_mode_text() yet, as m_is_deep not set yet } - // the publishing here seems to be required for tests synchronization + // the publishing here is required for tests synchronization m_pg->publish_stats_to_osd(); m_flags.deep_scrub_on_error = request.deep_scrub_on_error; } @@ -1919,7 +1920,6 @@ void PgScrubber::on_digest_updates() } } - /* * note that the flags-set fetched from the PG (m_pg->m_planned_scrub) * is cleared once scrubbing starts; Some of the values dumped here are @@ -2049,7 +2049,7 @@ pg_scrubbing_status_t PgScrubber::get_schedule() const void PgScrubber::handle_query_state(ceph::Formatter* f) { - dout(10) << __func__ << dendl; + dout(15) << __func__ << dendl; f->open_object_section("scrub"); f->dump_stream("scrubber.epoch_start") << m_interval_start; @@ -2095,7 +2095,8 @@ PgScrubber::PgScrubber(PG* pg) m_osds->get_nodeid()); } -void PgScrubber::set_scrub_begin_time() { +void PgScrubber::set_scrub_begin_time() +{ scrub_begin_stamp = ceph_clock_now(); } diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index 895ff1232daa8..8da41a0235373 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -671,8 +671,8 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener { */ void request_rescrubbing(requested_scrub_t& req_flags); - ScrubQueue::sched_params_t - determine_scrub_time(const requested_scrub_t& request_flags); + ScrubQueue::sched_params_t determine_scrub_time( + const requested_scrub_t& request_flags) const; void unregister_from_osd(); @@ -745,7 +745,7 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener { */ class preemption_data_t : public Scrub::preemption_t { public: - preemption_data_t(PG* pg); // the PG access is used for conf access (and logs) + explicit preemption_data_t(PG* pg); // the PG access is used for conf access (and logs) [[nodiscard]] bool is_preemptable() const final { return m_preemptable; }