From a99fa1c51eecdacea9b8523dd0542440ad4828cd Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Wed, 13 Nov 2024 03:02:00 -0600 Subject: [PATCH 1/4] common: a simple API to extract md_config_cacher_t cached value Using a type-deduced operator() (i.e. my_opt() ), instead of the existing ValueT() operator (which requires a type-matching cast). Signed-off-by: Ronen Friedman --- src/common/config_cacher.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/common/config_cacher.h b/src/common/config_cacher.h index a84bad08eee..206cfa70263 100644 --- a/src/common/config_cacher.h +++ b/src/common/config_cacher.h @@ -53,6 +53,10 @@ public: operator ValueT() const { return value_cache.load(); } + + ValueT operator*() const { + return value_cache.load(); + } }; #endif // CEPH_CONFIG_CACHER_H From 3cd7d0226fe1230ce4653847f68a610c8fb870d7 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Wed, 6 Nov 2024 08:33:12 -0600 Subject: [PATCH 2/4] osd/scrub: cache frequently used configuration parameters using the md_config_cacher_t, which is a cache object that registers itself to the config observer and caches the up-to-date configuration value. Signed-off-by: Ronen Friedman --- src/common/options/osd.yaml.in | 8 ++-- src/osd/scrubber/pg_scrubber.cc | 66 ++++++++++++++++++--------------- src/osd/scrubber/pg_scrubber.h | 23 ++++++++++++ 3 files changed, 64 insertions(+), 33 deletions(-) diff --git a/src/common/options/osd.yaml.in b/src/common/options/osd.yaml.in index 3bfff92c907..49099f42b71 100644 --- a/src/common/options/osd.yaml.in +++ b/src/common/options/osd.yaml.in @@ -346,7 +346,7 @@ options: default: 5 see_also: - osd_scrub_chunk_max - with_legacy: true + with_legacy: false - name: osd_scrub_chunk_max type: int level: advanced @@ -357,7 +357,7 @@ options: default: 15 see_also: - osd_scrub_chunk_min - with_legacy: true + with_legacy: false - name: osd_shallow_scrub_chunk_min type: int level: advanced @@ -369,7 +369,7 @@ options: see_also: - osd_shallow_scrub_chunk_max - osd_scrub_chunk_min - with_legacy: true + with_legacy: false - name: osd_shallow_scrub_chunk_max type: int level: advanced @@ -380,7 +380,7 @@ options: see_also: - osd_shallow_scrub_chunk_min - osd_scrub_chunk_max - with_legacy: true + with_legacy: false # sleep between [deep]scrub ops - name: osd_scrub_sleep type: float diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index bdea0765279..aa53df5ae8a 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -3,13 +3,13 @@ #include "./pg_scrubber.h" // '.' notation used to affect clang-format order +#include + #include #include #include #include -#include - #include "debug.h" #include "common/ceph_time.h" @@ -824,21 +824,21 @@ namespace { * an aux function to be used in select_range() below, to * select the correct chunk size based on the type of scrub */ -int size_from_conf( +int64_t size_from_conf( bool is_deep, const ceph::common::ConfigProxy& conf, - std::string_view deep_opt, - std::string_view shallow_opt) + const md_config_cacher_t& deep_opt, + const md_config_cacher_t& shallow_opt) { if (!is_deep) { - auto sz = conf.get_val(shallow_opt); + auto sz = *shallow_opt; if (sz != 0) { // assuming '0' means that no distinction was yet configured between // deep and shallow scrubbing - return static_cast(sz); + return sz; } } - return static_cast(conf.get_val(deep_opt)); + return *deep_opt; } } // anonymous namespace @@ -917,16 +917,16 @@ std::optional PgScrubber::select_range() dout(20) << fmt::format( "{} {} mins: {}d {}s, max: {}d {}s", __func__, (m_is_deep ? "D" : "S"), - conf.get_val("osd_scrub_chunk_min"), - conf.get_val("osd_shallow_scrub_chunk_min"), - conf.get_val("osd_scrub_chunk_max"), - conf.get_val("osd_shallow_scrub_chunk_max")) + *osd_scrub_chunk_min, + *osd_shallow_scrub_chunk_min, + *osd_scrub_chunk_max, + *osd_shallow_scrub_chunk_max) << dendl; - const int min_from_conf = size_from_conf( - m_is_deep, conf, "osd_scrub_chunk_min", "osd_shallow_scrub_chunk_min"); - const int max_from_conf = size_from_conf( - m_is_deep, conf, "osd_scrub_chunk_max", "osd_shallow_scrub_chunk_max"); + const int min_from_conf = static_cast(size_from_conf( + m_is_deep, conf, osd_scrub_chunk_min, osd_shallow_scrub_chunk_min)); + const int max_from_conf = static_cast(size_from_conf( + m_is_deep, conf, osd_scrub_chunk_max, osd_shallow_scrub_chunk_max)); const int divisor = static_cast(preemption_data.chunk_divisor()); const int min_chunk_sz = std::max(3, min_from_conf / divisor); @@ -1640,7 +1640,7 @@ void PgScrubber::replica_scrub_op(OpRequestRef op) advance_token(); const auto& conf = m_pg->get_cct()->_conf; const int max_from_conf = size_from_conf( - m_is_deep, conf, "osd_scrub_chunk_max", "osd_shallow_scrub_chunk_max"); + m_is_deep, conf, osd_scrub_chunk_max, osd_shallow_scrub_chunk_max); auto cost = get_scrub_cost(max_from_conf); m_osds->queue_for_rep_scrub(m_pg, m_replica_request_priority, @@ -2546,6 +2546,16 @@ PgScrubber::PgScrubber(PG* pg) , m_pg_id{pg->pg_id} , m_osds{m_pg->osd} , m_pg_whoami{pg->pg_whoami} + , osd_scrub_chunk_max{m_osds->cct->_conf, "osd_scrub_chunk_max"} + , osd_shallow_scrub_chunk_max{m_osds->cct->_conf, + "osd_shallow_scrub_chunk_max"} + , osd_scrub_chunk_min{m_osds->cct->_conf, "osd_scrub_chunk_min"} + , osd_shallow_scrub_chunk_min{m_osds->cct->_conf, + "osd_shallow_scrub_chunk_min"} + , osd_stats_update_period_scrubbing{ + m_osds->cct->_conf, "osd_stats_update_period_scrubbing"} + , osd_stats_update_period_not_scrubbing{ + m_osds->cct->_conf, "osd_stats_update_period_not_scrubbing"} , preemption_data{pg} { m_fsm = std::make_unique(m_pg, this); @@ -2674,7 +2684,8 @@ const OSDMapRef& PgScrubber::get_osdmap() const LoggerSinkSet& PgScrubber::get_logger() const { return *m_osds->clog.get(); } -ostream &operator<<(ostream &out, const PgScrubber &scrubber) { +ostream& operator<<(ostream& out, const PgScrubber& scrubber) +{ return out << scrubber.m_flags; } @@ -2788,16 +2799,14 @@ void PgScrubber::update_scrub_stats(ceph::coarse_real_clock::time_point now_is) using clock = ceph::coarse_real_clock; using namespace std::chrono; - const seconds period_active = seconds(m_pg->get_cct()->_conf.get_val( - "osd_stats_update_period_scrubbing")); + const seconds period_active = seconds(*osd_stats_update_period_scrubbing); if (!period_active.count()) { // a way for the operator to disable these stats updates return; } - const seconds period_inactive = - seconds(m_pg->get_cct()->_conf.get_val( - "osd_stats_update_period_not_scrubbing") + - m_pg_id.pgid.m_seed % 30); + const seconds period_inactive = seconds( + *osd_stats_update_period_not_scrubbing + + m_pg_id.pgid.m_seed % 30); // determine the required update period, based on our current state auto period{period_inactive}; @@ -2831,10 +2840,10 @@ void PgScrubber::update_scrub_stats(ceph::coarse_real_clock::time_point now_is) // ///////////////////// preemption_data_t ////////////////////////////////// -PgScrubber::preemption_data_t::preemption_data_t(PG* pg) : m_pg{pg} +PgScrubber::preemption_data_t::preemption_data_t(PG* pg) : m_pg{pg}, + osd_scrub_max_preemptions{pg->cct->_conf, "osd_scrub_max_preemptions"} { - m_left = static_cast( - m_pg->get_cct()->_conf.get_val("osd_scrub_max_preemptions")); + m_left = *osd_scrub_max_preemptions; } void PgScrubber::preemption_data_t::reset() @@ -2843,8 +2852,7 @@ void PgScrubber::preemption_data_t::reset() m_preemptable = false; m_preempted = false; - m_left = static_cast( - m_pg->cct->_conf.get_val("osd_scrub_max_preemptions")); + m_left = *osd_scrub_max_preemptions; m_size_divisor = 1; } diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index 3d7e16cd359..0d9e8c1e9f6 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -75,6 +75,8 @@ Main Scrubber interfaces: #include #include +#include "common/config_proxy.h" +#include "common/config_cacher.h" #include "osd/PG.h" #include "osd/scrubber_common.h" @@ -895,6 +897,24 @@ class PgScrubber : public ScrubPgIF, // scrub state. ceph::coarse_real_clock::time_point m_last_stat_upd{}; + // ------------------ cached (frequently used) configuration values + + /// initial (& max) number of objects to scrub in one pass - deep scrub + md_config_cacher_t osd_scrub_chunk_max; + /// initial (& max) number of objects to scrub in one pass - shallow + md_config_cacher_t osd_shallow_scrub_chunk_max; + + /// chunk size won't be reduced (when preempted) below this + /// value (deep scrub) + md_config_cacher_t osd_scrub_chunk_min; + /// chunk size won't be reduced below this value (shallow scrub) + md_config_cacher_t osd_shallow_scrub_chunk_min; + + /// stats update (publish_stats_to_osd()) interval while scrubbing + md_config_cacher_t osd_stats_update_period_scrubbing; + /// stats update interval while not scrubbing + md_config_cacher_t osd_stats_update_period_not_scrubbing; + // ------------ members used if we are a replica epoch_t m_replica_min_epoch; ///< the min epoch needed to handle this message @@ -991,6 +1011,9 @@ class PgScrubber : public ScrubPgIF, mutable ceph::mutex m_preemption_lock = ceph::make_mutex("preemption_lock"); bool m_preemptable{false}; bool m_preempted{false}; + + /// the number of preemptions allowed before we start blocking + md_config_cacher_t osd_scrub_max_preemptions; int m_left; size_t m_size_divisor{1}; bool are_preemptions_left() const { return m_left > 0; } From 71de8c0accdbae59b1d66581627e9d1306539336 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Thu, 14 Nov 2024 06:01:50 -0600 Subject: [PATCH 3/4] osd: rm all uses of the cast operator of md_config_cacher_t as this interface is to be removed in the next commit. Signed-off-by: Ronen Friedman --- src/osd/PrimaryLogPG.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index 14d2f85f40f..44f8e85b5ef 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -6006,7 +6006,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& ops) object_info_t& oi = obs.oi; const hobject_t& soid = oi.soid; const bool skip_data_digest = osd->store->has_builtin_csum() && - osd->osd_skip_data_digest; + *osd->osd_skip_data_digest; PGTransaction* t = ctx->op_t.get(); @@ -6069,9 +6069,9 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& ops) // munge ZERO -> TRUNCATE? (don't munge to DELETE or we risk hosing attributes) if (op.op == CEPH_OSD_OP_ZERO && obs.exists && - op.extent.offset < static_cast(osd->osd_max_object_size) && + op.extent.offset < *osd->osd_max_object_size && op.extent.length >= 1 && - op.extent.length <= static_cast(osd->osd_max_object_size) && + op.extent.length <= *osd->osd_max_object_size && op.extent.offset + op.extent.length >= oi.size) { if (op.extent.offset >= oi.size) { // no-op @@ -6781,7 +6781,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& ops) } result = check_offset_and_length( op.extent.offset, op.extent.length, - static_cast(osd->osd_max_object_size), get_dpp()); + *osd->osd_max_object_size, get_dpp()); if (result < 0) break; @@ -6838,7 +6838,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& ops) } result = check_offset_and_length( 0, op.extent.length, - static_cast(osd->osd_max_object_size), get_dpp()); + *osd->osd_max_object_size, get_dpp()); if (result < 0) break; @@ -6888,7 +6888,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& ops) { // zero result = check_offset_and_length( op.extent.offset, op.extent.length, - static_cast(osd->osd_max_object_size), get_dpp()); + *osd->osd_max_object_size, get_dpp()); if (result < 0) break; @@ -6953,7 +6953,7 @@ int PrimaryLogPG::do_osd_ops(OpContext *ctx, vector& ops) result = check_offset_and_length( op.extent.offset, op.extent.length, - static_cast(osd->osd_max_object_size), get_dpp()); + *osd->osd_max_object_size, get_dpp()); if (result < 0) break; From f92042853a014a9a1538a17a97ee33c775671aa3 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Thu, 14 Nov 2024 06:12:23 -0600 Subject: [PATCH 4/4] common: rm the cast operator of md_config_cacher_t as its replacement - the type-deduced operator() - is now in use, and the resulting code is more readable and less error-prone. Signed-off-by: Ronen Friedman --- src/common/config_cacher.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/common/config_cacher.h b/src/common/config_cacher.h index 206cfa70263..91b8152dde1 100644 --- a/src/common/config_cacher.h +++ b/src/common/config_cacher.h @@ -50,10 +50,6 @@ public: conf.remove_observer(this); } - operator ValueT() const { - return value_cache.load(); - } - ValueT operator*() const { return value_cache.load(); }